feat(core): add plugin-contributed context menus to Table#3256
Open
humbertovirtudes wants to merge 6 commits into
Open
feat(core): add plugin-contributed context menus to Table#3256humbertovirtudes wants to merge 6 commits into
humbertovirtudes wants to merge 6 commits into
Conversation
|
@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
Bot
added
the
CLA Signed
Contributor
Author
Screen.Recording.2026-06-29.at.4.15.02.PM.mov |
humbertovirtudes
requested review from
cixzhang,
ejhammond,
imdreamrunner,
josephfarina,
marie-lucas,
nynexman4464 and
thedjpetersen
as code owners
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).
cixzhang
reviewed
Jun 30, 2026
humbertovirtudes
force-pushed
the
navi/feat/table-context-menu
branch
from
fcde00c to
fd1517b
Compare
cixzhang
reviewed
Jun 30, 2026
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
cixzhang
requested changes
Jun 30, 2026
cixzhang
left a comment
cixzhang
left a comment
Contributor
There was a problem hiding this comment.
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?
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.
cixzhang
reviewed
Jun 30, 2026
cixzhang
reviewed
Jun 30, 2026
cixzhang
requested changes
Jun 30, 2026
cixzhang
left a comment
cixzhang
left a comment
Contributor
There was a problem hiding this comment.
Back to you for a few pieces:
- Let's add a test for the sortable's context menu actions and make sure it's updating appropriately from the entry's direction.
- 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.
cixzhang
reviewed
Jul 1, 2026
Comment on lines
+164
to
+166
| // ============================================================================= | ||
| // Sortable integration | ||
| // ============================================================================= |
Contributor
There was a problem hiding this comment.
Since this tests the sortable plugin, we probably want it to be part of the test suite for useTableSortable to scale. That way when we add context menu actions to more plugins, their menu behaviors are tested colocated with that plugin's behaviors.
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
2 participants
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
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. withuseTableSortableenabled, 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.
TableContextActiontype and two optional, backward-compatibleTablePluginmethods:getHeaderContextActions(column, columnIndex)andgetRowContextActions(item, rowIndex).tableContextMenu.tsx:collectHeaderContextActions/collectRowContextActions(aggregate across plugins) +wrapInTableContextMenu(maps actions →ContextMenuoptions, 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.BaseTablecollects + wraps at the header render path;useBaseTablePluginsregisters the two new plugin keys.useTableSortableadds 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
getRowContextActionsand the row collector is built/exported, but row-menu rendering into<tr>is a follow-up: Astryx'sContextMenuwraps its trigger in a<div>, invalid between<tbody>and<tr>. Needs anasChild/Slot option onContextMenu(or anonContextMenu-on-<tr>approach). Header menus — the sortable use case — work fully.Demo
Surfaces on the existing
Core → TableSortablestories (no new stories) — right-click any sortable column header.Validation
pnpm -F @astryxdesign/core build✅ (incl. tsc),eslint✅tableContextMenu.test.tsx(6 tests: aggregation, header render, no-op passthrough, sortable asc/desc/clear). Full Table suite: 318 pass.[feat]).