Skip to content

feat(ContextMenu): a sub-menu's header and footer show inside a ContextMenu - #7840

Open
vjeux wants to merge 20 commits into
facebook:mainfrom
vjeux:context-menu-submenu-regions
Open

vjeux wants to merge 20 commits into
facebook:mainfrom
vjeux:context-menu-submenu-regions

Conversation

@vjeux

@vjeux vjeux commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #7839: review the commits after it. This diff includes #7839 until it merges.

User need

A DropdownMenuSubMenu with header or footer (#7530) shows its rows alone inside a ContextMenu, with a development warning: the ContextMenu has no place outside its menu for the dialog. A filter field or a "Create" button on a sub-menu works in a DropdownMenu and disappears in a right-click menu built from the same rows.

Problem and solution fit

ContextMenu renders the same surface host beside its menu as DropdownMenu and provides it on the internal surface context, so a sub-menu with regions gets the same modal dialog outside every menu element: the flyout on a fine pointer, the drilled-in view on a phone. In the ContextMenu bottom sheet, a level whose data item has a header or footer shows them around its actions, as #7839 does for DropdownMenu's sheet.

<ContextMenu menuContent={<DropdownMenuSubMenu label="Labels" header={<TextInput label="Filter labels" … />}>{rows}</DropdownMenuSubMenu>}>
  {row}
</ContextMenu>

Current specification

component:ContextMenu (draft) gains proposed FR12, projecting component:DropdownMenu FR20 and DEC-13, which now name the root, DropdownMenu or ContextMenu, that renders the host. Other proposals in open pull requests: #7303 ContextMenu FR10, #7308 ContextMenu FR11; in component:DropdownMenu, #7319 FR18, #7352 FR19, #7532 FR21 and #7541 FR22, and #7839 amends FR20 for data items and adds FR23, live sheet levels, which FR12 also points at.

Public delta and caller burden

  • In a ContextMenu's pointer menu, a sub-menu's header and footer render, its row says aria-haspopup="dialog", and the development warning no longer fires; compound content in its bottom sheet keeps the rows alone, with the warning.
  • ContextMenu bottom sheet: a level with its own regions shows them.
  • No new props; without regions nothing changes.

Design and alternatives

Rejected: keeping rows alone in a ContextMenu, where the same sub-menu would lose its controls by root.

Writing rubric results

  • R1 narration: none.
  • R2 research in a record: none.
  • R3 evidence naming a change: none.
  • R4 rejected alternatives over one line or outside a decision: none.
  • R5 pull request or issue references: none.
  • R6 ruled open questions listing options: none.

Evidence

See Testing.

Scope

  • One capability is added.
  • Unrelated fixes, cleanup, and policy changes were removed or split.
  • Consumer docs, focused tests and a changeset ship with the feature.
  • Public text and artifacts contain no internal Meta context.

Testing

Rebased onto main 9f3ab05 (2e70bd8) with #7839's commits, whose content matches #7839's 8bb57c6: #7914 judges a press inside a shadow root on the menu's own root before the document, and the surface-host check sits beside it in the one predicate both listeners run, so a press in a sub-menu's dialog stays inside there too. The patch is otherwise unchanged. DropdownMenu, ContextMenu, Breadcrumbs and hooks suites: 948 tests; sub-menu header and footer, menu press shadow-root and ContextMenu audit Chromium specs: 27; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Review round (02fb449, on #7839's 8bb57c6): ContextMenu's sheet reads each opened level from the current items with the same path entries as DropdownMenu's, where main kept a snapshot; FR12 now says its sheet levels follow DropdownMenu's new FR23, and a case re-renders a ContextMenu sheet with a row added to an open level, failing with the level read from its open-time row, as on main. DropdownMenu, ContextMenu, Breadcrumbs, Layer and MoreMenu suites: 689 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Review round (43ae3eb): a right-click on a regioned sub-menu's row reached the browser, since its rows render in the dialog outside the root's menu element; the handler now cancels a right-click inside any role="menu", and a field in a header or footer keeps the browser's own menu. The case also right-clicks the sub-menu's "Bug" row and fails with the previous ContextMenu.tsx. The pending sub-menu header and footer changeset no longer says a ContextMenu shows neither. DropdownMenu, ContextMenu, Breadcrumbs, Layer and MoreMenu suites: 687 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Restacked onto #7839's 84cb23f (a sheet level with a region of its own shows neither of the menu's); FR20 and the DropdownMenuSubMenu header doc merged with this PR's ContextMenu wording, FR20 at 1186 of 1188. No code change here. DropdownMenu, ContextMenu, Breadcrumbs, Layer and MoreMenu suites: 687 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Review round (91a0eeb, on #7839's 2f03c7c): the menu cancels contextmenu, and React bubbles the dialog's events through it although the dialog renders beside it, so a right-click on a header field there lost the browser's own menu (paste, spelling); the handler now cancels only events that begin inside the menu element. A case right-clicks the dialog's field (not cancelled) and a row (cancelled), and fails with the previous ContextMenu.tsx. The label step for an opened row whose id is gone is #7839's: a row with an id is found by identity or that id alone (6df50bc, in this stack). DropdownMenu, ContextMenu, Breadcrumbs, Layer and MoreMenu suites: 686 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Restacked onto #7839's 8e8d156 (comments and docs: the sheet's fallback is the open-time shape; when a row needs an id), no conflicts or changes here. DropdownMenu, ContextMenu, Breadcrumbs, Layer and MoreMenu suites: 685 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Rebased onto #7839's 9206efa (FR20 scopes the empty-items rule to its roots): here FR20 reads "a DropdownMenu or ContextMenu makes its surface a modal dialog", with the empty-items rule under both, since ContextMenu passes showsRegions; Breadcrumbs keeps such a row a plain row. Record wording only on top of the previous head. DropdownMenu, ContextMenu, Breadcrumbs, Layer and MoreMenu suites: 685 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Review round (32f3358, on #7839's 3c96ff6): for the owner's decision, FR2, the Pointer menu surface's design row, the paragraph above the theming anatomy, the ownership boundary and FR2's verification row now name the surface host that carries the context-menu target while a sub-menu's drilled-in view shows (proposed FR12); the host is part of the Pointer menu surface, so the public anatomy and the theming map are unchanged. How a sub-menu whose filter matched nothing is announced is left to that decision: today its menu is empty and announces nothing of its own, and focus stays in the field, as for the root menu's header. Record wording only, plus #7839's sheet fix. DropdownMenu, ContextMenu, Breadcrumbs, Layer and MoreMenu suites: 685 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Rebased onto #7839's 6df50bc (the sheet finds a row by its label only when the label was its own), no conflicts or changes here; ContextMenu's sheet uses the same path entries. DropdownMenu, ContextMenu, Breadcrumbs, Layer and MoreMenu suites: 684 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Rebased onto #7839's ecedadd, which fixes the positional fallback in the shared sheet path (a case where the menu's search filters out the opened row, ids or not, locks it in). ContextMenu passes showsRegions to renderDropdownItems (pointer menu) and MenuBottomSheetActionList (sheet), since it now shows a sub-menu's regions in both, so a data item with a header and empty items opens a sub-menu that shows its field; #7839's ContextMenu plain-row case becomes that case here, and it fails without the flags. DropdownMenu, ContextMenu, Breadcrumbs, Layer and MoreMenu suites: 683 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Rebased onto #7839's 056b6f6: ContextMenu's sheet keeps the opened rows as #7839's path entries (openSheetLevel), so it also finds a level whose label changes. DropdownMenu, ContextMenu, Layer and MoreMenu suites: 631 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Review round (5c782b2, on #7839's head e9f7275 over main 968130f): a data item whose header filter leaves its items empty became a plain row. The fix is #7839's, in the code this pull request shares with it (renderDropdownItems, MenuBottomSheetActionList), so a row with header or footer stays a sub-menu while its items are empty in ContextMenu too. A new ContextMenu bottom-sheet case types a filter that matches nothing, goes Back and opens the level again with the field as left; it fails with the previous MenuBottomSheetActionList.tsx. ContextMenu.doc.mjs says so. DropdownMenu, ContextMenu, Layer and MoreMenu suites: 629 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Review round (3d7064b): ContextMenu dismisses on a document mousedown outside its menu, and the dialog renders in the surface host beside it, so a mouse press, or a touch tap's compatibility mousedown, on its field, button or rows closed the whole menu without acting; the check now counts the host as inside. ContextMenu also starts the shared interaction-modality tracker, as DropdownMenu does, so a drilled-in regions view opened by mouse reads pointer input and shows no keyboard ring (#7303 adds the same call for its own rule; whichever merges second keeps one). With the previous ContextMenu.tsx, three new cases fail: a user-event case clicking the field, the button and a row; a modality case in its own file, since the tracker is document-wide once started; and a Chromium case on a new ContextMenu "Submenu / header and footer" story (the field click finds no field). With the change all pass. DropdownMenu, ContextMenu, Layer and MoreMenu suites: 626 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 26; typecheck, lint:strict, check:knowledge, Storybook build: clean.

Review round (on #7839's head fcf3a1d over main cab6e90): the bottom sheet, which renders no surface host, provides no surface context, so a compound sub-menu with regions shows its rows alone there with the development warning instead of its dialog inside the sheet's menu; the surface host carries the menu's context-menu theme target and the caller's xstyle, className and style, so a drilled-in view keeps a themed or caller-styled menu's look; ContextMenu.doc.mjs lists the header and footer item keys; FR12, FR20 and the changeset say so. Two new ContextMenu.test.tsx cases (the sheet's rows alone; the view's surface carrying the theme target, class and style) fail with the previous ContextMenu.tsx and DropdownMenuSubMenu.tsx. DropdownMenu, ContextMenu, Layer and MoreMenu suites: 624 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 25; typecheck, lint:strict, check:knowledge, Storybook build: clean.

On #7839's head 232688c over main b7860bc.

  • ContextMenu.test.tsx: "renders a sub-menu with a header and footer beside the menu, in a dialog named by its row" replaces the rows-alone case, and "shows a level's own header and footer around its actions in the BottomSheet" is new. With the parent's ContextMenu.tsx and DropdownMenuSubMenu.tsx, 2 failed; with the change, 2 passed.
  • DropdownMenu, ContextMenu, Layer, MoreMenu and doc-prop suites: 622 tests; sub-menu header and footer and ContextMenu audit Chromium specs: 25; typecheck, lint:strict, check:knowledge, Storybook build: clean.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 21, 2026
@vercel

vercel Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Oct 22, 2026 7:22pm UTC

Request Review

@astracat-bot

astracat-bot Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
❓ Needs maintainer input (for maintainers only) Oct 22, 2026, 7:46 PM UTC

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR

🧪 Sandbox Preview

View Sandbox for this PR

Modified Components

ContextMenu (@astryxdesign/core) · View in Storybook
Metric Before After Delta
Bundle Size (ESM) N/A N/A N/A
Lines of Code N/A 748 -
Complexity N/A Very High (95) -

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.9KB 1.2KB

Accessibility Audit

Status: No accessibility violations detected.

Visual Regression

46 of 494 shot(s) changed.

A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.

component story theme mode pixels
Selector Indicator Space Bottom Sheet Narrow End RTL neutral light 753,933
Selector Indicator Space Bottom Sheet Wide Start neutral light 740,072
Chat Full AI Chat probe light 230,823
Chat Full AI Chat probe dark 229,805
RichTextEditor Markdown parity: side by side probe dark 94,866
RichTextEditor Markdown parity: side by side probe light 89,762
Collapsible Dividers probe dark 66,445
Collapsible Dividers probe light 66,445
Chat Default neutral dark 36,304
Chat Default neutral light 35,767
Chat Empty State probe light 23,531
Chat Empty State probe dark 22,622
ChatAutoScroll Scroll Behavior Comparison neutral dark 15,378
ChatAutoScroll Scroll Behavior Comparison neutral light 14,107
Chat With Attachments probe light 13,406
Chat With Attachments probe dark 11,232
ProgressBar With Target Mark probe dark 7,895
ProgressBar With Target Mark probe light 7,895
Markdown Default neutral dark 7,573
Markdown Default neutral light 7,566

and 26 more.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on a maintainer decision: component:ContextMenu FR12 is new and component:DropdownMenu FR20 and DEC-13 are rewritten, all in draft records, and no owner has approved that text at this head, so a maintainer needs to approve it or say what must change (including FR20's change from showing a ContextMenu sub-menu's rows alone to showing its regions). Separately, one fix is needed whatever the decision: in a ContextMenu with presentation="bottom-sheet" and compound menuContent, a sub-menu with header or footer now renders its dialog, field and button inside the sheet's role="menu" without the development warning, because the surface context is provided where no host is rendered (ContextMenu.tsx, the DropdownMenuSurfaceContext provider).

  • Providing null for the surface context when the bottom sheet is used would keep the previous rows-alone fallback there, and a compound bottom-sheet test would lock it in.
  • Smaller points: ContextMenu.doc.mjs does not list the new header and footer item keys, and on a phone the drilled-in regions view is painted without the context-menu theme target or the caller's styling.

[Automated review]

<DropdownMenuRootOpenContext value={isOpen}>
{resolvedMenuContent}
</DropdownMenuRootOpenContext>
<DropdownMenuSurfaceContext value={surfaceValue}>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renderedMenu is also the compound content of the bottom sheet (line 830), where no surface host is rendered. A DropdownMenuSubMenu with header or footer there sees the context, turns regions on (aria-haspopup="dialog"), finds no host element, and renders its dialog flyout in place, inside the sheet's role="menu", with no dev warning. Passing usesBottomSheet ? null : surfaceValue keeps the rows-alone fallback in the sheet.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 5656b06: the surface context is null in the bottom sheet, so a compound sub-menu with header or footer shows its rows alone there, with the development warning, now worded "only in a DropdownMenu or in a ContextMenu's pointer menu". A new compound bottom-sheet case pins it and fails with the previous ContextMenu.tsx. ContextMenu FR12, DropdownMenu FR20 and the changeset say so.

// menu, never inside it. It owns no box until such a sub-menu's drilled-in
// view shows there; then the menu steps aside (see `menu`) and the host
// paints the menu's surface in its place.
surfaceHost: {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While a regioned drilled-in view shows, this host paints the menu's surface with fixed tokens, but the context-menu theme target and the caller's xstyle, className and style stay on the hidden menu (lines 750–759). Themed or caller-styled ContextMenus change look when the view opens.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 5656b06: the host carries the menu's context-menu theme target and the caller's xstyle, className and style, with its own display rule applied last, so it still owns no box until a view shows. Its radius reads --_dropdown-menu-radius, as the menu's does, so a theme's radius applies too. A new case checks that the drilled-in view's surface carries the theme class and the caller's class and style; it fails on the previous head. ContextMenu.doc.mjs now lists the header and footer item keys.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on a maintainer decision: component:ContextMenu FR12 is new and component:DropdownMenu FR20 and DEC-13 are rewritten, all in draft records, and no owner has approved that text at this head, so a maintainer needs to approve it or say what must change (including FR20's change from showing a ContextMenu sub-menu's rows alone to showing its regions). Separately, one fix is needed whatever the decision: inside a ContextMenu, a mouse press on a regioned sub-menu's row, header field or footer button closes the whole menu without acting, because ContextMenu's outside-press listener (ContextMenu.tsx lines 503–521) treats the new surface host as outside the menu.

  • Counting the surface host as inside that check, with a ContextMenu test that clicks a row, the field and the button using real pointer events, would fix and lock it in.
  • Smaller: a drilled-in regions view opened by mouse shows the keyboard focus ring, because ContextMenu does not start the shared interaction-modality tracker as DropdownMenu does.

[Automated review]

<LayerDepthProvider>
{renderedMenu}
<div
ref={attachSurfaceHost}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This host renders beside the role="menu" element, outside listRef, so the document mousedown listener at lines 503–521 (!menu.contains(e.target)) closes the ContextMenu on any mouse press inside a regioned sub-menu. In Chromium a click on one of its rows, its header field or its footer button closes the whole menu without acting, and a touch tap on the field does too; only the keyboard reaches them. Counting the host as inside (for example && !surfaceHostRef.current?.contains(e.target as Node)) plus a ContextMenu case that clicks a row, the field and the button with real pointer events would cover it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 3d7064b: the outside check is now menu && !menu.contains(target) && !surfaceHostRef.current?.contains(target), so a press in the dialog (its field, button or rows) stays inside, and so does a touch tap's compatibility mousedown. Two cases cover it, both failing with the previous ContextMenu.tsx:

  • a user-event case that clicks the field (it keeps focus and takes typing), the button (it acts) and a row (it acts), with the menu's popover still open after the first two;
  • a Chromium case on a new ContextMenu "Submenu / header and footer" story that right-clicks, opens Labels, then clicks the field, types, clicks "Create label" and picks a row. Against a Storybook built from the previous ContextMenu.tsx, the field click finds no field.

[closeMenu, size, drillIn, label],
);
// What a sub-menu with a header or footer needs, on an internal context.
const surfaceValue = useMemo<DropdownMenuSurfaceValue>(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the surface provided here, a regioned sub-menu's drilled-in view paints the keyboard focus ring when getInteractionModality() reads 'keyboard' (DropdownMenuSubMenu.tsx line 1177). DropdownMenu starts the shared tracker at its root (DropdownMenu.tsx line 972); ContextMenu does not, so on a page with nothing else tracking, a mouse open of the view shows the ring. Calling useInteractionModalityTracking() in ContextMenu would match DropdownMenu.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 3d7064b: ContextMenu calls useInteractionModalityTracking(), as DropdownMenu does, so a mouse open of the drilled-in view reads pointer input and shows no keyboard ring. Its case lives in a file of its own (ContextMenuInteractionModality.test.tsx), because once any component starts the document-wide tracker it stays on for the file, and the bottom-sheet cases start it through BottomSheet. It reads "keyboard" with the previous ContextMenu.tsx and "pointer" with the change. #7303 adds the same call for its own rule; whichever merges second keeps one.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on a maintainer decision: component:ContextMenu FR12 is new and component:DropdownMenu FR20 and DEC-13 are rewritten, all in draft records, so a maintainer needs to approve that text on this head or say what must change, including FR20's switch from showing a ContextMenu sub-menu's rows alone to showing its header and footer. The earlier press and focus-ring problems are fixed. One fix is needed whatever the decision: a data item whose header filter leaves its items empty is rendered as a plain row (renderDropdownItems.tsx line 114 and MenuBottomSheetActionList.tsx line 82), so in the popover the field and footer vanish after one unmatched character, and in the bottom sheet the level cannot be reopened after Back.

  • Treating a data item that has items (even empty) and a header or footer as a sub-menu in both places, with a test that types a filter matching nothing, would fix it and lock it in.

[Automated review]

label={option.label}
isDisabled={option.isDisabled}>
isDisabled={option.isDisabled}
header={option.header}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These keys reach the sub-menu only while option.items.length > 0 (line 114); a header filter that empties items turns the row into a leaf (renderLeafItem strips header and footer), so the field and footer unmount mid-typing and focus falls to the page. MenuBottomSheetActionList.tsx line 82 has the same test, so in the sheet the level can't be reopened after Back. Treating a row with items (even empty) plus a renderable header or footer as a sub-menu in both places, as compound DropdownMenuSubMenu already behaves, would fix it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed where the code lives: in #7839's e9f7275, which this pull request now sits on (5c782b2). renderDropdownItems and MenuBottomSheetActionList both read a new opensSubMenu. A row with items opens a sub-menu, and with a renderable header or footer it stays one while items is empty, as compound DropdownMenuSubMenu does. Without either, an empty items is still a plain row.

Tests that type a filter matching nothing, each failing with the previous code:

FR20 and the docs say so.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on a maintainer decision: component:ContextMenu FR12 is new and component:DropdownMenu FR20, its Sub-menu regions concept and DEC-13 are rewritten in draft records, and no engineering owner has approved that text on this head. A maintainer needs to approve it with an exact-head /approve-spec or say what must change, including FR20's switch from showing a ContextMenu sub-menu's rows alone to showing its header and footer. The problems raised on earlier heads are fixed here, including a data item whose filter matches nothing now keeping its level, and no other code change is needed. One point for that decision: the proposed text does not say how an empty filtered level is announced to screen-reader users.

[Automated review]

@vjeux
vjeux force-pushed the context-menu-submenu-regions branch from 5c782b2 to 42522c6 Compare October 22, 2026 02:06

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on a maintainer decision: component:ContextMenu FR12 is new and its FR2 is reworded, and component:DropdownMenu FR20, its Sub-menu regions row and DEC-13 are rewritten in draft records, so an engineering owner needs to approve that text on this head with /approve-spec or say what must change, including FR20's switch from showing a ContextMenu sub-menu's rows alone to showing its header and footer. One code fix is needed whatever the decision: the menu's onContextMenu (ContextMenu.tsx line 780) now leaves every right-click in a regioned sub-menu's dialog to the browser, so a right-click on one of its rows opens the browser's menu where the base cancels it. Cancelling when the target is inside any role="menu", with a case that right-clicks such a row, would fix it and lock it in.

  • Smaller: the pending #7530 changeset still says a ContextMenu does not show a sub-menu's header or footer, which contradicts this pull request's changeset.

[Automated review]

Comment thread packages/core/src/ContextMenu/ContextMenu.tsx Outdated
Comment thread .changeset/context-menu-submenu-regions.md

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on a maintainer decision: component:ContextMenu FR12 is new and its FR2 is reworded, and component:DropdownMenu FR20, its Sub-menu regions row and DEC-13 are rewritten and FR23 is new, all in draft records with no engineering-owner approval on this head. An engineering owner, the author included, needs to approve that text with /approve-spec naming this head, or say what must change, including FR20's switch from showing a ContextMenu sub-menu's rows alone to showing its header and footer, and whether the records should say how a sub-menu whose filter matches nothing is announced. No code change is needed: the problems raised on earlier heads are fixed and covered by tests.

[Automated review]

vjeux added 20 commits October 22, 2026 12:10
…nd the bottom sheet shows a level's own

A data item with items takes header and footer, as DropdownMenuSubMenu does: renderDropdownItems passes them to the sub-menu it renders, so the popover shows them as for any sub-menu with regions.

A level whose data item has a header or footer shows them in place of the menu's own. The level is read from the current items (by identity, id, or a label unique at its level), so a field there filters it live. Each level's regions are keyed by whose they are, so what was typed into one field never shows up in another.
…nu while its items are empty

renderDropdownItems and MenuBottomSheetActionList made a row a sub-menu only while items.length > 0, so a header filter that matched nothing turned it into a plain row: in the popover the dialog, its focused field and its footer unmounted mid-typing, and in the bottom sheet the level could not be opened again after Back. Both now read opensSubMenu: a row with items opens a sub-menu, and with a header or footer it stays one while items is empty, as the compound DropdownMenuSubMenu does. Without either an empty items is still a plain row. FR20, the docs and the changeset say so. Two new cases (the popover's field keeps focus and value through a filter that matches nothing; the sheet's row opens its level again after Back) fail with the previous renderDropdownItems.tsx and MenuBottomSheetActionList.tsx.
…nd finds a level whose label changes

Two defects at a level read live from inline items. The keyboard-open focus effect depended on currentItems, a new array on every caller render, so after a keyboard open each keystroke in a level's field (or the menu's own controlled header) refocused the sheet's first action; it now runs on the open and on level changes (submenuPath). And a row whose label changes while its level shows (a count in it) matched no row, so the level kept its open-time header and rows; the sheet now remembers where each opened row sat, and when no row at its level carries its label any more it finds the row there, if that row opens a sub-menu. Repeated labels still keep the open-time shape without an id, as before. A keyboard-open case typing into a level's field and a counted-label case typing a filter fail with the previous DropdownMenu.tsx and menuSheetPath.ts respectively. The header JSDoc and the changeset say when an id is needed.
…can be the same row, and empty levels only where regions show

The positional fallback in the sheet's level path took whichever sub-menu row sat at the opened row's index, so a row filtered out of its level, by the menu's own search header say, switched the sheet to a different level, ids or not. Each path entry now keeps its level's row count, and position is used only for a row without an id while the level keeps that count; otherwise the level keeps its last known shape. And the empty-items rule (a row with header or footer stays a sub-menu while items is empty) applied wherever renderDropdownItems and MenuBottomSheetActionList render, ContextMenu and Breadcrumbs included, where no sub-menu regions show: a row with header and items: [] there became an empty sub-menu instead of a plain row. Both now take showsRegions, which DropdownMenu's popover and sheet pass and others leave off. A sheet case where the menu's search filters out the opened row (without ids and with them) and a ContextMenu case for a header with empty items fail with the previous code.
…abel was its own

When the opened row left items while its level showed, the sheet took the one row that now carried its label, even when the opened row had an id that row did not share: with two rows labelled Size, ids included, the menu's own search filtering out the opened one switched the sheet to the other's sub-menu. A row with an id is now found by identity or that id alone, and a row without one by its label only when the label was its own at its level when it opened (SheetPathEntry.isLabelUnique) and still is; otherwise the level keeps its last known shape. A case where the opened one of two rows labelled alike leaves items, with ids and without, fails with the previous menuSheetPath.ts in both arms. The header JSDoc and the changeset say so.
…s its last rows

When an open level's row was found again but no longer had items (a Recent row built as items: recents.length > 0 ? ... : undefined, emptied by a Clear recent button in the menu's own footer), the sheet fell back to the menu's root items and listed them under the level's heading, Back and dialog name. resolveSheetLevel now takes a found row as the level only while it opens a sub-menu; otherwise the level keeps its last known shape, as it does for a row filtered out. The header comment also says that, without an id, the label match reads uniqueness from the rows shown when the level opened, so rows whose labels repeat need an id for the guarantee. A case where the open level's row loses its items fails with the previous menuSheetPath.ts.
… and the items doc says when the sheet needs an id

FR20's "or a data item with items, even empty" read as applying under ContextMenu and Breadcrumbs too, where such a row stays a plain row (showsRegions is off there); it now reads "in a DropdownMenu even empty", and the cell keeps its width. The items doc and the id JSDoc said an id is needed only when the array reorders or filters; they now also name the bottom sheet's case, a row with nested items whose label another row at its level shares, as the header JSDoc does. Record and doc wording only.
…the docs say when a row needs an id

When the sheet cannot find an opened row again, the level keeps the shape it had when it opened, its rows, header and footer then, so a controlled field in that open-time header shows its open-time value. The header comment called that the last known shape; it now says the open-time shape and names the case the bot found: a row without an id whose label changes while its level shows (Labels (${n})) is found only while its level keeps its row count, so adding a root row brings the open-time level back. The items doc, the id and header JSDoc and the changeset now say an id is needed when the array reorders, filters or grows, and in the sheet when a row's label repeats or changes while its level shows. Comments and docs only.
…sses none on

The data-mode case asserted that the Rename row carries no header attribute, but Rename had no header, so it passed with or without renderLeafItem's strip. Rename now has a header: the case checks that no such field renders and that the row carries no header attribute, which fails when the strip is removed.
…her of the menu's

In the bottom sheet a level whose row has a header or footer of its own shows its regions and neither of the menu's, so a level with only a footer also drops the menu's header: the menu-wide search would filter rows the level does not show. The header and footer JSDoc, the items and DropdownMenuSubMenu docs, the changeset and FR20 read as a per-region swap; they now say so (FR20 keeps its width). A case drills into a level with only a footer and finds neither the menu's search nor its footer; it fails with a per-region fallback.
… from the current items

The sheet reads each opened level from the current items for every caller, regions or not, so a row the caller adds, removes or relabels while a level shows, or that the menu's own header filters, shows at once; the base kept a snapshot from when the level opened. FR20 ended "Without either, nothing changes", which the live read contradicted. Live levels are intended (a menu-wide search that filters nested rows needs them, with or without level regions), so proposed FR23 states them, with how the sheet finds the opened row again and the open-time fallback, and FR20 now says that without a region the sub-menu's surface is unchanged (the cell keeps its width). The items JSDoc and docs and the changeset say so. A case re-renders a menu with no regions anywhere with a row added to an open level; it fails with the level read from its open-time row, as the base did.
…xtMenu

ContextMenu renders the same surface host beside its menu as DropdownMenu and provides the internal surface context, so a DropdownMenuSubMenu with header or footer gets its dialog outside every menu element there too, and the ContextMenu bottom sheet shows a data item's own regions. Both pass showsRegions to renderDropdownItems and MenuBottomSheetActionList, so a data item with header or footer stays a sub-menu while its items are empty, as in DropdownMenu. Proposed ContextMenu FR12, following DropdownMenu FR20 and DEC-13.
… drilled-in view is painted as the menu is

The surface context is null in the bottom sheet, which renders no surface host, so a compound sub-menu with header or footer shows its rows alone there with the development warning, instead of its dialog inside the sheet's menu. The surface host carries the menu's context-menu theme target and the caller's xstyle, className and style, its own display rule last, so a drilled-in view keeps a themed or caller-styled menu's look. The docs list the header and footer item keys; ContextMenu FR12, DropdownMenu FR20 and the changeset say both. Two new cases fail with the previous ContextMenu.tsx and DropdownMenuSubMenu.tsx.
…u, and the menu tracks input modality

ContextMenu dismisses on a document mousedown outside its menu. A sub-menu with header or footer renders its dialog in the surface host beside the menu, so a mouse press, or a touch tap's compatibility mousedown, on its field, button or rows closed the whole menu without acting. The check now counts the host as inside. ContextMenu also starts the shared interaction-modality tracker, as DropdownMenu does, so a drilled-in regions view opened by mouse reads pointer input and shows no keyboard focus ring. A user-event case clicking the field, the button and a row, a modality case in its own file (the tracker is document-wide once started) and a Chromium case on a new ContextMenu story fail with the previous ContextMenu.tsx.
… its filter matches nothing

ContextMenu's popover and bottom sheet read the same opensSubMenu as DropdownMenu's (facebook#7839's fix). A ContextMenu bottom-sheet case types a filter that matches nothing, goes Back, and opens the level again with the field as left; it fails with the previous MenuBottomSheetActionList.tsx. The items doc says a row with header or footer stays a sub-menu while items is empty.
…that carries the context-menu target

With a sub-menu's drilled-in view showing in the Pointer menu surface's place, the surface host carries the context-menu target and the caller's styling. FR2, the Pointer menu surface's design row, the paragraph above the theming anatomy, the ownership boundary and FR2's verification row now say so, pointing at proposed FR12. The host is part of the Pointer menu surface rather than a new anatomy element, so the public anatomy and the theming map are unchanged. Record wording only; check:knowledge and prettier pass.
…ser's own menu

The menu cancels contextmenu so a right-click on it opens no browser menu. A sub-menu's dialog renders in the surface host beside the menu, but React bubbles its events through the menu, so a right-click on a header field there lost the browser's own menu (paste, spelling), which the same field in a DropdownMenu keeps. The handler now cancels only events that begin inside the menu element. A case right-clicks the dialog's field (not cancelled) and a row (cancelled); it fails with the previous ContextMenu.tsx.
…ncelled

The menu cancelled contextmenu only for events that began inside its own element, but a regioned sub-menu's rows render in its dialog in the surface host, outside that element, so a right-click on one opened the browser's menu over the app's. The handler now cancels any right-click inside a role="menu" element, the root's or a sub-menu's, while a field in a header or footer, outside every menu, keeps the browser's own menu. The case now also right-clicks a regioned sub-menu's row; it fails with the previous ContextMenu.tsx.
…ger says a ContextMenu shows neither

The pending changeset from the DropdownMenuSubMenu header and footer change said a ContextMenu does not show them; this PR makes it show them, and both entries would ship in one release. The sentence is dropped.
…enu's FR23

ContextMenu's sheet now finds its levels with the same path entries as DropdownMenu's, so each opened level is read from the current items while it shows, where main kept a snapshot from when it opened. FR12 now says its sheet levels follow component:DropdownMenu FR23 (the cell stays within its column), and its verification row names live levels. A case re-renders a ContextMenu sheet with a row added to an open level; it fails with the level read from its open-time row, as on main.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on a maintainer decision: component:ContextMenu FR12 is new and its FR2 is reworded, and component:DropdownMenu FR20, its Sub-menu regions row and DEC-13 are rewritten and FR23 is new, all in draft records with no engineering-owner approval on this head. An engineering owner, the author included, needs to approve that text with /approve-spec naming this head, or say what must change, including FR20's switch from showing a ContextMenu sub-menu's rows alone to showing its header and footer, and whether the records should say how a sub-menu whose filter matches nothing is announced. No code change is needed: after the rebase a press in the sub-menu's dialog still keeps the menu open, and the earlier fixes stay covered by tests.

[Automated review]

This branch was successfully deployed

1 active deployment
Preview — 2e70bd8c Deployed Oct 22, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant