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

Components: Extract Reusable Post Sticky Component #3114

Merged
merged 1 commit into from
Oct 27, 2017

Conversation

youknowriad
Copy link
Contributor

Related to #2761 (comment)

All these (the sidebar panels) must be refactored slightly to drop all the "layout" pieces and keep only the "forms". For example, the sidebar panels like PostTaxonomies, we should extract only the PostTaxonomyForms into a reusable component without the expandable panel behavior that is specific to our current UI. (This step could be done in a separate PR preceding the directory structure change)

This is the first one of multiple PRs addressing the comment above and extracting reusable pieces from our current sidebar panels. This PR does so for the PostSticky component.

Testing instructions

  • Test that the post sticky toggle still works and shows as expected.

@youknowriad youknowriad self-assigned this Oct 24, 2017
@youknowriad youknowriad requested a review from aduth October 24, 2017 08:38
*/
import { getCurrentPostType } from '../selectors';

export function PostStickyCheck( { postType, children, user } ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid duplicating the "support sticky" check, I extracted it to its own component. I could reuse this approach for all the other sidebar panels.

Copy link
Contributor

Choose a reason for hiding this comment

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

This works well.

Otherwise, I could imagine the following:

// new HoC
const withUserPrivileges = ( predicate ) =>
  flowRight(
    withAPIData( constant( { user: '/wp/v2…' } ) ),
    withRenderIf( predicate )
  );

// consumer
import withUserPrivilege from ;
const applyPostStickyCheck = withUserPrivileges( ( { postType, user } ) =>
  postType !== 'post' ||
    ! user.data ||
    ! user.data.capabilities.publish_posts ||
    ! user.data.capabilities.edit_others_posts
);

export default applyPostStickyCheck( MyComponent );

Copy link
Contributor

Choose a reason for hiding this comment

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

This could obfuscate component render logic, though. Right now most HoCs only deal with prop injection and side effects (like click-outside), not affecting when things should be rendered. Which is why the current way seems appropriate. 👌

@codecov
Copy link

codecov bot commented Oct 24, 2017

Codecov Report

Merging #3114 into master will decrease coverage by 0.06%.
The diff coverage is 26.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3114      +/-   ##
==========================================
- Coverage   32.23%   32.17%   -0.07%     
==========================================
  Files         216      218       +2     
  Lines        6151     6160       +9     
  Branches     1081     1083       +2     
==========================================
- Hits         1983     1982       -1     
- Misses       3517     3525       +8     
- Partials      651      653       +2
Impacted Files Coverage Δ
editor/sidebar/post-sticky/index.js 0% <0%> (-54.55%) ⬇️
editor/post-sticky/index.js 0% <0%> (ø)
editor/post-sticky/check.js 71.42% <71.42%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 653e555...721594f. Read the comment docs.

@youknowriad youknowriad requested review from mcsf and gziolo October 26, 2017 09:59
Copy link
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

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

Nice and clean.

@youknowriad youknowriad merged commit c9a05b2 into master Oct 27, 2017
@youknowriad youknowriad deleted the update/post-sticky-component branch October 27, 2017 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants