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

Post Editor: remove check for 'post' prop when rendering post formats accordion #25025

Merged
merged 1 commit into from
May 24, 2018

Conversation

jsnajdr
Copy link
Member

@jsnajdr jsnajdr commented May 23, 2018

The Post Format Accordion does its own check for null post or siteId and doesn't render anything in that case: it waits until the postFormats for the site and post type are fully known.

There's no need to check the post prop in the EditorDrawer component. This patch removes that check, which is one of the last remaining usages of the Flux-based post object.

I also do a little cleanup of the Post Formats Accordion, namely removing the isLoading prop. As the component doesn't render anything at all until all data are available, it's redundant. The prop is a leftover from earlier refactoring in #8593.

How to test:
Verify that the Post Formats still work as expected:

  • shown only on posts (not pages or other post types)
  • shown only on themes that support them (I used "Pique")
  • no console errors on loading, especially on a new post or a post not in local state

… accordion

The Post Format Accordion does its own check for null `post` or `siteId` and
doesn't render anything in that case: it waits until the `postFormats` for the
site and post type are fully known.

There's no need to check the `post` prop in the `EditorDrawer` component. This patch
removes that check, which is one of the last remaining usages of the Flux-based post
object.

I also do a little cleanup of the Post Formats Accordion, namely removing the `isLoading`
prop. As the component doesn't render anything at all until all data are available, it's
redundant. The prop is a leftover from earlier refactoring in #8593.
@jsnajdr jsnajdr 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. [Status] Needs e2e Testing labels May 23, 2018
@jsnajdr jsnajdr self-assigned this May 23, 2018
@jsnajdr jsnajdr requested review from spen, tyxla and a team May 23, 2018 17:23
Copy link
Contributor

@spen spen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also tested with Pique, tried creating a new post etc. all works well :) Thanks @jsnajdr!

@jsnajdr jsnajdr merged commit 0942c75 into master May 24, 2018
@jsnajdr jsnajdr deleted the remove/editor-drawer-post-prop branch May 24, 2018 19:57
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. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants