Skip to content

Commit

Permalink
Toolbar Button: Handle click instead of mousedown
Browse files Browse the repository at this point in the history
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]: c4b9d5b
  • Loading branch information
seanpdoyle committed Dec 3, 2024
1 parent d34ebea commit a70f8b1
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/test/test_helpers/toolbar_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { delay, nextFrame } from "./timing_helpers"
export const clickToolbarButton = async (selector) => {
selectionChangeObserver.update()
const button = getToolbarButton(selector)
triggerEvent(button, "mousedown")
triggerEvent(button, "click")
await delay(5)
}

Expand Down
4 changes: 2 additions & 2 deletions src/trix/controllers/toolbar_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ export default class ToolbarController extends BasicObject {
this.actions = {}
this.resetDialogInputs()

handleEvent("mousedown", {
handleEvent("click", {
onElement: this.element,
matchingSelector: actionButtonSelector,
withCallback: this.didClickActionButton,
})
handleEvent("mousedown", {
handleEvent("click", {
onElement: this.element,
matchingSelector: attributeButtonSelector,
withCallback: this.didClickAttributeButton,
Expand Down

0 comments on commit a70f8b1

Please sign in to comment.