Repository navigation
fix: Icons motion perf issue - #5161
Conversation
| 'aria-disabled': disabled || undefined, | ||
| // The hover region is the whole card (see the root element); focusing | ||
| // the inner control also plays the motion. | ||
| 'data-awsui-motion-trigger': 'focus', |
There was a problem hiding this comment.
It's dead now. Only the old root :has() looked for it, and the root now uses :focus-within. The other focus rule needs the icon inside the focused element, which never happens here, since the button doesn't contain the icon
| data-awsui-motion-trigger="hover" | ||
| // Keyboard focus on the nested header or overlay button, which does not contain the icon, | ||
| // also plays the motion. | ||
| data-awsui-motion-trigger="hover delegated-focus" |
There was a problem hiding this comment.
hover plays the motion while the pointer is over the card. delegated-focus plays it when keyboard focus is inside the card, because the focused button doesn't contain the icon
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5161 +/- ##
=======================================
Coverage 97.72% 97.73%
=======================================
Files 995 996 +1
Lines 32129 32152 +23
Branches 11851 11859 +8
=======================================
+ Hits 31399 31424 +25
- Misses 684 722 +38
+ Partials 46 6 -40 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟢 Approval recommended
The focused selector changes match existing focus and icon-rendering contracts, with regression coverage and no identified blocking issues.
No open issues 🎉
What changed in this PR
Removes costly :has() selectors from icon motion CSS while preserving keyboard-focus motion, including action cards.
Changes:
- Uses direct focus selectors and delegated focus for action cards.
- Simplifies cancellation of whole-icon motion for icons with part-specific animations.
- Adds regression assertions for selectors, icon parts, and action-card wiring.
| File | Description |
|---|---|
| src/icon/hover-motion.scss | Replaces :has() selectors and simplifies motion cancellation. |
| src/icon/__tests__/icon-hover-motion.test.ts | Checks focus selectors, absence of :has(), and animated icon parts. |
| src/action-card/internal.tsx | Enables delegated-focus motion on the card root. |
| src/action-card/__tests__/action-card.test.tsx | Verifies delegated-focus wiring for both button layouts. |
Review effort: Balanced
|
LGTM. One thing: we use |
Description
Removes
:has()from the one-theme icon hover-motion CSS. Its selectors were evaluated against every motion trigger on each style recalc and DOM insert, which slowed down pages with many triggers or frequent DOM updates. Focus motion now uses plain :focus-visibleselectors. Action card, where the focused button does not contain the icon, uses a new delegated-focus trigger: :focus-within, gated on thedata-awsui-focus-visiblebody attribute so only keyboard focus plays it. The cancel rule for icons with only part motion no longer checks the parts with :has(). Tests assert that the compiled motion CSS has no:has().Related links, issue #, if available: n/a
How has this been tested?
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.