[Toolkit][Shadcn] Add navigation-menu recipe

[Amoifr]

Q	A
Bug fix?	no
New feature?	yes
Deprecations?	no
Documentation?	no
Issues	Part of #3233
License	MIT

Adds the navigation-menu recipe to the Shadcn kit.

Part of the split of #3467 into one PR per component, tracking #3233.

The submenu opens on hover and on keyboard focus (a Stimulus controller toggles data-state on each Item), and stays open while the focus moves between the trigger and the links of the submenu.

Note: the ux.symfony.com companion PR (symfony/ux.symfony.com#68) is obsolete since kits are auto-discovered there.

[Amoifr]

Hi @Kocal! Just pushed an update aligning this recipe with the patterns we've validated on the other Shadcn components shipped recently (notably hover-card #3478 merged earlier today).

The original implementation relied on the pure-CSS group-hover/nav-item + group-focus-within/nav-item + tabindex="0" trio, which we found out has a real UX issue: clicking a trigger gives it focus, so the submenu stays stuck open even after the mouse leaves. Removing group-focus-within fixes the click case but breaks keyboard accessibility.

The fix follows the same approach you confirmed for hover-card:

• New navigation_menu_controller.js toggling data-state on the Item (one controller per item)
• mouseenter/mouseleave on the Item, focus/blur on the Trigger
• Content and chevron rotation use in-data-[state=open]:… instead of group-hover/nav-item
• Optional openDelay / closeDelay props on Item, default 0

Companion PR for ux.symfony.com is updated too: symfony/ux.symfony.com#68.

No rush at all, just letting you know it's ready for another look! 🙏

[Kocal]

Thanks for this! Nice to see the hover behavior already moved from group-hover to a Stimulus controller. The main issue: this branch is based on a fairly old 3.x (merge-base is ~418 commits back), and the Toolkit recipe conventions changed a lot since then. A rebase onto current 3.x plus a migration to the new format is needed. Grouped findings below.

Blocking / conventions

• Recipe format: the recipe still uses examples/Demo.html.twig / Usage.html.twig / RTL.html.twig and the old snapshot keys. Current recipes ship a single README.md with inline {"preview":true} blocks (hero preview + ## Usage + ## Examples with one ### Variant per example, ### RTL last, plus the ::: installation / ::: api-reference directives) and no examples/ directory.
• Class merge: every template uses ('<base> ' ~ attributes.render('class'))|tailwind_merge. Since the base and the attributes sink are on the same element, this should become attributes.defaults({ class: '<base>'|tailwind_classes }).
• data-slot and Stimulus value attrs live inside attributes.defaults() (on NavigationMenu, List, Item, Content, Link, plus data-navigation-menu-open-delay-value / -close-delay-value on Item). AttributesDefaultsChecker rejects this: they must be literal attributes, only class, data-controller and data-action belong in defaults().
• Docblocks fail bin/ux-toolkit-kit-lint --fail-on-warning kits/shadcn: Item's docblock still has Defaults to 0 in its text, but defaults belong only in {%- props -%}. Every @prop / @block description is also missing its trailing period.

Accessibility / behavior

• Keyboard users cannot reach the submenu. Trigger wires blur->navigation-menu#hide, so tabbing from the trigger into the Content links fires blur -> hide() and the content becomes invisible before the link can receive focus. Suggest driving open/close from focusin/focusout on the Item with a relatedTarget-within check, so the menu stays open while focus is inside it.
• Content uses in-data-[state=open]:visible, which matches any open ancestor and will leak state into a nested NavigationMenu. A named group is safer: group/navigation-menu on the item plus group-data-[state=open]/navigation-menu:visible (same idea for the chevron rotation).
• The trigger exposes no aria-expanded / data-state reflecting the open state (Radix does). Worth syncing aria-expanded from the controller on show/hide.

Minor

• manifest.json under-declares dependencies (only tales-from-a-dev/twig-tailwind-extra:^1.0.0) while the templates use {%- props -%} and html_attr_type. After moving to tailwind_classes the canonical set is tales-from-a-dev/twig-tailwind-extra:^1.3.0, symfony/ux-twig-component:^3.5, twig/html-extra:^3.24.0. symfony/ux-icons isn't needed since icons only appear in examples.
• Please regenerate snapshots after the rework (phpunit -d --update-snapshots) and check git status for orphaned snapshot files once the examples move into README.md.

[Kocal]

^ I'm on it btw

[Kocal]

A couple of corrections to my review above, after checking against the shipped hover-card recipe (the closest precedent in this kit):

• Please disregard the in-data-[state=open]:visible point. hover-card ships exactly this, and it's correct for a single-level menu: sibling Items don't leak state into each other since each Content only matches its own open <li> ancestor. Named groups (group/dropdown-menu + group-data-[state=open]/dropdown-menu:visible) are only needed when content is genuinely nested, like dropdown-menu's submenus. No change needed here unless you plan to nest NavigationMenus.

• The aria-expanded / data-state-on-trigger point is optional, not required. hover-card's trigger doesn't expose them either (it uses role="tooltip" on the content); these simplified hover recipes intentionally don't mirror Radix's full ARIA model. Feel free to skip it.

One point still stands, but softer than I first put it: the blur->navigation-menu#hide behavior is the same pattern hover-card uses (blur->hover-card#hide), so it's consistent with the kit. The only difference is that a navigation menu's Content holds real links, so closing on blur does keep keyboard users from tabbing into them. Whether that matters is your call, given these recipes are deliberately simplified.

The rest of the review stands, the README migration, tailwind_classes in defaults(), literal data-slot / value attrs, and the docblock fixes are the real blockers. Sorry for the noise on those two.

[Amoifr]

Thanks for the thorough review, and no worries about the noise! Rebased onto current 3.x (single commit) and migrated to the current recipe format:

• README.md with hero preview, Usage, Examples (Basic, With delays, RTL) and the ::: installation / ::: api-reference directives, no more examples/
• attributes.defaults({ class: ...|tailwind_classes }) everywhere, data-slot and the Stimulus value attrs as literal attributes, only class / data-controller / data-action left in defaults()
• docblocks fixed (bin/ux-toolkit-kit-lint --fail-on-warning kits/shadcn is clean), aria-label now comes from a label prop (default Main) since the linter rejects ARIA in defaults()
• manifest with the canonical dependency set and version-added: 3.6; twig/extra-bundle dropped as the trigger no longer needs html_attr_type
• snapshots regenerated, no orphan left

On keyboard access I went with your focusin / focusout + relatedTarget suggestion on the Item: a navigation menu whose links cannot be tabbed into felt wrong, and it stays consistent with the hover behavior. Skipped aria-expanded and the named group as discussed.

[Kocal]

no no no I'm on it 😭

[Amoifr]

Oops, I read "I'm on it" as "I'm on the review", sorry for racing you on this one! 😅 #3801 is clearly the better version (shared viewport, overflow clamp, native trigger, aria-expanded): happy to see it supersede this PR, feel free to close this one whenever #3801 is in. Thanks for taking it over, and for the detailed review, I learned a few kit conventions along the way.

[Kocal]

Sorry it was obvious to me, but looking back, it wasn't clear at all 😓

Thanks for taking it over, and for the detailed review, I learned a few kit conventions along the way.

Basically I'm telling Claude to "update the PR UX and ux.symfony.com + reference our Toolkit kit skill", and it is clever enough to do all the upgrades to make the recipe compliant to Toolkit 3.5 :)