Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Files : Left panel dropdown improvement for NC 25 #38338

Conversation

Jerome-Herbinet
Copy link
Member

@Jerome-Herbinet Jerome-Herbinet commented May 17, 2023

Respawn of #38217 (which is now closed)

Before :

before.2.webm

After :

after.2.webm

Checklist

@szaimen
Copy link
Contributor

szaimen commented May 17, 2023

@Jerome-Herbinet do you have some screenshots?

@szaimen szaimen added the 3. to review Waiting for reviews label May 17, 2023
@szaimen szaimen added this to the Nextcloud 25.0.7 milestone May 17, 2023
@Jerome-Herbinet
Copy link
Member Author

@Jerome-Herbinet do you have some screenshots?

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

@szaimen
Copy link
Contributor

szaimen commented May 17, 2023

Can you please add them to the PRs description?

@Jerome-Herbinet
Copy link
Member Author

Can you please add them to the PRs description?

Done :-)

@blizzz blizzz mentioned this pull request May 17, 2023
Copy link
Contributor

@szaimen szaimen left a comment

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
Member Author

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
Contributor

szaimen commented May 17, 2023

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
Member

blizzz commented May 17, 2023

moving to 25.0.8

@blizzz
Copy link
Member

blizzz commented May 17, 2023

Why is it open against stable25 in first place?

@szaimen
Copy link
Contributor

szaimen commented May 17, 2023

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

Copy link
Member

@jancborchardt jancborchardt left a comment

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
Member Author

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
Member Author

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.

@blizzz blizzz mentioned this pull request Jun 12, 2023
@blizzz
Copy link
Member

blizzz commented Jun 15, 2023

moving to 25.0.9

@blizzz blizzz mentioned this pull request Jul 10, 2023
@blizzz
Copy link
Member

blizzz commented Jul 10, 2023

What's the outlook here?

@blizzz
Copy link
Member

blizzz commented Jul 13, 2023

moving to 25.0.10

@blizzz
Copy link
Member

blizzz commented Aug 3, 2023

moving to 25.0.11

@jancborchardt
Copy link
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 susnux force-pushed the feature/left-panel-dropdown-improvement/stable25 branch from 0837a77 to 7e6c1c7 Compare August 3, 2023 12:59
@susnux
Copy link
Contributor

susnux commented Aug 3, 2023

@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
Member Author

New PR #39779

@Jerome-Herbinet Jerome-Herbinet deleted the feature/left-panel-dropdown-improvement/stable25 branch August 31, 2023 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants