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

React Native - Heading toolbar #9570

Merged
merged 9 commits into from Sep 8, 2018

Conversation

Projects
None yet
4 participants
@SergioEstevao
Contributor

SergioEstevao commented Sep 3, 2018

Description

This PR adds the toolbar icons to change the type of Heading (h1,h2,h3,h4) on the Heading block.

screen shot 2018-09-03 at 12 44 37

Refs: wordpress-mobile/gutenberg-mobile#133

How has this been tested?

Use this PR on the RN project to test it.

Screenshots

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.

SergioEstevao added some commits Aug 24, 2018

@SergioEstevao SergioEstevao added this to the 3.9 milestone Sep 3, 2018

@SergioEstevao SergioEstevao requested review from koke and hypest Sep 3, 2018

@SergioEstevao

This comment has been minimized.

Show comment
Hide comment
@SergioEstevao

SergioEstevao Sep 3, 2018

Contributor

@hypest ready for another look.

Contributor

SergioEstevao commented Sep 3, 2018

@hypest ready for another look.

@koke

This comment has been minimized.

Show comment
Hide comment
@koke

koke Sep 4, 2018

Contributor

Instead of just extracting the createLevelControl method, what do you think about extracting all the heading level selectors to a HeadingToolbar component?

<HeadingToolbar
	minLevel="2"
	maxLevel="5"
	level={ level }
	onChange={ ( newLevel ) => setAttributes( { level: newLevel } ) }
/>
Contributor

koke commented Sep 4, 2018

Instead of just extracting the createLevelControl method, what do you think about extracting all the heading level selectors to a HeadingToolbar component?

<HeadingToolbar
	minLevel="2"
	maxLevel="5"
	level={ level }
	onChange={ ( newLevel ) => setAttributes( { level: newLevel } ) }
/>

SergioEstevao added some commits Sep 4, 2018

@SergioEstevao SergioEstevao requested a review from gziolo Sep 4, 2018

@SergioEstevao

This comment has been minimized.

Show comment
Hide comment
@SergioEstevao

SergioEstevao Sep 4, 2018

Contributor

@koke followed your suggestion :).

@gziolo do you mind to give this a look?

Contributor

SergioEstevao commented Sep 4, 2018

@koke followed your suggestion :).

@gziolo do you mind to give this a look?

@hypest

Looks good to me @SergioEstevao !

BTW, I'm still trying to wrap my head aroung the "parse" based trick but since that's used in the Paragraph block too, I don't consider it a blocker for this PR.

Also, I like the code changes to unify/extract the common code between web/mobile for the toolbar but, I guess we better have someone from the web side give it the green light as well. You already pinged Greg so, I'll leave it at that.

@koke

koke approved these changes Sep 5, 2018

import { Toolbar } from '@wordpress/components';
class HeadingToolbar extends Component {
createLevelControl( targetLevel, selectedLevel, onChange ) {

This comment has been minimized.

@koke

koke Sep 5, 2018

Contributor

Minor improvement, but you can read { selectedLevel, onChange } directly from this.props instead of passing it as arguments.

@koke

koke Sep 5, 2018

Contributor

Minor improvement, but you can read { selectedLevel, onChange } directly from this.props instead of passing it as arguments.

@SergioEstevao SergioEstevao merged commit 410f78a into master Sep 8, 2018

2 checks passed

codecov/project 50.32% (-0.06%) compared to c9668e9
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@SergioEstevao SergioEstevao deleted the rnmobile/heading_toolbar branch Sep 10, 2018

@phpbits

This comment has been minimized.

Show comment
Hide comment
@phpbits

phpbits Sep 15, 2018

@SergioEstevao How can I use HeadingToolbar? Is it available on wp.components? Thanks!

phpbits commented Sep 15, 2018

@SergioEstevao How can I use HeadingToolbar? Is it available on wp.components? Thanks!

@SergioEstevao

This comment has been minimized.

Show comment
Hide comment
@SergioEstevao

SergioEstevao Sep 16, 2018

Contributor
Contributor

SergioEstevao commented Sep 16, 2018

@phpbits

This comment has been minimized.

Show comment
Hide comment
@phpbits

phpbits Sep 16, 2018

@SergioEstevao Thanks! Just copied the code and created and my own component ;)

phpbits commented Sep 16, 2018

@SergioEstevao Thanks! Just copied the code and created and my own component ;)

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