Skip to content

Conversation

@koke
Copy link
Contributor

@koke koke commented Mar 9, 2019

Description

This PR adds a HorizontalRule wrapper to the hr HTML tag, using react-native-hr on the native implementation, to allow for a cross platform separator block.

How has this been tested?

Tested on mobile:

  • Create a new post
  • Insert a new block, verify that separator is available
  • Insert a separator, verify it renders correctly

Tested on web:

  • Create a new post
  • Insert a separator block, verify it renders correctly, block styles still work
  • Preview the post and verify it renders correctly

Screenshots

Simulator Screen Shot - iPhone Xs - 2019-04-17 at 10 16 46

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@koke koke force-pushed the rnmobile/separator-block branch from 830e289 to 7336885 Compare April 12, 2019 11:11
@koke koke changed the base branch from rnmobile/develop to master April 12, 2019 11:14
@koke
Copy link
Contributor Author

koke commented Apr 12, 2019

@gziolo thanks for the suggestions. I'll do some more testing next week and get this ready to review and merge

@koke
Copy link
Contributor Author

koke commented Apr 17, 2019

I think this is ready to review. I tested on mobile and web and everything looks ok. I considered adding a README.md to the new primitive, but I couldn't come up with anything good, since it's mostly a wrapper for <hr> with no specific props. If you have any suggestions around documentation, I'd be happy to include that.

@koke koke changed the title [WIP] Separator block / Hr component HorizontalRule component / Cross-platform separator block Apr 17, 2019
@koke koke changed the title HorizontalRule component / Cross-platform separator block HorizontalRule component for cross-platform separator block Apr 17, 2019
@gziolo
Copy link
Member

gziolo commented Apr 17, 2019

I considered adding a README.md to the new primitive, but I couldn't come up with anything good, since it's mostly a wrapper for <hr> with no specific props. If you have any suggestions around documentation, I'd be happy to include that.

I think this is totally expected to be that simple :)
We need to have very basic docs to ensure they are picked up by Gutenberg Handbook:
https://wordpress.org/gutenberg/handbook/designers-developers/developers/components/

It will be exposed in the sidebar.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Let's add docs and get it merged 👍

@koke koke requested review from mkaz and oandregal as code owners April 17, 2019 08:47
@gziolo gziolo added [Package] Components /packages/components [Type] Bug An existing feature does not function as intended [Type] Task Issues or PRs that have been broken down into an individual action to take [Feature] UI Components Impacts or related to the UI component system and removed [Type] Bug An existing feature does not function as intended labels Apr 17, 2019
@koke
Copy link
Contributor Author

koke commented Apr 17, 2019

Added a README and entry on the CHANGELOG, let me know if that looks good 😁

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I proposed expanded description based on MDN docs :)

Thanks for working on it 👍

"parent": "components"
},
{
"title": "HorizontalRule",
Copy link
Member

Choose a reason for hiding this comment

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

Automation works :)

Copy link
Contributor Author

@koke koke Apr 17, 2019

Choose a reason for hiding this comment

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

Kind of 😁
I think this was generated by the precommit hook, but not included in the commit that added the README. I only noticed it because I did a second commit with the Usage section

@gziolo gziolo added this to the 5.6 (Gutenberg) milestone Apr 17, 2019
@koke koke merged commit 71e0d1b into master Apr 17, 2019
@koke koke deleted the rnmobile/separator-block branch April 17, 2019 10:45
@gziolo gziolo mentioned this pull request Apr 18, 2019
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components [Type] Task Issues or PRs that have been broken down into an individual action to take

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants