Skip to content

Fix mouse clicks not firing combobox-commit when the input blurs - #112

Open
cpruijsen wants to merge 2 commits into
github:mainfrom
cpruijsen:fix/issue-54
Open

cpruijsen wants to merge 2 commits into
github:mainfrom
cpruijsen:fix/issue-54

Conversation

@cpruijsen

@cpruijsen cpruijsen commented Sep 11, 2026

Copy link
Copy Markdown

Summary

The list listens on mousedown and on click. mousedown commits, and the click that follows it is skipped, so one mouse selection fires one combobox-commit and a blur handler that calls stop() no longer loses it (mousedown runs before blur; stop() removed the old click listener). A click with no preceding mousedown, such as option.click(), still commits, so the keyboard path is unchanged: commit() calls target.click() and the click listener fires the event with the click in detail.event. Non-primary buttons are ignored, matching click.

The guard holds the option the mousedown committed and is cleared by the next click anywhere in the document, so a press the pointer never completed cannot swallow a later click on that option.

For a mouse selection detail.event is now the mousedown event rather than the click. That follows from committing on mousedown.

Fixes #54.

Decision

The library listens on mousedown (the approach in #54) rather than leaving commit on click and adding an interactingWithList flag in the example, as github/auto-complete-element and github/text-expander-element do for this race. Closing the list on input blur is normal. Binding commit to click alone makes every such consumer lose the mouse path; the demo was the reproduction.

Test plan

  • New test: mousedown -> blur/stop() -> click still fires one combobox-commit.
  • New test: a direct option.click() fires one combobox-commit.
  • New test: a mousedown whose click lands elsewhere does not suppress a later option.click().
  • Fails with the old click-only listener; passes with mousedown plus the guarded click.
  • Existing Enter and option-click tests still pass, including <a> options updating location.hash.
  • npm test (eslint, tsc, Karma/Chrome Headless): 27 tests completed.

@cpruijsen
cpruijsen requested a review from a team as a code owner September 11, 2026 07:39

@llastflowers llastflowers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working on this fix! Before we can merge this we need to:

  • Preserve option.click() with a guarded click listener (as mentioned in PR description)
  • Add a direct option.click() regression test
  • Remove the new direct fireCommitEvent(target) call from the keyboard path: Let target.click() reach the click listener as it previously did, preserving the existing event order and detail.event value
  • Ensure the guard resets when a mousedown is not followed by a click

@llastflowers
llastflowers requested a review from a team September 16, 2026 16:43
mousedown commits the option; the click that follows is skipped so one mouse
selection fires one combobox-commit. A click with no preceding mousedown, such
as option.click(), still commits, which is also how Enter and Tab commit again.

The guard holds the committed option and is cleared by the next click anywhere
in the document, so a mousedown the pointer never completed cannot swallow a
later click.
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.

Click by mouse not fire commit

2 participants