Repository navigation
feat(webkit): rework the NavigationMenu panel and fix its dead keyboard model - #943
Merged
Merged
Conversation
…rd model The panel is reworked for the mega-menu shape, and two silent defects found while building it are fixed. Panel treatment: - The group heading is an overline with a hairline under it, not the `Overline` COMPONENT — which painted brand primary at `text-overline-md` and brought its own padding, and was what made the heading shout. The inset is a margin, not padding, so the words start on the entries' icon column while the painted bottom edge stops short of the track on each side; adjacent columns are then separated by a real break instead of the 2px their transparent side borders leave. - A group heading takes an optional `href`, so a section can link to its own page above the entries that lead into parts of it. Following it closes the menu, the same close a `close-on-click` entry performs: the panel must not outlive the navigation it triggered. - `NavigationMenuPopup` takes `kind="contrast"`, which INVERTS the panel rather than lightening it. Over a dark canvas a backdrop cannot darken black and a shadow on black is invisible, and the fill cannot simply be raised: entry copy is `--text-muted` (#808080), which holds 4.66:1 on `--bg-surface-raised` and falls under the 4.5:1 floor by ~#181818. The variant redefines the tokens the panel's parts already paint from, so no sub-component knows the panel inverted. - Entry rhythm comes from the entry's own block padding, with no gap on top of it — setting the vertical spacing from two unrelated numbers is what made the column read as loose. Entry icons move to the system's 16px box, and descriptions to 14px so the pair reads as one two-line block instead of a footnote. Fixes: - The trigger's keyboard model was entirely dead. `@keydown="!isLink ? root.onTriggerKeydown : undefined"` compiles to an inline statement — Vue only treats a bare identifier or member path as a method handler — so the ternary was evaluated per keystroke and its result discarded. Nothing errored: `Escape` never closed the panel and the arrows never roved, while `Enter`/`Space` kept working because a native button turns them into a click. Now a real handler, guarded by two tests that fail against the old form. - Panel measurement took the height at the wrong width. `--popup-width` was measured with `width: auto`, which is "fill the parent" and only reads as the panel's natural width while the positioner is shrink-wrapped; give the positioner a width of its own (a mega-menu on a page column) and every panel measures as that column. It is `fit-content` now, and the height is taken in a second pass at the width the panel is actually laid out at — measuring it while the target is still `max-content` reads a wider layout, so every description that will wrap is still on one line and the popup, which is `overflow-hidden` at exactly that height, clipped its own last row.
isaque-bock-azion
previously approved these changes
Sep 9, 2026
…tandard ratchet Trim four verbose-comment-block / comment-heavy-file violations flagged by the Adoption Toolkit CI check in viewport-size sizing, the trigger's keydown handler note, and the popup style presets — keeping the non-obvious why, dropping the narrative.
guilherme-santana-azion
approved these changes
Sep 9, 2026
isaque-bock-azion
approved these changes
Sep 9, 2026
Merged
gabriel-lisboa-azion
added a commit
that referenced
this pull request
Sep 21, 2026
demo/vue-sample had drifted 540 ahead / 31 behind main while most of its webkit work was reviewed and landed upstream (#941 command-menu, #942 GlobalHeader placements, #943 NavigationMenu, #952 Menu, #960 sidebar collapse, #973 floating-panel placement, #975 paginator, #977 templates exports, #921 amount ladder), after which main swept comments (#951) and reworked the CLI onto a derived bundle (#968/#970). Main is the source for everything the two share. Of the 28 conflicted source files, 17 differed from main only in comments — their code was byte-identical — and 6 more were genuinely newer on main, so all 23 take main verbatim, including the use-placement TypeScript rewrite that replaces the old index.js. Three carve-outs keep demo work main does not have yet: - sidebar: main's CSS-driven collapse is the base; the `side` axis is re-applied on top of it as data-[side=end] variants rather than the JS edge map it used to be, and the drag sign moves into the rail composable. The one test that asserted the removed inline transform now asserts the panel's published edge, since the direction is CSS the unstyled test env cannot see. - theme: texts.data.js keeps demo's 13 extra text-wrap and 10 extra tracking declarations (PR #982, in flight). Both sides carry the same 34 entries, so an entry count would not have caught the loss. - doc-prose keeps its chrome-padding fix (PR #981, in flight). catalog.json, authoring-baseline.json, the theme dist and the lockfile are regenerated rather than hand-resolved; the 13 conflicted visual baselines take main's and are regenerated from CI next. Verified: every file main touched is byte-identical to main except the 22 accounted for above; 2364 browser tests, 179 toolkit tests, lint, vue-tsc, 99.98% type coverage, both ratchets, the Storybook build and the webkit-sample build all pass.
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
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.
What
The
NavigationMenupanel is reworked for the mega-menu shape, and two silent defects found while building it are fixed.Panel treatment
Overlinecomponent, which painted brand primary attext-overline-mdand brought its own padding. That component, not the uppercase, was what made the heading shout. The inset is a margin, not padding, and that is the trick: the words start on the entries' icon column while the painted bottom edge stops that far short of the track on each side, so adjacent columns are separated by a real break (twice the inset) instead of the 2px their transparent side borders happen to leave.href, so a section can link to its own page above the entries that lead into parts of it. It stays the same quiet row — pointer, ink lifting to--text-default, focus ring, no hover surface (a filled row here would read as one of the entries it labels) and no radius (a rounded box would propose a control that is not there). Following it closes the menu, the same close aclose-on-clickentry performs and for the same reason: the panel must not outlive the navigation it triggered.NavigationMenuPopuptakeskind="contrast", which inverts the panel rather than lightening it. Over a dark canvas the two separators an overlay leans on are inert — a backdrop cannot darken black, a shadow on black is invisible — and the fill cannot simply be raised either: entry copy is--text-muted(#808080), which holds 4.66:1 on--bg-surface-raised(#141414) and falls to 2.07:1 by ~#4D4D4D, so every fill lighter than ~#181818 is under the 4.5:1 floor. The variant redefines the tokens the panel's parts already paint from, so no sub-component knows the panel inverted. Measured: title 20.3:1, description 5.7:1.MenuItemgives a row's glyph), and descriptions to 14px so title and description read as one two-line block rather than a footnote under it.Fixes
The trigger's keyboard model was entirely dead.
compiles to an inline statement — Vue only treats a bare identifier or member path as a method handler — so the ternary was evaluated on every keystroke and its result thrown away. Nothing errored, and the failure was partial in the most misleading way:
Escapenever closed the panel and the arrows never roved between triggers, whileEnterandSpacekept working because a native<button>turns them into a click.Panel measurement took the height at the wrong width.
--popup-widthwas measured withwidth: auto, which is "fill the parent" — that reads as the panel's natural width only while the positioner is itself shrink-wrapped. Give the positioner a width of its own (a mega-menu laid on a page column) andautomeasures every panel as that column, so they all morph to one width and none is ever measured. It isfit-contentnow. The height is then taken in a second pass at the width the panel will actually be laid out at: measuring it while the target is stillmax-contentreads a wider layout, so every description that will wrap is still on one line, the height comes back short, and the popup —overflow-hiddenat exactly that height — clipped its own last row. When nothing constrains the popup the two passes lay out identically and it is a no-op.Verification
vitest(browser mode, Playwright Chromium) — 18/18 passEscapecloses,ArrowRightroves and opens) were run against the old dead-ternary form first and both fail there; they pass with the handler. Without that step this fix would have merged unguarded.vue-tsc --noEmitcleaneslint --max-warnings=0cleanprettier --checkcleancheck-authoring.mjs— 0 new violationscatalog.jsonregenerated — no diff (the root's public API is unchanged;kind/hrefare sub-component props)Contrast ratios and the panel geometry are asserted by measurement, not by the unit suite — that env renders without Tailwind, so a computed-style assertion there would pass in both the broken and fixed states.