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

Fixes #5105; For the Sitemap component, don't hardcode root as navigation action path #5106

Merged
merged 2 commits into from Aug 22, 2023

Conversation

tiberiuichim
Copy link
Contributor

@tiberiuichim tiberiuichim commented Aug 21, 2023

The plone.restapi navigation endpoint will compute a navigation root from a context path, but the problem is that Volto will trigger the navigation endpoint either on root or on the lang folder.

https://github.com/plone/plone.restapi/blob/ed070e0769c4ac3efda05448db8846f61dae255c/src/plone/restapi/services/navigation/get.py#L40

https://github.com/plone/volto/blob/4523b13d6586891b9dad7db1caedaab5f6f7dd4b/src/components/theme/Sitemap/Sitemap.jsx#L115C21-L115C21

path: `${url}/@navigation${

@netlify
Copy link

netlify bot commented Aug 21, 2023

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit 74d5e5a
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/64e348c2146dff0008840001

@cypress
Copy link

cypress bot commented Aug 21, 2023

Passing run #6928 ↗︎

0 553 20 0 Flakiness 0

Details:

Add newsfile
Project: Volto Commit: 74d5e5a2d3
Status: Passed Duration: 12:40 💡
Started: Aug 21, 2023 11:25 AM Ended: Aug 21, 2023 11:37 AM

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

@erral
Copy link
Sponsor Member

erral commented Aug 21, 2023

this should also be taken into account in #3537

@tiberiuichim tiberiuichim changed the title Fixes #5105; Don't hardcode root as navigation action path Fixes #5105; For the Sitemap component, don't hardcode root as navigation action path Aug 21, 2023
@sneridagh sneridagh merged commit bcbafcf into master Aug 22, 2023
53 checks passed
@sneridagh sneridagh deleted the navroot_sitemap branch August 22, 2023 13:03
sneridagh pushed a commit that referenced this pull request Aug 25, 2023
@erral
Copy link
Sponsor Member

erral commented Sep 13, 2023

@tiberiuichim revisiting this PR in order to do a more complete use of the navroot endpoint (#3537), I see quite easy to change the usage of isMultilingual and toBackendLang here and replace them with the navroot.

But I don't get at all how this could be done for the SSR part (I guess, that's what it's done in the asyncConnect part, right?)

Perhaps, if #3537 is merged, I can prepare a PR bringing the usage of @navroot here and we can discuss there how to bring it also to the SSR part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants