plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
426 stars 574 forks source link

Sort control panels alphabetically by title within each group #3737

Closed JeffersonBledsoe closed 5 months ago

JeffersonBledsoe commented 1 year ago

The control panels within each group are sorted alphabetically

netlify[bot] commented 1 year ago

Deploy Preview for volto ready!

Name Link
Latest commit 988502d620bda767778c1baab6f8334b1a8fee69
Latest deploy log https://app.netlify.com/sites/volto/deploys/654923ac4888620008572c05
Deploy Preview https://deploy-preview-3737--volto.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

cypress[bot] commented 1 year ago

Passing run #5107 ↗︎

0 493 20 0 Flakiness 0

Details:

Merge branch 'master' into sort-controlpanel-by-title
Project: Volto Commit: 4cc3886c1f
Status: Passed Duration: 14:36 💡
Started: May 11, 2023 3:24 PM Ended: May 11, 2023 3:38 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

stevepiercy commented 1 year ago

Will sorting work for all languages?

Will the sort be within groups of control panels?

erral commented 1 year ago

AFAIK if you are using the title they will be sorted by title, because p.restapi takes care of exposing the translated title in the relevant controlpanel. But I can provide some screenshot in some languages to check it.

sneridagh commented 1 year ago

We tried this in the past (in classic, I contributed to it), and it does not work, because of languages. People preferred to have the icon in the same place across languages than have them alphabetically ordered.

erral commented 1 year ago

But we already fixed it some time ago in classic to have it sorted by title:

We should have it both in classic and volto in the same way. I don't know which one (I prefer have it sorted) but the same.

sneridagh commented 1 year ago

I do not know now the state in classic, but I agree that we should follow the same approach.

JeffersonBledsoe commented 1 year ago

I updated the PR title to reflect that it's sorted within each group.

if you are using the title they will be sorted by title

@erral That was my understanding too, but I haven't verified this yet.

People preferred to have the icon in the same place across languages than have them alphabetically ordered.

@sneridagh My experience with multi-lingual sites is very limited, but I imagine most users would use a single language most of the time? I agree that it's not ideal having them move in cases when they do switch, but I can't think of another way right now for them to be logically sorted the other 80% of the time. I'll finish this PR off, can always revisit it :)

erral commented 1 year ago

Here are the screenshots using the branch in this PR with Plone 6b3

French

controlpanels-fr

Spanish

controlpanel-es

Basque

controlpanels-eu

So this works.

davisagli commented 1 year ago

Hmm, why are there different numbers of control panels shown in the different languages?

erral commented 1 year ago

Good question :thinking: I think that's because I didn't change the Volto language when testing this, I only modified Plone language, but I will check it again

erral commented 1 year ago

French

controlpanels-fr-v2

Basque

controlpanels-eu-v2

Spanish

controlpanel-es-v2

Here we have the screenshots with frontend + backend both configured correctly with the same language.

The controlpanel differences in French are because of some controlpanels are directly coded in Volto and are not comming from Plone, so their category indicator must be translated into French in the PO/JSON file and it is not. If you check it carefully we have a category called Général (coming from Plone) and the one below called General which comes from Volto and it is not translated yet.

erral commented 1 year ago

@JeffersonBledsoe can you add a changelog entry so that the build is green?

JeffersonBledsoe commented 1 year ago

@erral I'll sort that shortly, but I've still some tests failing locally

JeffersonBledsoe commented 1 year ago

@tiberiuichim Update the snapshot and did some more testing, all finished :)

JeffersonBledsoe commented 1 year ago

@sneridagh Thoughts on including this in the Plone6 release?

sneridagh commented 11 months ago

@JeffersonBledsoe upss I merged your other PR related to the SSR control panel and now there is a conflict that must be resolved.

JeffersonBledsoe commented 11 months ago

@sneridagh Thanks for merging that other PR! I've fixed the merge conflict here, just waiting for tests to run. Are you fussed about the import sorting that VS Code seems to have done, or want me to undo it for a cleaner PR?

tkimnguyen commented 5 months ago

I have reached my (very low) limit of comprehension about why Controlpanels.test.jsx is problematic :)

sneridagh commented 5 months ago

Doh, clear example of buried PR... should we resurect it? BTW, I took a quick look, so we are sorting by (English - not translated GS property) title? Also in classic?

davisagli commented 5 months ago

@sneridagh I think we should do it. It sorts on the translated title (it's translated in the @controlpanels API). It doesn't make sense to sort in English if we're displaying a different language.

sneridagh commented 5 months ago

Ok I just checked and the endpoint return the translations. @davisagli ok! let's do it.