-
-
Notifications
You must be signed in to change notification settings - Fork 714
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
Use all the apiExpanders in use, so we perform a single request for getting all the required data. #4946
Conversation
…etting all the required data.
✅ Deploy Preview for volto ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I don't know if there's an extra check for duplicate appextras, some projects may have added themselves. A word in the upgrade guide is needed, for sure. |
@@ -0,0 +1 @@ | |||
Use all the apiExpanders in use, so we perform a single request for getting all the required data. @sneridagh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am no Steve, but I will still suggest a minor change just to avoid the repetition of the word "use" and to clarify 😛
Use all the apiExpanders in use, so we perform a single request for getting all the required data. @sneridagh | |
Use all the available apiExpanders, so we perform a single request for getting all the required data. @sneridagh |
@@ -76,6 +76,8 @@ let config = { | |||
publicURL, | |||
apiPath, | |||
apiExpanders: [ | |||
// Added here for documentation purposes, addded at the end because it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Added here for documentation purposes, addded at the end because it | |
// Added here for documentation purposes, added at the end because it |
match: '', | ||
GET_CONTENT: ['navigation'], | ||
querystring: { | ||
'expand.navigation.depth': config.settings.navDepth, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work if final projects change navDepth
? I am not entirely sure, or I don't get why it works at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct, it won't... since it's passed once, with the "old", default value... Not an easy one :/
Suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sneridagh An idea: make it possible to pass a function that constructs the querystring, instead of a prebuilt querystring object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it. Now that this is dealt with, I would move this whole section to line 78, where apiExpanders
is set inside config.settings
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pnicolli it turns out we can't because we haven't set config.settings.navDepth
, unless we assume we are going to sync them manually (I guess that the default is not going to change that much).
…on when the actions expander is set
Passing run #6443 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
@davisagli for some reason the Plone 5 expander for types is not working? :/ How that could be? |
… of the token, in order to remove the types expander safely
@davisagli @pnicolli ready. |
* master: (42 commits) make selectedView and className props available for Search block (#4997) Release @plone/volto-testing 4.0.0-alpha.0 Release 17.0.0-alpha.21 Upgrade to Cypress 12.17.1 (latest) (#4981) Image rendering (#3337) feat(Url.js): add getFieldURL helper function to get the url value of a field based on its structure (#4731) Handle @linkintegrity response with items but no breaches (#4832) Release 17.0.0-alpha.20 Use all the apiExpanders in use, so we perform a single request for getting all the required data. (#4946) Fix the condition deciding on listing pagination format so it takes into account container blocks as well (#4978) Release 17.0.0-alpha.19 Fix search block input clear button doesn't reset the search (#4837) Add /ok route as an express middleware (#4432) handles condition for yearly frequency in recurrence (#4604) Remove dangling out of place Guillotina Cypress tests (#4980) Update to latest plone.restapi and Plone 6.0.6 (#4979) Update browserlist (#4977) `Links and references` view via content menu [Add `Links to item` view] (#4787) Release 17.0.0-alpha.18 Toc responsive (#4912) ...
No description provided.