Skip to content

Editor: Add ifPostTypeSupports higher-order component (try conditional render HoC pattern)#4815

Closed
aduth wants to merge 2 commits intomasterfrom
add/if-post-type-supports
Closed

Editor: Add ifPostTypeSupports higher-order component (try conditional render HoC pattern)#4815
aduth wants to merge 2 commits intomasterfrom
add/if-post-type-supports

Conversation

@aduth
Copy link
Copy Markdown
Member

@aduth aduth commented Feb 1, 2018

This pull request seeks to implement a new ifPostTypeSupports higher-order component which serves in place of the existing PostTypeSupportCheck and attempts to explore a new generalized pattern for conditional component rendering.

Example Before:

function MyFeaturedImage() {
	return (
		<PostTypeSupportCheck supportKeys="thumbnail">
			<img />
		</PostTypeSupportCheck>
	);
}

export default connect( /* ... */ )( MyFeaturedImage );

Example After:

function MyFeaturedImage() {
	return <img />;
}

export default compose( [
	ifPostTypeSupports( 'thumbnail' ),
	connect( /* ... */ ),
] )( MyFeaturedImage );

Implementation notes:

The thinking behind the naming convention is that we could create higher-order component-libraries with the following expectations:

Name Behavior
ifSomeCondition Renders component only if condition applies
withSomeProps Renders component with additional props

Testing instructions:

Verify that there are no regressions in the behavior of post-type-conditional elements:

  • Discussion (comments or trackbacks supports)
  • Featured image (thumbnail support)
  • Excerpt (excerpt support)
  • Post format (post-formats support)
  • Author (author support)
  • Order (page-attributes support)
  • Last Revision (revisions support)

@aduth aduth added the [Type] Question Questions about the design or development of the editor. label Feb 1, 2018
@gziolo
Copy link
Copy Markdown
Member

gziolo commented Feb 1, 2018

@jorgefilipecosta opened a similar PR. See #4812. In general both implementation are similar 😃

@aduth
Copy link
Copy Markdown
Member Author

aduth commented Feb 1, 2018

Well then... 😅

I think we could probably combine the two branches:

  • Take README.md and completed replacement of PostTypeSupportsCheck from mine...
  • ...and withConditionalRender (good idea!) and tests from @jorgefilipecosta 's

Can take a look tomorrow, or if @jorgefilipecosta wants to go ahead, by all means.

@jorgefilipecosta
Copy link
Copy Markdown
Member

Thank you @aduth, I will merge both PR's 👍

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Feb 2, 2018

Thank you @aduth, I will merge both PR's

Yes, that would be nice. This one answers also some questions I had in the other PR :)

@aduth
Copy link
Copy Markdown
Member Author

aduth commented Feb 7, 2018

Closing in favor of #4812.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Question Questions about the design or development of the editor.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants