docs(menu): add 2nd-gen migration plan - #6606
Conversation
Documents the 1st-gen and React Spectrum S2 Menu API surfaces, dependencies, breaking changes, and a phased migration checklist ahead of the swc-menu implementation work under SWC-1980.
Rajdeepc
added
the
Component:Documentation
|
📚 Branch Preview Links🔍 Gen1 Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
Coverage Report for CI Build 32253792956Warning No base build found for commit Coverage: 96.241%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
Ships swc-menu without selection first (Phase A), deferring the selection engine to a follow-up ticket (Phase B) so reviewers validate a smaller surface per PR. Holds regardless of how Q11 (picker/combobox selection dependency) resolves.
swc-menu anchors via PlacementController directly, not <swc-popover> -- Popover already shipped (PR #6356) and is explicitly excluded from menu/listbox/combobox use per its own a11y doc's design update. Removes the false Popover sequencing blocker entirely. Also: adds B7 disabled-focus conflict, B9/B10 dropped API, an Events (2nd-gen) section, a full 1st-gen member disposition table, corrects Q1's Figma-file framing, strengthens the Figma evidence behind Q2 (selection scope), and renumbers Q1-Q15 accordingly.
Selection lives entirely on Menu in 1st-gen, not MenuItem, so unlike B1/B3/B8 there's no cross-plan ownership question. When Q2 resolves, A2 promotes to Must-ship here directly rather than spinning up a separate migration-plan.md.
Cross-checked the actual Menu item component variants (node 125485:44694) against the property-table inference already in the plan -- matches exactly. Adds one detail: the Unavailable contextual- help bubble is its own sized (S/M/L/XL) component, not fixed-size.
Design confirmed no newer replacement exists for the deprecated-titled S2/Web file. Unblocks Q3 (xl size confirmation).
…onent Regroups Q2-Q15 into "Blocking for swc-menu" (its own properties, architecture, sizing, mobile fate, Jira) and "Cross-component follow-ups" (menu-item/menu-group/menu-separator/action-menu docs and existence questions this plan doesn't own). Q1 stays resolved at the top. Renumbered and cross-checked programmatically.
…city Resolves 6 questions rather than leaving them open: - Q3: PlacementController anchoring, confirmed (already the plan's own strongest evidence -- Tooltip precedent + popover a11y doc). - Q5: selection dependency stays Additive/Phase B; no consumer has a scheduled migration forcing it earlier. - Q6: no `trigger` property in Phase A -- no consumer request, matches 1st-gen, addable later without a breaking change. - Q11: swc-menu-group breaks from MenuGroup extends Menu -- confirmed, free to do now, expensive to do later. - Q12: no shared base class with swc-action-menu yet -- it doesn't exist; premature abstraction risk. Revisit once both exist. - Q14: swc-menu-separator is not built. swc-divider already does everything MenuDivider ever did (wraps divider.css, role=separator). This narrows epic scope and conflicts with the existing menu-separator a11y doc -- flagged for epic-level confirmation rather than buried. Propagated across every reference site (Dependencies, Changes overview, 2nd-gen API decisions, Architecture, Migration checklist, Disposition table) and re-verified programmatically.
| | `open` | `boolean` | `false` | `open` | **Inferred**, matching the menu-button pattern's open/close state, parallel to `swc-action-menu`. | | ||
| | `disabled` | `boolean` | `false` | `disabled` | **Inferred** carry-forward for the trigger. | | ||
| | `trigger` | `'press' \| 'longPress' \| 'contextMenu'` | `'press'` | `trigger` | **Open question** ([Q6](#blocking-for-swc-menu)) — React Spectrum S2's `MenuTrigger.trigger` has no 1st-gen equivalent (1st-gen composes the trigger externally). Depends on whether `swc-menu` owns trigger-invocation semantics itself or defers entirely to how the consumer wires the trigger slot. | | ||
| | `shouldCloseOnSelect` | `boolean` | `true` (**inferred**, matches 1st-gen's default close-on-select for non-`selects` items) | `should-close-on-select` or similar | **Open question**, tied to [Q2](#blocking-for-swc-menu)/[Q5](#blocking-for-swc-menu) — only meaningful once selection ships, since 1st-gen's unconditional `close` event today only fires for non-selectable items. React Spectrum S2 exposes this at both `Menu` and `MenuItem` level. | |
There was a problem hiding this comment.
Do we need to offer this API?
There was a problem hiding this comment.
I confirmed we dont need to offer this API. Leaving it out for Phase A; it can be added additively later without a breaking change if a concrete need shows up.
|
|
||
| #### Slots (2nd-gen) | ||
|
|
||
| | Slot | Content | Notes | |
There was a problem hiding this comment.
So not slot for the actual label? Then the "label" description on the properties above is incorrect. How do we put text on the trigger button?
There was a problem hiding this comment.
Yes this was a gap. Added a label slot for visible trigger content (text, icon, or both), matching sp-action-menu's precedent. The label property stays, but now does double duty.
There was a problem hiding this comment.
We have dropped the "accessible name fallback" chain in other instances and I vote we do so here as well, it's complication we don't need to own when our new standard is use of accessible-label if it needs an aria-label for some reason (which should be lower probability for this type of menu).
| | # | What changes | 1st-gen behavior | 2nd-gen behavior | Consumer migration path | | ||
| | --- | --- | --- | --- | --- | | ||
| | **B1** | `swc-menu-group` no longer extends the full menu host | `MenuGroup extends Menu` — inherits selection engine, roving tabindex, `value`, etc., and overrides `ownRole`/`controlsRovingTabindex` to suppress most of it | `swc-menu-group` is a plain grouping/labeling primitive (per a11y analysis); no selection or roving-tabindex API of its own | Consumers using `sp-menu-group` only for grouping + `header` slot see no functional change; any (unsupported) reliance on inherited `Menu` API on a group instance breaks. Source: [Menu group a11y doc](../menu-group/accessibility-migration-analysis.md#what-swc-menu-group-is-2nd-gen). | | ||
| | **B2** | Link items drop the proxy double-activation pattern | `href` items fire `change` via `Menu`'s click handling in addition to native anchor navigation ([SWC-923](https://jira.corp.adobe.com/browse/SWC-923)) | Real `<a href>` descendant is the only activation path; no proxy `change` dispatch for link rows | Consumers listening for `change` on link-style items must switch to listening on the anchor itself or the native `click`/navigation. | |
There was a problem hiding this comment.
Link styling may become difficult, if we say tags will just be slotted.
There was a problem hiding this comment.
Doesn't block swc-menu's Phase A, but it needs to be solved before menu-item ships link rows.
Its a real risk and it's not this plan's implementation to own — swc-menu-item's own migration plan owns how link rows render. Filed it as Cross-component follow-ups pointing at the menu-item plan owner: if the link-style row works by having the consumer slot a real <a href> into the item's light DOM (per the menu-item a11y doc), styling that anchor to match the row's hover state, full-row hit target, focus ring, and icon alignment is materially harder than 1st-gen's shadow-internal <a class="anchor"> render.
There was a problem hiding this comment.
Just a note: We can probably use the CSS link expansion trick (also used for Card) to increase the slotted links surface to cover the menu item without it actually needing to wrap the other elements, but we may need to test that out.
|
|
||
| | Property | Type | Default | Attribute | Notes | | ||
| | --- | --- | --- | --- | --- | | ||
| | `label` | `string` | `''` | `label` | **Inferred** carry-forward from 1st-gen; accessible name for the trigger. | |
There was a problem hiding this comment.
Is this the trigger button text?
There was a problem hiding this comment.
It was meant to be the accessible name only, but you're right that leaves no way to render anything visible. Added a label slot for the actual visible content; this property row now documents its fallback-text/accessible-name role instead.
|
|
||
| | Slot | Content | Notes | | ||
| | --- | --- | --- | | ||
| | default | `swc-menu-item`, `swc-menu-group`, `swc-menu-separator` only | **Confirmed** per a11y analysis; verify enforcement in 2nd-gen source once implemented. | |
There was a problem hiding this comment.
Gen1 ActionMenu had a "label" slot, we could do something like that? Alternatively calling it "trigger" slot.
There was a problem hiding this comment.
Going with your first suggestion — a label slot, matching sp-action-menu's exactly (checked ActionMenu.ts also it has @slot label for visible content plus a label property used as accessible-name fallback). Let's keep it to a single label slot for now rather than also adding sp-action-menu's separate icon/label-only slots. Thanks for catching this.
React Spectrum's MenuSection supports independent selectionMode per section (plain + single-select + multi-select coexisting), but the Figma "Changes in S2 Menu" note already quoted in Q2 recommends against mixing checkbox sections with other menu types -- these two sources disagree on whether mixing should even be allowed. Separately, even if allowed, the Q11 decision to break `MenuGroup extends Menu` removes the mechanism 1st-gen used to give each group independent selection scope for free, so Phase B needs to design per-group selection scope deliberately (e.g. a `selection-mode` property on swc-menu-group) rather than inherit it. Folded into Q2, A2, and a new "Mixed menus" note under Behavioral semantics.
Matches React Spectrum's Menu.selectionMode + MenuSection.selectionMode split: swc-menu owns a top-level mode for ungrouped items plus the aggregate value/selectedItems across the whole menu; swc-menu-group gets its own independent selection-mode property rather than inheriting one. Together these structurally enable mixed menus. This means Phase B genuinely spans two plans -- corrected the earlier claim that selection stays entirely in this plan because it's "Menu-owned." Propagated to the header note, A2, the disposition table, and the Phase B checklist.
Splits implementation into three phases instead of two: - Phase A (this pass): trigger + plain swc-menu-item children only -- label plus one leading visual (icon or image, not both). No submenus, no swc-menu-group, no divider/separator, no selection. - Phase B (structural completeness, not scheduled): submenus (B3), swc-menu-group (B1), swc-divider as separator (Q14), and the richer item content (A3 Unavailable, A5 highlight badge, A6 external link). - Phase C (selection, not scheduled): single selection mode for the whole menu, matching React Spectrum's own foundational Menu.selectionMode pattern -- confirmed via a docs re-check that section-level/mixed selection is documented there as an advanced pattern layered on top, not the basic case. Per-group selectionMode moves out to Phase C+ / future, not designed into this pass. Tagged every B/A item and checklist bullet that assumed submenu/group/ divider support was in scope for this pass.
…nController's real API Reviewed react-spectrum.adobe.com/Menu#slots and #content, plus the FocusgroupNavigationController's own docs, to make the roving-tabindex model concrete instead of a citation to "the controller": - direction: 'vertical' -- matches the controller's own documented Vertical menu example directly. - wrap: true -- decided; the controller defaults to false (toolbar- oriented), menus need the override for ArrowDown-wraps-to-first convention. - skipDisabled: false -- confirmed to be the controller's own default, not an override swc-menu has to implement. This makes B7 (disabled rows stay focusable) the path of least resistance, not just a preference -- strengthens Q4 accordingly. - Home/End -- built in, no config. - Focus-on-open -- setActiveItem() + queueMicrotask(focus) is the controller's own documented pattern, not something to invent. - A7 (printable character nav) -- confirmed cheap: the controller already exposes focusFirstItemByTextPrefix() for this exact case. Also cross-checked Content/Slots against React Spectrum's own basic MenuItem example (icon + label + description + keyboard shortcut) -- noted that Phase A's icon-or-image-only scope is narrower than React Spectrum's basic case by explicit choice, not because it's gated there as advanced.
- Q2 (selection scope vs a11y doc): decided not blocking -- selection stays Additive/Phase C regardless of when that doc gets corrected. - Q8 (mobile drilldown replacement): decided out of scope -- no replacement committed; A8 stands as the record. - Q7 (xl size Figma confirmation) and Q9 (gen1 Jira issues table): dropped entirely, no longer tracked. Only Q4 (B7 disabled-focus conflict) remains genuinely blocking for swc-menu. Tightened every "once Q2 resolves" gating phrase into "whenever Phase C is scheduled," since Q2 no longer needs resolving to proceed. Re-verified all Q cross-references programmatically.
Collapsed the verbose B/A decision tables into three compact tables: Must ship (Phase A items only, one line each), Additive (A1/A4, Phase A only), and a new Deferred to later phases table holding everything else (B1, B3, A2, A3, A5-A8) without the accumulated inline justification prose. Also fixed a pre-existing bug where A1 pointed at the Must-ship anchor instead of Additive. Renamed section headings (Must ship / Additive / Deferred to later phases) to short, anchor-friendly text and repointed all ~35 B#/A# cross-references throughout the document accordingly. Re-verified with the nav script.
Rajdeepc
added
Status:Ready for review
Ruben caught a real gap: carrying `label` forward as an accessible-name-only property leaves no way to render visible content on swc-menu's own trigger, since 1st-gen sp-menu never had a built-in trigger to put content on. Adds a `label` slot matching sp-action-menu's precedent (Q16), and flags the separate concern he raised about styling consumer-slotted `<a>` link items as a menu-item-owned follow-up (Q17).
4 participants
Description
Adds the Phase 1 (Preparation) migration plan for
swc-menuatCONTRIBUTOR-DOCS/03_project-planning/03_components/menu/migration-plan.md.This is a documentation-only change; no runtime code is touched.
Scope of this ticket (read this first)
The first pass is a deliberately simple menu:
PlacementController-anchored surface + shadow-internalrole="menu"list (not<swc-popover>— that's arole="dialog"component; menu/listbox/combobox build on the sharedPlacementControllerdirectly, per the popover a11y doc's own design update)swc-menu-itemchildren only — a label plus one leading visual, icon or image (not both, not combined with description/value/badge/unavailable content)FocusgroupNavigationController, already shipped and used byTabs): vertical arrow keys, Home/End, wrap-around, disabled items stay focusablehref) are in scopeExplicitly not in this pass (real, documented, just not now):
swc-menu-group, no divider/separatorUnavailablestate, highlight badge, external-link indicator — deferred alongside the aboveSee Implementation phasing for the full three-phase breakdown (A: this pass, B: submenus/groups/dividers, C: selection) and Changes overview for the three short decision tables (Must ship / Additive / Deferred).
Plan summary (for reviewers)
Breaking changes, this pass only (B2, B4–B10):
changepattern that caused double activation (B2, fixes SWC-923)RovingTabindexController→FocusgroupNavigationController(B4, already shipped)--mod-*CSS surface removed (B5)role="menu"moves into shadow DOM, off the custom element host (B6)ignoreproperty dropped (B9);getNeighboringFocusableElement()dropped as public API (B10)Additive, this pass:
xlsize (A1), image as an alternative to icon (A4).Deferred to later phases (documented, not scheduled):
swc-menu-groupbreaking its 1st-gen inheritance (B1), submenus (B3), selection at both menu and group level (A2),Unavailablestate (A3), highlight badge (A5), external-link indicator (A6), printable-character navigation (A7), mobile drilldown — dropped with no replacement (A8).swc-menu-separatoris not being built at all;swc-divideris composed directly instead.Open questions: down to one real blocker — Q4, whether disabled rows should stay focusable (WAI-ARIA APG, and the
FocusgroupNavigationControllerdefault) or match React Spectrum'sdisabledKeys(removes them from focus entirely). Nine other questions are resolved and recorded with their reasoning (architecture model,xlsize, mobile drilldown scope, trigger API, etc.); three are cross-component follow-ups that don't block this plan.Where to start reading: Scope of this ticket above → TL;DR → Blockers and open questions.
Motivation and context
Required planning output ahead of
swc-menuimplementation work, per the "Analyze component and create migration plan" ticket for the Menu epic.Related issue(s)
Screenshots (if appropriate)
N/A — documentation only.
Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
Scope matches what's actually buildable this pass
Plan matches source material
CONTRIBUTOR-DOCS/03_project-planning/03_components/menu/migration-plan.md1st-gen/packages/menu/src/Menu.ts,MenuItem.ts,MenuGroup.ts,MenuDivider.tsRemaining open question is the right one to block on
Device review
Accessibility testing checklist
FocusgroupNavigationControllerwithdirection: 'vertical',wrap: true,skipDisabled: false(the controller's own default), Home/End built in, Escape to close.role="menu"/menuitemsemantics and shadow-DOM placement for the future implementation to satisfy.