Use navroot to render the sitemap (targets 17.x.x)
#5185
Conversation
✅ Deploy Preview for volto ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| settings.isMultilingual | ||
| ? toBackendLang(getState().intl.locale) | ||
| : null, | ||
| const navroot = getState().navroot.data?.navroot?.navroot?.['@id']; |
There was a problem hiding this comment.
Is the navroot available in both 7.x and 8.x branches of plone.restapi?
There was a problem hiding this comment.
No right now it is not. But I can try to add a PR to backport them .
There was a problem hiding this comment.
Why does it need to be backported? This fix will go in Volto 17 and maybe Volto 16, which both expect plone.restapi 8.x, even on Plone 5.
There was a problem hiding this comment.
https://github.com/plone/plone.restapi/blob/ed192208499989b379de8534a3aa12d037316db4/setup.py#L11
Volto 16 works fine with Plone 4, we're using it in production on a complex site. If this PR is backported to Volto 16, it will be a breaking change.
There was a problem hiding this comment.
Okay, I didn't know you are using Volto 16 with Plone 4. That is not officially supported, as far as I am aware, and there are no automated tests for it in volto. So this is a risk you took on in using Volto 16 with an unsupported Plone version. But, I agree we should avoid breaking that, unless it's really hard to avoid.
There was a problem hiding this comment.
The navroot endpoints are not that complex, I would say that they could be backported to 7.x easily. But let's see what the tests say 😅
There was a problem hiding this comment.
Co-authored-by: David Glick <[email protected]>
Co-authored-by: Nina <[email protected]> Co-authored-by: Steve Piercy <[email protected]> Co-authored-by: Piero Nicolli <[email protected]>
Co-authored-by: Nilesh <[email protected]>
Co-authored-by: Nilesh <[email protected]> Co-authored-by: Víctor Fernández de Alba <[email protected]>
Co-authored-by: nileshgulia1 <[email protected]>
Co-authored-by: Nilesh <[email protected]>
Co-authored-by: Nilesh <[email protected]>
Co-authored-by: Nilesh <[email protected]>
| @@ -23,12 +23,6 @@ const messages = defineMessages({ | |||
| }, | |||
| }); | |||
|
|
|||
| export function getSitemapPath(pathname = '', lang) { | |||
There was a problem hiding this comment.
This would be a breaking change because the getSitemapPath is removed. Let's keep it and mark it as deprecated? @sneridagh your thoughts on this?
There was a problem hiding this comment.
I have kept the function and the tests, but mark it as deprecated with a docstring. This way, as you say, if anyone is using the function, it keeps working.
Would that be enough?
erral
changed the title
17.x.x)
|
@erral Sorry, apparently, this got lost in the sea of PRs, merging now! |
In #5105 we changed the sitemap component to be rooted to the language.
Instead doing that, in the PR we use the available navroot to do it.
Moreover I have extracted the hardcoded value of 4 used to call the
getNavigationaction to asiteMapDepthsetting.