Skip to content

Files : Left panel dropdown improvement for NC 25 - #38338

Closed
Jerome-Herbinet wants to merge 3 commits into
nextcloud:stable25from
Jerome-Herbinet:feature/left-panel-dropdown-improvement/stable25
Closed

Files : Left panel dropdown improvement for NC 25#38338
Jerome-Herbinet wants to merge 3 commits into
nextcloud:stable25from
Jerome-Herbinet:feature/left-panel-dropdown-improvement/stable25

Conversation

@Jerome-Herbinet

@Jerome-HerbinetJerome-Herbinet commented May 17, 2023

Copy link
Copy Markdown
Member

Respawn of #38217 (which is now closed)

Before :

before.2.webm

After :

after.2.webm

Checklist

@szaimen

Copy link
Copy Markdown
Contributor

@Jerome-Herbinet do you have some screenshots?

@szaimenszaimen added the 3. to review Waiting for reviews label May 17, 2023
@szaimenszaimen added this to the Nextcloud 25.0.7 milestone May 17, 2023
@Jerome-Herbinet

Copy link
Copy Markdown
MemberAuthor

@Jerome-Herbinet do you have some screenshots?

Could you check the previous PR that contained the same commits #38217 ?

@szaimen

Copy link
Copy Markdown
Contributor

Can you please add them to the PRs description?

@Jerome-Herbinet

Copy link
Copy Markdown
MemberAuthor

Can you please add them to the PRs description?

Done :-)

@blizzzblizzz mentioned this pull request May 17, 2023

@szaimenszaimen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but didnt test

btw, why is the dropdown button of shares not shown all the time?

@Jerome-Herbinet

Copy link
Copy Markdown
MemberAuthor

Can somebody test the Mail app with this PR ?
@GretaD warned me about some side-effects there (see equivalent PR for NC 26 nextcloud-libraries/nextcloud-vue#4103).

@szaimen

Copy link
Copy Markdown
Contributor

Can somebody test the Mail app with this PR ?

Mail is using the vue components so this specific PR should not change the behaviour there...

@blizzz

Copy link
Copy Markdown
Member

moving to 25.0.8

@blizzz

Copy link
Copy Markdown
Member

Why is it open against stable25 in first place?

@szaimen

Copy link
Copy Markdown
Contributor

Why is it open against stable25 in first place?

Because starting with 26 the files app uses the vue component. 25 still has the old aidebar

@jancborchardtjancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice improvement to make it more obvious @Jerome-Herbinet!

One thing I also noted in the other PR: The triangle pointing to the right is confusing, it would be expected to have a triangle pointing down when in closed state, indicating more things open downwards. :)

When open, the arrow should still point down too. Then it would behave exactly like a standard HTML select element.

@Jerome-Herbinet

Copy link
Copy Markdown
MemberAuthor

Very nice improvement to make it more obvious @Jerome-Herbinet!

One thing I also noted in the other PR: The triangle pointing to the right is confusing, it would be expected to have a triangle pointing down when in closed state, indicating more things open downwards. :)

When open, the arrow should still point down too. Then it would behave exactly like a standard HTML select element.

It should be OK with the commit I just made @jancborchardt ; could you test it ?
I have one question ; when I do a "npm run sass", I obtain a large amount of modified files ; is it more than necessary ? Can I commit and push all of them ?
2023-05-22_10-30

@Jerome-Herbinet

Copy link
Copy Markdown
MemberAuthor

Very nice improvement to make it more obvious @Jerome-Herbinet!
One thing I also noted in the other PR: The triangle pointing to the right is confusing, it would be expected to have a triangle pointing down when in closed state, indicating more things open downwards. :)
When open, the arrow should still point down too. Then it would behave exactly like a standard HTML select element.

It should be OK with the commit I just made @jancborchardt ; could you test it ? I have one question ; when I do a "npm run sass", I obtain a large amount of modified files ; is it more than necessary ? Can I commit and push all of them ? 2023-05-22_10-30

... and concerning the other PR, I'll do a complementary PR a little later because #38338 has already been merged.

@blizzzblizzz mentioned this pull request Jun 12, 2023
@blizzz

Copy link
Copy Markdown
Member

moving to 25.0.9

@blizzzblizzz mentioned this pull request Jul 10, 2023
@blizzz

Copy link
Copy Markdown
Member

What's the outlook here?

@blizzz

Copy link
Copy Markdown
Member

moving to 25.0.10

@blizzz

Copy link
Copy Markdown
Member

moving to 25.0.11

@jancborchardt

Copy link
Copy Markdown
Member

@Jerome-Herbinet what’s the current state here? :) If you need help with the frontend/compilation, maybe @juliushaertl@susnux can advise?

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
@susnux
susnuxforce-pushed the feature/left-panel-dropdown-improvement/stable25 branch from 0837a77 to 7e6c1c7CompareAugust 3, 2023 12:59
@susnux

Copy link
Copy Markdown
Contributor

@Jerome-Herbinet
I think you just need to sign the last commit (or squash them and ensure the resulting one is signed).
Build the assets (npm ci && npm run build) (and commit).

Otherwise the PR seems to be fine.

@Jerome-Herbinet

Copy link
Copy Markdown
MemberAuthor

New PR #39779

@Jerome-Herbinet
Jerome-Herbinet deleted the feature/left-panel-dropdown-improvement/stable25 branch August 31, 2023 09:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Jerome-Herbinet@szaimen@blizzz@jancborchardt@susnux