Skip to content

fix(#3605): use :focus-visible across interactive components#3911

Open
twjeffery wants to merge 1 commit into
devfrom
tom/focus-visible
Open

fix(#3605): use :focus-visible across interactive components#3911
twjeffery wants to merge 1 commit into
devfrom
tom/focus-visible

Conversation

@twjeffery
Copy link
Copy Markdown
Collaborator

@twjeffery twjeffery commented May 5, 2026

Fixes #3605

Replaces #3861 (auto-closed when the branch was renamed.)

Before

Mouse click on interactive components left the focus ring visible for mouse users. In some cases the ring rendered without the designed gap/offset because :focus fired on the mouse path alongside :focus-visible.

After

Mouse click no longer shows the focus ring. Keyboard Tab and arrow-key navigation show the ring with the correct gap/offset. V1 and V2 paths both behave correctly.

Components updated

  • Button.svelte
  • Chip.svelte (V1, deprecated)
  • Checkbox.svelte (V1)
  • DataGrid.svelte
  • Details.svelte
  • FilterChip.svelte
  • Link.svelte
  • MicrositeHeader.svelte
  • Notification.svelte
  • TextArea.svelte
  • Tooltip.svelte
  • assets/css/reset.css

Make sure that you've checked the boxes below before you submit the PR

  • I have read and followed the setup steps
  • I have created necessary unit tests
  • I have tested the functionality in both React and Angular.

Steps needed to test

  1. Check out this branch and run npm run serve:prs:react (or :angular or :web).
  2. Navigate to /bugs/3605 (React/Angular) or /issues/3605 (Web).
  3. For each section, press Tab through the interactive elements. A focus ring should appear with the correct gap/offset.
  4. Click the same elements with a mouse. No focus ring should remain after the click. Holding mouse down (:active) should still show the pressed/focused state on buttons.
  5. For DataGrid: use arrow keys inside both the table example and the cards example. Focus ring appears on keyboard nav, not on mouse click. Cells containing interactive components (checkbox, button, menu-button) do not stack our ring on top of the component's native ring.
Screen.Recording.2026-04-21.at.7.15.30.PM.mov
Screen.Recording.2026-04-21.at.7.17.35.PM.mov

@twjeffery twjeffery requested a review from vanessatran-ddi May 5, 2026 20:16
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://GovAlta.github.io/ui-components/pr-preview-angular/pr-3911/

Built to branch gh-pages at 2026-05-06 16:59 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

Preview links

Target URL
Docs https://govalta.github.io/ui-components/pr-preview/pr-3911/
React playground https://govalta.github.io/ui-components/pr-preview-react/pr-3911/
Angular playground https://govalta.github.io/ui-components/pr-preview-angular/pr-3911/

Built from commit eb22356. Previews are removed automatically when this PR closes.

@twjeffery twjeffery closed this May 5, 2026
@twjeffery twjeffery reopened this May 5, 2026
@twjeffery twjeffery force-pushed the tom/focus-visible branch from 891a4fc to 3053966 Compare May 5, 2026 22:13
@twjeffery twjeffery marked this pull request as draft May 6, 2026 15:40
@twjeffery twjeffery force-pushed the tom/focus-visible branch from 3053966 to eb22356 Compare May 6, 2026 16:56
@twjeffery twjeffery marked this pull request as ready for review May 6, 2026 17:04
Copy link
Copy Markdown
Collaborator

@vanessatran-ddi vanessatran-ddi left a comment

Choose a reason for hiding this comment

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

Looks good!

<goa-text as="h2" mt="0" mb="xs">7. Link — <code>::slotted(a:focus-visible)</code></goa-text>
<goa-text mb="s">
Ring is now on the slotted anchor directly (reliable across shadow DOM).
<strong>Trade-off:</strong> ring wraps the anchor text only, not the icons.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You point this out here, and it is true, focus on a link with trailing or leading icons, the focus border only goes around the text, not the icons. Do we want that?

If we don't, a possible solution could be to use the focusin event for the Link, and set a variable if it's triggered, then set a class on the outer container if that variable is true. And finally set the border based on that class.

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.

Use focus-visible consistently across all interactive components

3 participants