Files : Left panel dropdown improvement for NC 25

[Jerome-Herbinet]

Respawn of #38338

/!\ Reviewers' test needed

Before :

before.2.webm

After :

after.2.webm

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[jancborchardt]

Let me know if the video is outdated @Jerome-Herbinet, just same feedback as in the other review. :)

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]

Let me know if the video is outdated @Jerome-Herbinet, just same feedback as in the other review. :)

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.

@jancborchardt here is an updated video :
Capture vidéo du 10-08-2023 11:11:35.webm

[jancborchardt]

Very nice! Would be good to also habe hover/focus feedback on the triangle button, but that can also be done in a follow-up.

[Jerome-Herbinet]

Very nice! Would be good to also habe hover/focus feedback on the triangle button, but that can also be done in a follow-up.

Thanks @jancborchardt ! Can you mention another appropriate reviewer for testing + approval ?

[jancborchardt]

@skjnldsv@szaimen@susnux another review needed. :)

[jancborchardt]

Also cc @AndyScherzinger would be nice to get this fix in for 27.1.0 as it makes the navigation subitems more obvious.

[AndyScherzinger]

@jancborchardt this PR explicitely targets stable25, so it is not following any backport-logic.

I can't tell if this is even applicable for anything newer than 25 tbh, will need feedback from @skjnldsv

If it is applicable on 26/27 then we can see if we can get it in, but sure is too late for beta1 and hence likely to be blocked by Marketing.

[AndyScherzinger]

Checked with the devs, this PR is v25-only (!)
v26+ the left sidebar got migrated to vue in contrast to 25 being legacy JS. So this can't be used on anything but 25 and would need to be completely re-implemented for 26+ -> not happening for 27.1 then

[jancborchardt]

Ah got it, sorry for the noise then!

[jancborchardt]

@Jerome-Herbinet do you think this is still applicable then? I’d even say since it will soon be 3 versions old anyway, let’s look to the future instead?

[AndyScherzinger]

Ah got it, sorry for the noise then!

All good, still worth the question.

@Jerome-Herbinet do you think this is still applicable then? I’d even say since it will soon be 3 versions old anyway, let’s look to the future instead?

I'd second that question. @Jerome-Herbinet would you be willing to work on the solution for the master branch? That is targeting v28 but we could then still discuss a back-port to stable27 even though I believe it won't make it for the September release, so best maybe then to aim straight for v28.

[skjnldsv]

I'm not sure I get it. This is only about the triangle on the right instead of left, right?
Then Nextcloud 27 and 28 already have those 🤔

What's left to do on master then? 🤔

[Jerome-Herbinet]

I'm not sure I get it. This is only about the triangle on the right instead of left, right?
Then Nextcloud 27 and 28 already have those 🤔

What's left to do on master then? 🤔

ASFAIK recent major NC versions (since 26 ?), manage this in nextcloud-vue and it might be already done ; testing and confirmation might be necessary on 26 27 and 28 if possible.

[skjnldsv]

Absolutely, since 26 this is nc/vue, so we're good.
I think we can close then :)

[AndyScherzinger]

Well, then we could also just merge this one, no? Since it is already "resolved" on 26+ @skjnldsv@Jerome-Herbinet
Or we drop it since it is resolved on 26+

[skjnldsv]

I would tend to be on the careful side here and avoid touching core/css/apps.scss on a version that's going unsupported in a few weeks :)

[Jerome-Herbinet]

Well, then we could also just merge this one, no? Since it is already "resolved" on 26+ @skjnldsv@Jerome-Herbinet Or we drop it since it is resolved on 26+

@AndyScherzinger, I've just checked if my personal 26.0.5 instance ships this UI enhancement ... but it's not the case 🤔 ... that's strange, although this PR was merged nextcloud-libraries/nextcloud-vue#4103
Cc @skjnldsv

[skjnldsv]

Indeed, this is v8.0.0 and v7.11.5+ only
So it will come in upcoming 26.0.6 #39812