Repository navigation
Conversation
…t the card too architecture:interaction-modality INV3: one focus move has one owning indicator. Both cards drew their ring with focusWithin, so a button in their children (which the docs allow: "Nested interactive elements work independently") drew its own ring and the card drew a second around it. Each card now uses focusWithinOwner and marks its own link, button or checkbox with FOCUS_OWNER_ATTRIBUTE. SelectableCard gains a With Nested Button story; FocusRingOwner's browser spec covers both cards at 1280 and 390 px and fails on the previous head. focusRings ignores a visually hidden control's outline (the cards' screen-reader-only primaries), which paints nothing a person sees. Stacked on #8869 (focusWithinOwner).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks ready to approve: a keyboard-focused control inside a ClickableCard or SelectableCard now shows one focus ring instead of two, and the card still rings for its own link, button or checkbox, as the cards' focus contracts and architecture:interaction-modality INV3 require. The new browser cases pass on this branch and catch the double ring without the fix. As a non-blocking follow-up, both component records still describe the double ring as an open implementation gap and can drop those notes once this lands.
[Automated review]
…g-one-owner-cards
Follows #8869: focusWithinOwner and FOCUS_OWNER_ATTRIBUTE now come from the unexported internal/focusOutline.stylex.ts, so nothing new is published.
There was a problem hiding this comment.
This looks ready to approve: a keyboard-focused control inside a ClickableCard or SelectableCard now shows one focus ring instead of two, and the card still rings for its own link, button or checkbox, as the cards' focus contracts and architecture:interaction-modality INV3 require. The new browser cases pass on this branch and fail when the fix is reverted. Two non-blocking follow-ups: both component records still describe the double ring as an open implementation gap, and the spec's @pr-a11y scope line could also route on packages/core/src/internal/focusOutline.stylex.ts, where the owner ring now lives.
[Automated review]
| @@ -1,16 +1,19 @@ | |||
| // Copyright (c) Meta Platforms, Inc. and affiliates. | |||
| // @pr-a11y scope ^packages/core/src/Item/ ^packages/core/src/Token/ ^packages/core/src/utils/focusOutline ^apps/storybook/stories/(Item|Token) | |||
| // @pr-a11y scope ^packages/core/src/(Item|Token|ClickableCard|SelectableCard)/ ^packages/core/src/utils/focusOutline ^apps/storybook/stories/(Item|Token|ClickableCard|SelectableCard) | |||
There was a problem hiding this comment.
Non-blocking: the owner ring these card cases exercise now lives in packages/core/src/internal/focusOutline.stylex.ts, which none of these tokens matches, so a pull request that edits only that file will not select this spec. Adding ^packages/core/src/internal/focusOutline would route it.
…g-one-owner-cards
…g-one-owner-cards; the cards spread the focus-owner owner module's ring and props
…g-one-owner-cards
There was a problem hiding this comment.
This looks ready to approve: a keyboard-focused control inside a ClickableCard or SelectableCard now shows one focus ring instead of two, and the card still rings for its own link, button or checkbox, as the cards' focus contracts and architecture:interaction-modality INV3 require. The new browser cases fail on the unfixed components and pass on this branch. Non-blocking follow-ups:
- Both component records still describe the double ring as an open implementation gap.
- The spec's
@pr-a11y scopeline could also route onpackages/core/src/privateAttributes/focusOwner.stylex.tsandpackages/core/src/Card/, which the card ring now depends on. focusRings.tsskips any outlined box 1px or narrower even when it is not clipped; the clip checks alone keep every case passing.
[Automated review]
| @@ -1,16 +1,19 @@ | |||
| // Copyright (c) Meta Platforms, Inc. and affiliates. | |||
| // @pr-a11y scope ^packages/core/src/Item/ ^packages/core/src/Token/ ^packages/core/src/utils/focusOutline ^apps/storybook/stories/(Item|Token) | |||
| // @pr-a11y scope ^packages/core/src/(Item|Token|ClickableCard|SelectableCard)/ ^packages/core/src/utils/focusOutline ^apps/storybook/stories/(Item|Token|ClickableCard|SelectableCard) | |||
There was a problem hiding this comment.
Non-blocking: the card ring these cases exercise now lives in packages/core/src/privateAttributes/focusOwner.stylex.ts, and its > selector depends on Card rendering the hidden control straight inside its root, but no token here matches either path. Adding ^packages/core/src/(privateAttributes/focusOwner|Card/) would run this spec on a pull request that edits only one of them.
| box.width <= 1 || | ||
| box.height <= 1 || |
There was a problem hiding this comment.
Non-blocking: this skips any outlined box 1px or narrower even when it is not clipped, and an unclipped tiny control's outline still paints, so a real second ring there would go uncounted. The two clip checks below are enough for the cards' hidden controls: with these two lines removed all 12 cases still pass.
Part 2 of 3 for #8850: ClickableCard and SelectableCard. Stacked on #8869 (part 1, which adds
focusWithinOwner); retarget tomainonce it lands.User impact
The card docs say "Nested interactive elements work independently", but a keyboard user who tabs to a button inside a
ClickableCardorSelectableCardsees two rings: the button's, and a second around the whole card. Now the button shows one ring, and the card rings only when its own link, button or checkbox is focused.Problem and solution fit
focusWithin(:has(:focus-visible)), which matches any focused descendant.focusWithinOwnerfromprivateAttributes/focusOwner.stylex.tsand marks its own primary (ClickableCard's screen-reader-only link or button, SelectableCard's checkbox) withfocusOwnerProps. Both primaries are direct children of the card root.Current specification
architecture:interaction-modalityINV3 (current): one focus move has one owning indicator; sibling controls paint their own focus.Evidence
FocusRingOwner.a11y.chromium.spec.ts: gains a ClickableCard case (Core/ClickableCard › Nested Interactive Elements) and a SelectableCard case (the new Core/SelectableCard › With Nested Button), at 1280 and 390 px. On the previous head both show two rings while the inner button is focused; now 8/8 pass.focusRings.ts: the helper now ignores an outline on a visually hidden control (the cards' screen-reader-only primaries), since a person can't see it.Scope
Changeset:
[fix]patch for@astryxdesign/core.