Skip to content

refactor: extract popover focus management into a controller#11535

Merged
web-padawan merged 1 commit intomainfrom
refactor/popover-focus-controller
Apr 23, 2026
Merged

refactor: extract popover focus management into a controller#11535
web-padawan merged 1 commit intomainfrom
refactor/popover-focus-controller

Conversation

@web-padawan
Copy link
Copy Markdown
Member

Summary

  • Extract Tab / Shift+Tab routing from vaadin-popover.js into a new PopoverFocusController (packages/popover/src/vaadin-popover-focus-controller.js).
  • The controller models a logical tab order — scope focusables with the popover moved from its DOM position to right after its target — which collapses the previous 10 branching scenarios into straightforward next/prev lookups against that list.
  • vaadin-popover.js shrinks by ~230 lines; the focus logic becomes isolated in preparation for the opt-in API for Prevent TAB going into Popover [2 days] #11423.

Behavior is preserved: modal popovers still rely on the overlay's own focus trap, and the controller cooperates with FocusTrapController when the popover lives inside one (e.g. a dialog).

Test plan

  • yarn test --group popover — 322 tests
  • yarn test:it — 412 integration tests (includes dialog-popover.test.js trap-wrap scenarios)
  • yarn lint:js and yarn lint:types

🤖 Generated with Claude Code

Copy link
Copy Markdown
Contributor

@DiegoCardoso DiegoCardoso left a comment

Choose a reason for hiding this comment

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

Should it have a .d.ts file?

Move Tab/Shift+Tab routing and its helpers from vaadin-popover.js into a
new PopoverFocusController. The controller models a "logical" tab order
by moving the popover from its DOM position to right after its target,
which collapses the previous branching cases into straightforward
next/prev lookups against that list.

Behavior is preserved; all existing a11y and dialog-popover integration
tests continue to pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@web-padawan web-padawan force-pushed the refactor/popover-focus-controller branch from d188c66 to 5d35f81 Compare April 23, 2026 07:07
@web-padawan
Copy link
Copy Markdown
Member Author

Should it have a .d.ts file?

Added.

@sonarqubecloud
Copy link
Copy Markdown

@web-padawan web-padawan removed the request for review from sissbruecker April 23, 2026 10:43
@web-padawan web-padawan merged commit 597a99e into main Apr 23, 2026
12 of 13 checks passed
@web-padawan web-padawan deleted the refactor/popover-focus-controller branch April 23, 2026 10:45
@vaadin-bot
Copy link
Copy Markdown
Collaborator

Hi @web-padawan and @web-padawan, when i performed cherry-pick to this commit to 24.10, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 597a99e
error: could not apply 597a99e... refactor: extract popover focus management into a controller (#11535)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@vaadin-bot
Copy link
Copy Markdown
Collaborator

This ticket/PR has been released with Vaadin 25.2.0-alpha4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants