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

Editor: Use Redux post types state for determining page feature support #3336

Merged
merged 4 commits into from
Feb 26, 2016

Conversation

aduth
Copy link
Contributor

@aduth aduth commented Feb 16, 2016

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.

  1. Navigate to the post editor or post editor
  2. Select a site, if prompted
  3. Note that sidebar options are reflective of the features supported by the current type
    • Feature detection includes: Tags, excerpt, geolocation, and comments
    • If you're unfamiliar with supported features for your theme, check against the /sites/%s/post-types endpoint for your site (example)

@aduth aduth added [Feature] Post/Page Editor The editor for editing posts and pages. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 16, 2016
@aduth aduth self-assigned this Feb 16, 2016
@aduth aduth force-pushed the update/editor-drawer-post-types-query branch from c0cb733 to 3a512e0 Compare February 18, 2016 22:01
// assume positively if we can't determine support
if ( ! currentType || ! currentType.supports ) {
// Default to true until post types are known
if ( ! postTypes ) {
Copy link
Contributor

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?

Copy link
Contributor

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.

@timmyc
Copy link
Contributor

timmyc commented Feb 24, 2016

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

@timmyc timmyc added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 24, 2016
@aduth
Copy link
Contributor Author

aduth commented Feb 24, 2016

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.

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).

@aduth aduth force-pushed the update/editor-drawer-post-types-query branch from 3a512e0 to ebc5203 Compare February 24, 2016 18:56
@aduth aduth force-pushed the update/editor-drawer-post-types-query branch from ebc5203 to 4049db8 Compare February 26, 2016 13:04
aduth added a commit that referenced this pull request Feb 26, 2016
…pes-query

Editor: Use Redux post types state for determining page feature support
@aduth aduth merged commit ed0695c into master Feb 26, 2016
@aduth aduth deleted the update/editor-drawer-post-types-query branch February 26, 2016 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Post/Page Editor The editor for editing posts and pages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants