Uh oh!
There was an error while loading. Please reload this page.
feat(webkit): give Menu rows an arrow control split by href, and make Back a button - #952
Open
gabriel-lisboa-azion wants to merge 4 commits into
Open
feat(webkit): give Menu rows an arrow control split by href, and make Back a button#952gabriel-lisboa-azion wants to merge 4 commits into
gabriel-lisboa-azion wants to merge 4 commits into
Conversation
gabriel-lisboa-azionforce-pushed
the
feat/menu-split-trigger-and-back-button
branch
from
August 31, 2026 22:11
3a66975 to
c544773Compare… and make Back a button A row that owns children is a destination as well as a container, so it is now two controls: a label that references wherever the row points, and a trailing transparent IconButton that reveals its children. Condensed rows expand in place, drill rows push a level. Reaching the children no longer costs the reader the destination, and each control has its own surface, focus ring and hit area. aria-expanded and aria-controls move to the arrow, the control that expands the children. The arrow keys still work from the label, and focus returns to the arrow when a drill level is popped. Menu.Back is a compact button rather than another full-width row, so the one control that leaves a level does not read as one of the rows inside it, and its text names where it lands rather than the level the reader is already in. IconButton now forwards $attrs to its root, per .claude/rules/root-element.md. It set inheritAttrs: false and never spread them, so every consumer attribute was silently swallowed -- which is why the arrow could not carry aria-expanded.
gabriel-lisboa-azionforce-pushed
the
feat/menu-split-trigger-and-back-button
branch
from
August 31, 2026 22:26
c544773 to
df6a6d1Compare… arrow on every row A row that owns children now always renders its arrow as a transparent IconButton -- that arrow IS the affordance saying the row owns children, so it must not change shape according to whether the row also has a destination. What `href` decides is what the LABEL beside it does, and therefore how much of the row reveals the children: - with an `href`, the label is an <a> to that destination and the arrow alone reveals the children, so reaching them does not cost the reader the destination; - without one, the label reveals them too, so the whole row does. The unconditional split was wrong on a phone, measured in the sample's docs sheet: the label was 320x32 and navigated away AND dismissed the sheet, while the only control that expanded was 28x28 -- 7% of the row's area, under the 44px touch guideline. Tapping a chevron row to look inside it therefore threw the reader onto a page and closed the menu. Every one of the 28 container rows in that tree carries no href (a container is not a destination there), so all 28 now reveal from their full width. aria-expanded / aria-controls sit on whichever control expands the children and on exactly one of them: the arrow when the label is a link, the label otherwise -- in which case the arrow becomes a redundant pointer target (tabindex="-1" + aria-hidden), so it adds neither a second tab stop nor a second announced control. Focus after a pop returns to whichever control pushed. A disabled row drops its `href` outright, since an anchor with no destination is not a link.
gabriel-lisboa-azion added a commit
that referenced
this pull request
Sep 1, 2026
… Back button Merges feat/menu-split-trigger-and-back-button, which carries main up to 11d2bd5 (9 commits) plus the two of the PR. Resolutions, none of them mechanical: - packages/theme/src/tokens/semantic/texts.data.js — hand-merged. The demo side is a superset inside the conflict hunk (its own text-amount-* ladder, textWrapStyle on the heading tokens) and already carried main's regular-weight change there, so the hunk took ours; text-body-prose-md sat OUTSIDE the hunk, where only main had changed it light -> normal, so the auto-merge kept main's value. Canaries: 34 text-* entries (demo 34 / incoming 31), 0 fontWeight.light, 3 text-amount-*, 16 textWrapStyle. - packages/theme/dist/v4/globals.{css,scss} — generated; rebuilt from the merged tokens rather than hand-merged. The rebuilt CSS keeps the demo-only utilities and leaves exactly 3 font-weight:300 declarations, all @font-face for the Light files, matching the incoming side's count. - packages/webkit/scripts/authoring-baseline.json — generated; regenerated on the merged tree (224 entries, the demo's own count). - .specs/doc-prose.md — only checksum and last_updated conflicted; the body auto-merged, so neither side's checksum was valid and it was recomputed on the merged body. - packages/webkit/catalog.json — regenerated on the merged tree (258 exports @ 4.4.0). apps/webkit-sample never entered the merge: it does not exist at the base or on the incoming side, so its 579 files pass through untouched.
gabriel-lisboa-azion added a commit
that referenced
this pull request
Sep 1, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
IconButtonarrow — the affordance that says the row owns children.hrefdecides what the label beside it does: with one, the label is a real<a>and the arrow alone reveals the children; without one, the label reveals them too, so the whole row does.aria-expanded/aria-controlssit on whichever control expands the children and on exactly one of them; when the label owns them the arrow becomes a redundant pointer target (tabindex="-1"+aria-hidden), adding neither a second tab stop nor a second announced control.Menu.Backis a compact button instead of a full-width row with a centred label, and its text names where it lands (Back,Back to Settings, orlabel) rather than the level the reader is already in.How to test
pnpm webkit:test— 2216 pass, including the menu suite's axe checks.pnpm storybook:dev→ Components / Navigation / Menu / Types. Neither row has anhref, so each reveals its children from the whole row while still showing the arrow. Tab: one stop per row.href="/anything"to aSubTriggerin that story: the label becomes a real link (middle-click opens a tab), the arrow alone reveals, andaria-expandedmoves from the label to the arrow.Backappears as a compact left-aligned button readingBack. Push a second level:Back to <parent>.Escape/ArrowLeft/ clicking Back pops and returns focus to whichever control pushed.IconButton's filled disabled box would otherwise make the arrow the brightest thing on a row whose label had dimmed away).Notes
hrefbehaves as it did before this PR. The trigger's root is now always adivholding its controls, so a consumer whose tests or CSS target the trigger root as the interactive element should point at…__control/…__reference/…__arrow.SubTrigger.href(default''). In data-driven mode it comes fromMenuNode.href, which already existed — so the behaviour falls out of the tree with no new concept for consumers, and it followsroot-element.md(the element switches on a data prop, never anasstring).IconButtonnow forwards$attrsto its root, per.claude/rules/root-element.md. It declaredinheritAttrs: falseand never spread them, so every consumer attribute was silently swallowed — that is why the arrow could not carryaria-expanded.classanddata-testidstay excluded (already applied explicitly).