search

plone / volto

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

feat: publicUi routes to not have cms-ui in public nonContentRoutes #6173

Open giuliaghisini opened 1 month ago

giuliaghisini commented 1 month ago

if we have nonContentRoutes that aren't cms-ui, for example '/search' now are considered as cmsUi. This adds unnecessary things to the page like:

defining this public routes in config we can avoid this and everithing works well :-D

netlify[bot] commented 1 month ago

Deploy Preview for plone-components canceled.

Name Link
Latest commit c0a3f19204113cdfad8b4b7cc3daa0c58d6112ab
Latest deploy log https://app.netlify.com/sites/plone-components/deploys/66c77ce082949c000809d130
ichim-david commented 1 month ago

@mamico @giuliaghisini This doesn't look ready for testing yet due to the unit test failure unitest-failures

stevepiercy commented 3 weeks ago

The failing CI check for readme-link-check is due to https://www.cmscom.jp not responding. @terapyon would you please look into this?

Otherwise it is all green, but could use one more round of reviews. @giuliaghisini would you please add your review? Thank you!

ichim-david commented 3 weeks ago

@mamico you've turned a bugfix into a breaking change. If you modify where the config is at it will need a breaking notice and an upgrade mention. I would wait for this action though to hear what @davisagli thinks about your proposal to move the config setting. EDIT: Was too quick to react, I've added a reply here https://github.com/plone/volto/pull/6173#issuecomment-2294819425

ichim-david commented 3 weeks ago

@mamico you've turned a bugfix into a breaking change. If you modify where the config is at it will need a breaking notice and an upgrade mention. I would wait for this action though to hear what @davisagli thinks about your proposal to move the config setting.

@mamico disregard this statement I should of paid more attention that you added something instead of modifying the name and import. There is no documentation about this new option though

stevepiercy commented 3 weeks ago

For docs, see https://6.docs.plone.org/volto/contributing/documentation.html.

@ichim-david as far as what to document and where, I don't see anything for nonContentRoutes in Routing (so sad), The configuration registry, or Settings reference guide. Would one of these be appropriate locations?

ichim-david commented 3 weeks ago

For docs, see https://6.docs.plone.org/volto/contributing/documentation.html.

@ichim-david as far as what to document and where, I don't see anything for nonContentRoutes in Routing (so sad), The configuration registry, or Settings reference guide. Would one of these be appropriate locations?

It would be good to get the setting documented and in the routing docs to link to the routing based settings

giuliaghisini commented 2 weeks ago

For docs, see https://6.docs.plone.org/volto/contributing/documentation.html. @ichim-david as far as what to document and where, I don't see anything for nonContentRoutes in Routing (so sad), The configuration registry, or Settings reference guide. Would one of these be appropriate locations?

It would be good to get the setting documented and in the routing docs to link to the routing based settings

updated how-to file