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

Move HeadingToolbar controls to block toolbar #1352

Merged
merged 7 commits into from
Feb 19, 2020

Conversation

richtabor
Copy link
Contributor

@richtabor richtabor commented Feb 12, 2020

Description

This PR moves the HeadingToolbar component that we use in our blocks which manipulate headings within InnerBlocks (Services, Features). Instead of utilizing the Sidebar Settings, the controls are moved to the block toolbar.

There is already discussion around removing the sidebar control within the core Heading block. This aligns with the pending experience.

Screenshots

Before:
Screen Shot 2020-02-12 at 2 27 20 PM

After:
Screen Shot 2020-02-12 at 2 26 36 PM

Types of changes

New feature (non-breaking change which adds functionality), while removing an older feature (including the sidebar control).

How has this been tested?

Checklist:

  • My code is tested
  • My code follows accessibility standards
  • My code has proper inline documentation
  • I've included any necessary tests
  • I've added proper labels to this pull request

@richtabor richtabor added [Type] Enhancement Something new that adds functionality [Priority] Low This issue/pull request is not immediate [Status] Needs Review Tracking pull requests that need another set of eyes labels Feb 12, 2020
@richtabor richtabor self-assigned this Feb 12, 2020
@richtabor
Copy link
Contributor Author

Looks like the following test will need to be updated. I'd appreciate any extra hands for this! :)

it( 'Updates the inner core/heading blocks when the "Heading Level" control is changed.', function() {
		helpers.addCoBlocksBlockToPage( true, 'features' );

		helpers.openSettingsPanel( /features settings/i );

		cy.get( '.components-coblocks-heading-toolbar [aria-label="Heading 6"]' ).click( { force: true } );
		cy.get( '[data-type="coblocks/feature"] [data-type="core/heading"] h6' ).should( 'have.length', 2 );

		helpers.savePage();
		helpers.checkForBlockErrors( 'features' );
	} );

@richtabor richtabor added the [Type] Needs Tests This issue/pull request requires proper tests implemented label Feb 12, 2020
@richtabor
Copy link
Contributor Author

Gutenberg is implementing this here: WordPress/gutenberg#20246

@cypress
Copy link

cypress bot commented Feb 17, 2020



Test summary

10 0 0 0


Run details

Project CoBlocks
Status Passed
Commit 27e84e9
Started Feb 18, 2020 9:24 PM
Ended Feb 18, 2020 9:25 PM
Duration 01:12 💡
OS Linux Debian - 10.2
Browser Chrome 80

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Member

@AnthonyLedesma AnthonyLedesma left a comment

Choose a reason for hiding this comment

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

Tested and working well on my end. Approved.

@richtabor richtabor merged commit 7e18730 into master Feb 19, 2020
@richtabor richtabor deleted the try/heading-toolbar-controls branch February 19, 2020 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Priority] Low This issue/pull request is not immediate [Status] Needs Review Tracking pull requests that need another set of eyes [Type] Enhancement Something new that adds functionality [Type] Needs Tests This issue/pull request requires proper tests implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants