Skip to content

Conversation

@TrevorBurnham
Copy link
Contributor

All targeted browsers now support :focus-visible: https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-visible#browser_compatibility

This commit removes the use of the shim defined in @cloudscape-design/component-toolkit. Once this change is merged, the shim can be removed.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@TrevorBurnham TrevorBurnham requested a review from a team as a code owner August 24, 2025 23:35
@TrevorBurnham TrevorBurnham requested review from taheramr and removed request for a team August 24, 2025 23:35
@just-boris just-boris requested review from avinashbot and removed request for taheramr August 25, 2025 09:43
All targeted browsers now support :focus-visible. This commit removes
the use of the shim defined in @cloudscape-design/component-toolkit.
Once this change is merged, the shim can be removed.
@TrevorBurnham
Copy link
Contributor Author

I've updated this branch to take a more careful approach. Notably, the shim had greater specificity (two class selectors instead of one).

In one case I've used a double-specifier (:focus-visible:focus-visible) to ensure that the styles take precedence as intended. In other cases I think it should be fine to just use :focus-visible, but unexpected style interactions are always possible. If you really want to err on the side of caution, you could use :focus-visible:focus-visible consistently to preserve the existing specificity.

@avinashbot
Copy link
Member

Thanks for opening the PR! Running screenshot tests on the PR to confirm everything checks out visually.

I already made a similar attempt earlier (#3598, #3615, #3599, #3607), but it led to enough visual discrepancies that it got me to step back from it for a bit. For context, it mostly concerns the fact that the JS utility is hidden by default until a keyboard event is detected, but the native :focus-visible is visible by default until a mouse event is detected, which is especially apparent on Safari, and occasionally on other browsers after certain programmatic focus moves (couldn't nail down the logic browsers use to link an event handler to a focus move, especially with React rendering in the way). Let me confirm if this is still the case in your PR and check back.

@TrevorBurnham
Copy link
Contributor Author

@avinashbot Ah, sorry that I missed those earlier PRs!

The behavior in this PR would be the same: :focus-visible by its nature delegates to the browser's own heuristics. That's good from an a11y standpoint, but I agree that it can be jarring if it means that an outline appears because of a programmatic focus change triggered by a mouse event.

Can you tell me exactly what behavior you're seeing in Safari? Like, does the close button on a modal get visible focus after that modal was opened via mouse 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.

2 participants