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

Format toolbar: prevent mousedown triggering selection #11902

Merged
merged 1 commit into from Nov 15, 2018

Conversation

Projects
None yet
2 participants
@johngodley
Contributor

johngodley commented Nov 15, 2018

If the format toolbar is positioned over another component that has mousedown handlers it is possible to trigger these handlers by pressing the mouse in the toolbar and moving. This can then cause the toolbar to disappear.

Here you can see that click-dragging in the toolbar selects the underlying blocks, removing the toolbar.

flicker

This PR stops the mousedown event propagating outside of the toolbar, and follows a similar pattern to the <Modal /> and <Popover /> components in #11753

Fixes #11087

How has this been tested?

The problem is demonstrated in #11087 where a cover image block is created in such a way that clicking in the format toolbar and moving the mouse triggers a selection of the underlying blocks.

To test, follow the steps in #11087 and confirm that toolbar remains in place when the mouse button is down over a toolbar button and you then move the mouse.

Types of changes

This should be a non-breaking bug fix. I've only applied the wrapper to the inline toolbar, as I don't think it's necessary elsewhere. It shouldn't affect anything else inside the toolbar.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
Format toolbar: prevent mousedown triggering selection
If the format toolbar has mousedown event handlers in UI beneath the toolbar then clicking + dragging can trigger them. This stops the event propagating outside of the toolbar

@johngodley johngodley requested a review from WordPress/gutenberg-core Nov 15, 2018

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Nov 15, 2018

While the change look good to me, I was not able to reproduce the issue in master.

@youknowriad

I'm approving as I think it's necessary if the toolbar appears outside of the boundaries of the current block (because multiselection don't trigger anymore if we stay inside these boudaries).

@youknowriad youknowriad added this to the 4.4 milestone Nov 15, 2018

@johngodley johngodley merged commit a502e1c into master Nov 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@johngodley johngodley deleted the fix/format-bar-mouse-events branch Nov 15, 2018

@johngodley

This comment has been minimized.

Contributor

johngodley commented Nov 15, 2018

Thanks! Yeah, it's a bit fiddly to reproduce. It still happens on master for me, and only in very specific circumstances, so there might be some missing context to reproduce it.

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this pull request Nov 22, 2018

Format toolbar: prevent mousedown triggering selection (WordPress#11902)
If the format toolbar has mousedown event handlers in UI beneath the toolbar then clicking + dragging can trigger them. This stops the event propagating outside of the toolbar
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment