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

Toolbar Button: Handle click instead of mousedown #1201

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

seanpdoyle
Copy link
Contributor

Revert c4b9d5b

Requiring mouse events to trigger <button> element event listeners poses accessibility issues for keyboard users.

The context in the original commit (c4b9d5b) is fairly light, and there doesn't appear to be a link to a GitHub pull request with more information. The provided context is:

Prevents flickering of the placeholder text when clicking block
formatting buttons on an empty document.

Since the commit is from 2014, there is hope that the underlying flicker might have been resolved through the natural course of browser improvements and device enhancements.

The benefit of responding to keyboard events when the <button> element has focus outweighs the potential downsides.

Revert [c4b9d5b][]

Requiring mouse events to trigger `<button>` element event listeners
poses accessibility issues for keyboard users.

The context in the original commit ([c4b9d5b][]) is fairly light, and
there doesn't appear to be a link to a GitHub pull request with more
information. The provided context is:

> Prevents flickering of the placeholder text when clicking block
> formatting buttons on an empty document.

Since the commit is from 2014, there is hope that the underlying flicker
might have been resolved through the natural course of browser
improvements and device enhancements.

The benefit of responding to keyboard events when the `<button>` element
has focus outweighs the potential downsides.

[c4b9d5b]: basecamp@c4b9d5b
@seanpdoyle
Copy link
Contributor Author

@brunoprietog are you available to review this change? I think there's a potential to resolve a long-term accessibility issue.

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.

1 participant