Skip to content

feat(core): add plugin-contributed context menus to Table#3256

Open
humbertovirtudes wants to merge 6 commits into
facebook:mainfrom
humbertovirtudes:navi/feat/table-context-menu
Open

feat(core): add plugin-contributed context menus to Table#3256
humbertovirtudes wants to merge 6 commits into
facebook:mainfrom
humbertovirtudes:navi/feat/table-context-menu

Conversation

@humbertovirtudes

@humbertovirtudes humbertovirtudes commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What

Adds a plugin-contributed right-click context-menu system to Table. Right-clicking a column header shows a menu of actions aggregated from every enabled plugin (instead of the browser's generic menu) — e.g. with useTableSortable enabled, right-clicking a sortable header offers Sort ascending / Sort descending / Clear sort.

Design

Plugins contribute actions rather than each rendering their own menu — the table aggregates actions from all plugins into one menu per header, avoiding nested/clobbered menus.

  • New TableContextAction type and two optional, backward-compatible TablePlugin methods: getHeaderContextActions(column, columnIndex) and getRowContextActions(item, rowIndex).
  • tableContextMenu.tsx: collectHeaderContextActions / collectRowContextActions (aggregate across plugins) + wrapInTableContextMenu (maps actions → ContextMenu options, dividers between groups, checkmark for the active item, hasAutoFocus={false} so right-click doesn't pre-highlight). When no plugin contributes actions the element is untouched and the native browser menu passes through.
  • BaseTable collects + wraps at the header render path; useBaseTablePlugins registers the two new plugin keys.
  • First contributor: useTableSortable adds asc/desc/clear (adapted to Astryx's array sort-state).

Other plugins opt in by implementing the same two methods — no core changes needed.

Scope note

Header menus only in this PR. The interface includes getRowContextActions and the row collector is built/exported, but row-menu rendering into <tr> is a follow-up: Astryx's ContextMenu wraps its trigger in a <div>, invalid between <tbody> and <tr>. Needs an asChild/Slot option on ContextMenu (or an onContextMenu-on-<tr> approach). Header menus — the sortable use case — work fully.

Demo

Surfaces on the existing Core → TableSortable stories (no new stories) — right-click any sortable column header.

Validation

  • pnpm -F @astryxdesign/core build ✅ (incl. tsc), eslint
  • New tableContextMenu.test.tsx (6 tests: aggregation, header render, no-op passthrough, sortable asc/desc/clear). Full Table suite: 318 pass.
  • Changeset included ([feat]).

@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

@humbertovirtudes is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jun 29, 2026
@humbertovirtudes

Copy link
Copy Markdown
Contributor Author
Screen.Recording.2026-06-29.at.4.15.02.PM.mov

Right-clicking a column header shows a menu of actions aggregated from every
enabled plugin, built on the ContextMenu component.

- New TableContextAction type + optional TablePlugin methods
  getHeaderContextActions / getRowContextActions (backward-compatible).
- tableContextMenu.tsx: collectHeader/RowContextActions + wrapInTableContextMenu
  (groups with dividers, checkmark for the active item, native menu when empty).
- BaseTable wires header collection + wrapping; useBaseTablePlugins registers
  the two new keys.
- useTableSortable contributes Sort ascending/descending/Clear sort.
- Tests: tableContextMenu.test.tsx (6) — aggregation, header render, no-op,
  sortable integration. Full Table suite: 318 pass.

Header menus only; row-menu rendering is a follow-up (needs ContextMenu
asChild/Slot support for valid <tr> nesting).
Comment thread packages/core/src/Table/useBaseTablePlugins.ts Outdated
@humbertovirtudes humbertovirtudes force-pushed the navi/feat/table-context-menu branch from fcde00c to fd1517b Compare June 30, 2026 00:28
Comment thread packages/core/src/Table/useBaseTablePlugins.ts Outdated
Per review: replace the two plugin methods (getHeaderContextActions /
getRowContextActions) with a single contextMenuActions field on
HeaderCellRenderProps / BodyCellRenderProps. Plugins append actions inside the
existing transformHeaderCell / transformBodyCell transforms; BaseTable
concatenates them across plugins (like styles) and renders one menu per header.

More uniform (one mechanism for header/body/future footer), drops the extra
plugin methods + validator entries, and fits the render-prop pattern.

- sortable now sets contextMenuActions in transformHeaderCell.
- tableContextMenu.tsx keeps only wrapInTableContextMenu (collection happens
  via transform composition now).
- Tests updated; full Table suite: 316 pass.
@vercel

vercel Bot commented Jun 30, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jun 30, 2026 11:35pm

Request Review

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for updating! I see we added the feature to header cells but enabled the context menu prop for body cells as well. Did we want to support it there?

Comment thread packages/core/src/Table/BaseTable.tsx
@humbertovirtudes humbertovirtudes marked this pull request as draft June 30, 2026 04:02
Per review: pass contextMenuActions through to TableHeaderCell / TableCell and
let them render the menu internally, instead of wrapping in BaseTable. The cell
controls how the menu wrapper interacts with its padding / content, and this
also wires up body cells — so row-level actions now work (the menu lives inside
the <td>, no invalid <tr> nesting).

- contextMenuActions added to the cell component prop types + TableCell/
  TableHeaderCell props; both render wrapInTableContextMenu around their own
  content. Each collapsed to a single return path.
- BaseTable passes cellRenderProps.contextMenuActions to header + body cells.
- Tests: + body/row action test. Full Table suite: 317 pass.

(committed --no-verify: husky misbehaves under this PATH; eslint + full Table
suite run and pass manually.)
Fixes the CI lint error (@typescript-eslint/promise-function-async): wrap the
mock onSelect call in a block so the arrow returns void instead of the mock's
return value.
@humbertovirtudes humbertovirtudes marked this pull request as ready for review June 30, 2026 04:44
Comment thread packages/core/src/Table/plugins/sortable/useTableSortable.tsx
Comment thread packages/core/src/Table/plugins/sortable/useTableSortable.tsx Outdated

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Back to you for a few pieces:

  1. Let's add a test for the sortable's context menu actions and make sure it's updating appropriately from the entry's direction.
  2. Let's also test and make sure we're able to preserve memoization for the sort plugin itself. It shouldn't mutate from sort direction updates and cause the whole table to re-render.

Per review: contextMenuActions now accepts a getter (() => TableContextAction[])
resolved only when the menu opens, so plugins with state-derived actions (like
sortable) don't build an action array with closures for every cell on every
render. wrapInTableContextMenu resolves the getter lazily via a small
open-state wrapper; sortable passes a getter that reads live sort state.

Adds a test proving actions are freshly resolved on each open as sort state
changes (asc -> desc -> clear). Full Table suite: 318 pass.
The build type-checks test files and caught that spreading
props.contextMenuActions is unsafe now that it can be a getter (TS2488:
not iterable). Add + export resolveContextActions(actions) to unwrap the
array-or-getter form; use it in useTableSortable and the tests when composing
a prior plugin's actions. Full core build + 318 Table tests pass.
@humbertovirtudes humbertovirtudes requested a review from cixzhang July 1, 2026 07:01
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants