-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Editor: Use Redux post types state for determining page feature support #3336
Conversation
c0cb733
to
3a512e0
Compare
// assume positively if we can't determine support | ||
if ( ! currentType || ! currentType.supports ) { | ||
// Default to true until post types are known | ||
if ( ! postTypes ) { |
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.
could this return just be combined with the check for ! site
above?
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.
Ah I see it is returning true
- and that was the prior logic. Worst case I guess it could cause some flicker of a sidebar module.
Looks good and tests out well. As noted above, the defaulting to true until the post-types are loaded does cause sidebar accordions to be shown at first, then hide once the store is loaded ( i.e. tags on pages when not supported ). Since this is the current behavior - I think it is fine - and the other option would be to have these items "appear" after they redux state is loaded too - so we can't really get away from it I suppose. LGTM |
Yeah, if I recall, the original decision was based around the fact that it's more likely that the post type will support a particular feature than it not, especially in the case of editing posts, which support all of the features we'd want to detect (unless the theme explicitly removes support). |
3a512e0
to
ebc5203
Compare
For consistency, avoiding additional flickers
Mirrors current production behavior, avoids an unnecessary network request, definitely a hack.
ebc5203
to
4049db8
Compare
…pes-query Editor: Use Redux post types state for determining page feature support
Related: #3264, #3197
This pull request seeks to convert existing editor code using legacy
lib/post-types-list
to using Redux-based state and query components. This will become especially relevant with #3197, as otherwise the two approaches would make separate requests for post types from the REST API.Implementation notes:
There is significant overlap between this pull request and #3264. In fact, this pull request cherry-picks 079f8b5, 19b11d0, and 776afa8 to incorporate
<QueryPostTypes />
. Whichever pull request is merged first, the other will be rebased to remove the redundant commits.Testing instructions:
Verify that post type support continues to behave as expected. This can be dependent upon the current theme of the site, but typically posts support "all" features, whereas a page type will usually not support tags.
/sites/%s/post-types
endpoint for your site (example)