chore: Use CSS-native :focus-visible to track visible focus state (batch 3) #3607
Conversation
| ) | ||
| ); | ||
| } | ||
| // stylelint-disable-next-line selector-combinator-disallowed-list |
There was a problem hiding this comment.
This utility is used four times in the entirety of the package (and all @cloudscape-design packages). You'll see the other three in this PR too. I don't think it's worth pulling in component toolkit just for a tiny little mixin that isn't even particularly Cloudscape-specific to begin with.
| @@ -14,12 +14,22 @@ import { PointerEventMock } from '../../../../../lib/components/internal/utils/p | |||
| import styles from '../../../../../lib/components/internal/components/drag-handle-wrapper/styles.css.js'; | |||
| import tooltipStyles from '../../../../../lib/components/internal/components/tooltip/styles.css.js'; | |||
|
|
|||
| function mockFocusVisible(value: boolean) { | |||
There was a problem hiding this comment.
Some jerk (me) wrote in a dependency on the presence of data-awsui-focus-visible="true" as part of the component logic. Rewrote that to just check CSS instead (using document.body.matches()). And we can mock that just inside this file.
| document.body.dataset.awsuiFocusVisible = 'true'; | ||
| handle.focus(); | ||
| fireEvent.pointerDown(handle); | ||
| fireEvent.pointerUp(handle); |
There was a problem hiding this comment.
The buttons show up either on click (pointer down + no drag + pointer up) or on keyboard focus. The pointer method of doing this is easier and more straightforward to implement than faking keyboard focus.
avinashbot
force-pushed
the
focus-visible-final
branch
from
be35560 to
bf57dd9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3607 +/- ##
=======================================
Coverage 96.59% 96.59%
=======================================
Files 807 807
Lines 23582 23582
Branches 8261 8271 +10
=======================================
Hits 22780 22780
+ Misses 795 748 -47
- Partials 7 54 +47 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Reducing the potential blast radius of a visual change that affects tons of components. I'll be removing the internal JS-based
focus-visiblesolution in individual batches. This one contains all the weird interesting changes I've had to make, but it's the last one before I removeuseFocusVisible()from the components package entirely.See #3585 for the big original PR.
Related links, issue #, if available: AWSUI-60845
How has this been tested?
Tested the whole big PR in my dev pipeline, and the individual changes just manually.
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.