GridList: keep a row highlighted while a control inside it has a menu… - #40
Merged
Merged
Conversation
… open A Popover renders a `position: fixed; inset: 0` underlay whenever it is not `isNonModal` — the default, and what MenuList uses. The pointer ends up over the underlay rather than the row, so the browser fires pointerleave and the row's hover grey drops the instant the menu opens. Chakra's Menu was a Popper with useOutsideClick and no underlay, so this only appears at an app's kill-switch, with nothing in the diff to explain it (playbook gotcha #43). `data-hovered` is no help — the hover condition already covers it and it is downstream of the same pointerleave. The row needs a different question: does it hold an open overlay? A trigger carries `aria-expanded` from useOverlayTrigger, so `:has()` answers it on the row with no state plumbing between the list and whatever a call site renders inside it. Repeated inside the selected rule rather than trusting emit order, since `:has()` takes its argument's specificity and ties with `[data-selected]`. Found via classroom's class roster, which needs the same rule locally: its flat `bg` override silences the recipe's backgrounds (gotcha #21), so it cannot inherit this one. Also in the playbook, from the same investigation: - Gotcha #37 gains the other shape of its failure: a component that calls its slot recipe with no arguments at all. GridList, ListBox and Menu all do, so an app preset that adds a variant group to them gets nothing applied. Latent — those three have no variants yet — but they are not extensible today. - A decisions-due entry for `colorPalette` in place of per-component colour tokens, to be settled after the brand ramp review. The gridList item's greys are already gray.50 and gray.100, the two stops a palette swap would use, so the roster is a small first pilot. Includes what the ramp review has to decide first (completeness, not just correctness), and why the button.* tokens stay: the brand idiom is palette-shaped at brand.500/600/700, but black plus blackAlpha.800/700 spans two token groups and no palette produces it.
Deploying ui with
|
| Latest commit: |
6f52460
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b086e9dc.ui-2wg.pages.dev |
| Branch Preview URL: | https://gridlist-open-menu-row-highl.ui-2wg.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
… open
A Popover renders a
position: fixed; inset: 0underlay whenever it is notisNonModal— the default, and what MenuList uses. The pointer ends up over the underlay rather than the row, so the browser fires pointerleave and the row's hover grey drops the instant the menu opens. Chakra's Menu was a Popper with useOutsideClick and no underlay, so this only appears at an app's kill-switch, with nothing in the diff to explain it (playbook gotcha #43).data-hoveredis no help — the hover condition already covers it and it is downstream of the same pointerleave. The row needs a different question: does it hold an open overlay? A trigger carriesaria-expandedfrom useOverlayTrigger, so:has()answers it on the row with no state plumbing between the list and whatever a call site renders inside it. Repeated inside the selected rule rather than trusting emit order, since:has()takes its argument's specificity and ties with[data-selected].Found via classroom's class roster, which needs the same rule locally: its flat
bgoverride silences the recipe's backgrounds (gotcha 21), so it cannot inherit this one.Also in the playbook, from the same investigation:
Gotcha 37 gains the other shape of its failure: a component that calls its slot recipe with no arguments at all. GridList, ListBox and Menu all do, so an app preset that adds a variant group to them gets nothing applied. Latent — those three have no variants yet — but they are not extensible today.
A decisions-due entry for
colorPalettein place of per-component colour tokens, to be settled after the brand ramp review. The gridList item's greys are already gray.50 and gray.100, the two stops a palette swap would use, so the roster is a small first pilot. Includes what the ramp review has to decide first (completeness, not just correctness), and why the button.* tokens stay: the brand idiom is palette-shaped at brand.500/600/700, but black plus blackAlpha.800/700 spans two token groups and no palette produces it.