From 83e5c761870ac7a83f7866a3d14d2c316de59575 Mon Sep 17 00:00:00 2001 From: anna Date: Thu, 22 Oct 2026 15:16:00 -0500 Subject: [PATCH 01/20] refactor: for mui tables: move delete into row actions and click on row to edit --- .../mui/tables/components/row-actions-menu.js | 18 +- src/components/mui/tables/mui-table/index.js | 158 ++++-------------- src/i18n/en.json | 1 + 3 files changed, 46 insertions(+), 131 deletions(-) diff --git a/src/components/mui/tables/components/row-actions-menu.js b/src/components/mui/tables/components/row-actions-menu.js index 1fc1d4f8..b5773f7a 100644 --- a/src/components/mui/tables/components/row-actions-menu.js +++ b/src/components/mui/tables/components/row-actions-menu.js @@ -13,9 +13,11 @@ import * as React from "react"; import PropTypes from "prop-types"; +import T from "i18n-react/dist/i18n-react"; import IconButton from "@mui/material/IconButton"; import Menu from "@mui/material/Menu"; import MenuItem from "@mui/material/MenuItem"; +import Tooltip from "@mui/material/Tooltip"; import MoreVertIcon from "@mui/icons-material/MoreVert"; const RowActionsMenu = ({ actions }) => { @@ -25,13 +27,15 @@ const RowActionsMenu = ({ actions }) => { return ( <> - setAnchorEl(e.currentTarget)} - data-testid="row-actions-menu-trigger" - > - - + + setAnchorEl(e.currentTarget)} + data-testid="row-actions-menu-trigger" + > + + + = 2; - const collapseBreakpoint = getActionsMenuBreakpoint(columns.length); + (onDelete ? 1 : 0) + (onArchive ? 1 : 0) + (onSelect ? 1 : 0); - const totalColumnsCount = - columns.length + activeActionsCount + (collapseActions ? 1 : 0); + const totalColumnsCount = columns.length + activeActionsCount; const {sortCol, sortDir} = options; @@ -94,49 +84,18 @@ const MuiTable = ({ ...(col.headSx || {}) }); - // individual action cells hide below collapseBreakpoint once actions collapse into a menu - const getIndividualActionSx = () => - collapseActions ? { display: { xs: "none", [collapseBreakpoint]: "table-cell" } } : {}; - - const menuCellSx = { display: { xs: "table-cell", [collapseBreakpoint]: "none" } }; - - const getRowActions = (row) => - [ - onEdit && { - label: T.translate("general.edit"), - onClick: () => onEdit(row), - disabled: !!(options.disableProp && row[options.disableProp]) - }, - onArchive && { - label: row.is_archived - ? T.translate("general.unarchive") - : T.translate("general.archive"), - onClick: () => onArchive(row), - disabled: !!( - options.disableProp && - options.disableProp !== "is_archived" && - row[options.disableProp] - ) - }, - onDelete && - canDelete(row) && { - label: T.translate("general.delete"), - onClick: () => handleDelete(row), - disabled: !!(options.disableProp && row[options.disableProp]) - }, - onSelect && { - label: T.translate("general.view"), - onClick: () => onSelect(row), - disabled: !!(options.disableProp && row[options.disableProp]) - } - ].filter(Boolean); - const getCellSx = (row, col) => ({ ...getColumnWidthSx(col), ...(col.cellSx || {}), ...getArchivedRowSx(row, options.disableProp) }); + // whole row edits on click; disabled rows stay inert + const getRowEditProps = (row) => + onEdit && !(options.disableProp && row[options.disableProp]) + ? { hover: true, onClick: () => onEdit(row), sx: { cursor: "pointer" } } + : {}; + const handleDelete = createDeleteHandler({ onDelete, getName, @@ -160,6 +119,7 @@ const MuiTable = ({ {/* TABLE HEADER */} + {onDelete && } {columns.map((col) => ( ))} - {onEdit && } - {onArchive && } - {onDelete && } - {onSelect && } - {collapseActions && } + {onArchive && } + {onSelect && } @@ -188,7 +145,25 @@ const MuiTable = ({ {data.map((row) => row.renderRow ? ( {row.renderRow(totalColumnsCount)} ) : ( - + + {/* Delete column */} + {onDelete && ( + ev.stopPropagation()} + > + {canDelete(row) && ( + handleDelete(row), + disabled: !!(options.disableProp && row[options.disableProp]) + }]} + /> + )} + + )} {/* Main content columns */} {columns.map((col) => ( ))} - {/* Edit column */} - {onEdit && ( - - - - onEdit(row)} - sx={{ padding: 0 }} - data-testid="action-edit" - disabled={options.disableProp && row[options.disableProp]} - > - - - - - - )} {onArchive && ( ev.stopPropagation()} className={styles.dottedBorderLeft} > )} - {/* Delete column */} - {onDelete && ( - - {canDelete(row) && ( - - - handleDelete(row)} - data-testid="action-delete" - sx={{ padding: 0 }} - disabled={options.disableProp && row[options.disableProp]} - > - - - - - )} - - )} {onSelect && ( ev.stopPropagation()} > @@ -315,15 +234,6 @@ const MuiTable = ({ )} - {collapseActions && ( - - - - )} ))} {/* Here we inject extra rows passed as children */} diff --git a/src/i18n/en.json b/src/i18n/en.json index a3ebbaa6..7e19f986 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -31,6 +31,7 @@ "edit": "Edit", "delete": "Delete", "view": "View", + "more_actions": "More actions", "cancel": "Cancel", "apply": "Apply", "n_a": "N/A", From 1dd582543d55d0d2e479cf57aef55fa4f0ad0f7c Mon Sep 17 00:00:00 2001 From: anna Date: Thu, 22 Oct 2026 16:11:26 -0500 Subject: [PATCH 02/20] refactor: move delete into a row actions menu --- .../mui/BulkEditTable/BulkEditTable.js | 10 +-- .../mui/BulkEditTable/components/Row.js | 84 ++++++------------- .../mui/tables/components/table-styles.js | 4 - 3 files changed, 30 insertions(+), 68 deletions(-) diff --git a/src/components/mui/BulkEditTable/BulkEditTable.js b/src/components/mui/BulkEditTable/BulkEditTable.js index 925c6d4b..b2be12c9 100644 --- a/src/components/mui/BulkEditTable/BulkEditTable.js +++ b/src/components/mui/BulkEditTable/BulkEditTable.js @@ -33,7 +33,7 @@ import parsePaginationPosition from "../tables/components/pagination-position"; import showConfirmDialog from "../showConfirmDialog"; import { RESPONSIVE_TABLE_SX, - getActionsMenuBreakpoint + ACTION_CELL_SX } from "../tables/components/table-styles"; const BulkEditTable = ({ @@ -71,9 +71,6 @@ const BulkEditTable = ({ reset } = useRowSelection(idKey); - const collapseActions = (onEdit ? 1 : 0) + (onDelete ? 1 : 0) >= 2; - const actionsBreakpoint = getActionsMenuBreakpoint(columns.length); - const dataIds = data.map((row) => row[idKey]).join(","); // reset selection/edit state whenever the set of rows shown changes @@ -178,6 +175,7 @@ const BulkEditTable = ({ slotProps={{ input: { "aria-label": "select all" } }} /> + {onDelete && } {columns.map((col, i) => { const sortable = !!col.sortable; @@ -196,7 +194,7 @@ const BulkEditTable = ({ ); })} - {(onEdit || onDelete) && ( + {onEdit && ( ))} diff --git a/src/components/mui/BulkEditTable/components/Row.js b/src/components/mui/BulkEditTable/components/Row.js index e20c7718..5a3a1d60 100644 --- a/src/components/mui/BulkEditTable/components/Row.js +++ b/src/components/mui/BulkEditTable/components/Row.js @@ -19,11 +19,13 @@ import TableCell from "@mui/material/TableCell"; import Checkbox from "@mui/material/Checkbox"; import IconButton from "@mui/material/IconButton"; import EditIcon from "@mui/icons-material/Edit"; -import DeleteIcon from "@mui/icons-material/Delete"; import T from "i18n-react/dist/i18n-react"; import Cell from "./Cell"; import RowActionsMenu from "../../tables/components/row-actions-menu"; -import { getColumnWidthSx } from "../../tables/components/table-styles"; +import { + getColumnWidthSx, + ACTION_CELL_SX +} from "../../tables/components/table-styles"; import styles from "../BulkEditTable.module.less"; // the 250px min-width while editing comes from the .bulkEditCol class @@ -45,24 +47,11 @@ const Row = (props) => { onFieldChange, onEdit, onDelete, - idKey, - collapseActions, - actionsBreakpoint + idKey } = props; const isEditingRow = isSelected && editEnabled; - const rowActions = [ - onEdit && { - label: T.translate("general.edit"), - onClick: () => onEdit(row) - }, - onDelete && { - label: T.translate("general.delete"), - onClick: () => onDelete(row) - } - ].filter(Boolean); - const onRowChange = (ev) => { const { value, id } = ev.target; onFieldChange(id, value); @@ -84,6 +73,16 @@ const Row = (props) => { slotProps={{ input: { "aria-label": `Select row ${row[idKey]}` } }} /> + {onDelete && ( + + onDelete(row) + }]} + /> + + )} {columns.map((col) => ( { /> ))} - {(onEdit || onDelete) && ( + {onEdit && ( - - {onEdit && ( - onEdit(row)} - sx={{ padding: 0 }} - aria-label={`Edit row ${row[idKey]}`} - > - - - )} - {onDelete && ( - onDelete(row)} - sx={{ padding: 0 }} - aria-label={`Delete row ${row[idKey]}`} - > - - - )} + + onEdit(row)} + sx={{ padding: 0 }} + aria-label={`Edit row ${row[idKey]}`} + > + + - {collapseActions && ( - - - - )} )} @@ -156,17 +130,13 @@ Row.propTypes = { onFieldChange: PropTypes.func, onEdit: PropTypes.func, onDelete: PropTypes.func, - idKey: PropTypes.string, - collapseActions: PropTypes.bool, - actionsBreakpoint: PropTypes.string + idKey: PropTypes.string }; Row.defaultProps = { idKey: "id", onEdit: null, - onDelete: null, - collapseActions: false, - actionsBreakpoint: "md" + onDelete: null }; export default Row; diff --git a/src/components/mui/tables/components/table-styles.js b/src/components/mui/tables/components/table-styles.js index 004713e8..36fa9cd3 100644 --- a/src/components/mui/tables/components/table-styles.js +++ b/src/components/mui/tables/components/table-styles.js @@ -1,4 +1,3 @@ -export const SMALL_TABLE_MAX_COLUMNS = 3; // fewer than 4 data columns = small export const DEFAULT_COLUMN_MIN_WIDTH = 40; // floor for columns with no explicit width export const ARCHIVED_CELL_SX = { @@ -19,9 +18,6 @@ export const RESPONSIVE_TABLE_SX = { tableLayout: "auto" }; -export const getActionsMenuBreakpoint = (dataColumnCount) => - dataColumnCount <= SMALL_TABLE_MAX_COLUMNS ? "md" : "lg"; - export const getColumnWidthSx = (col) => ({ minWidth: col.width ?? DEFAULT_COLUMN_MIN_WIDTH, ...(col.width && { From 7314c43793cee14f87edf1e551c29d0f7a617b7d Mon Sep 17 00:00:00 2001 From: anna Date: Thu, 22 Oct 2026 16:23:01 -0500 Subject: [PATCH 03/20] fix: move delete into a row actions menu for editable and sortable tables --- .../mui/tables/editable-table/index.js | 41 +++++++++---------- .../mui-table-sortable-v2.js | 31 ++++++-------- .../mui/tables/sortable-table/index.js | 31 ++++++-------- 3 files changed, 46 insertions(+), 57 deletions(-) diff --git a/src/components/mui/tables/editable-table/index.js b/src/components/mui/tables/editable-table/index.js index 8563ff42..e3bdde4a 100644 --- a/src/components/mui/tables/editable-table/index.js +++ b/src/components/mui/tables/editable-table/index.js @@ -23,13 +23,13 @@ import TableHead from "@mui/material/TableHead"; import TableRow from "@mui/material/TableRow"; import { IconButton, TextField } from "@mui/material"; import EditIcon from "@mui/icons-material/Edit"; -import DeleteIcon from "@mui/icons-material/Delete"; import styles from "../components/styles.module.less"; import TableCellContent from "../components/table-cell-content"; import TableShell from "../components/table-shell"; import createDeleteHandler from "../components/create-delete-handler"; import SortableHeaderContent from "../components/sortable-header-content"; +import RowActionsMenu from "../components/row-actions-menu"; import { RESPONSIVE_TABLE_SX, getColumnWidthSx, @@ -210,6 +210,9 @@ const MuiTableEditable = ({ {/* TABLE HEADER */} + {onDelete && ( + + )} {columns.map((col) => ( )} - {onDelete && ( - - )} {/* TABLE BODY */} {data.map((row) => ( + {onDelete && ( + + handleDelete(row) + }]} + /> + + )} {columns.map((col) => ( )} - {onDelete && ( - - handleDelete(row)} - size="small" - aria-label={T.translate("general.delete")} - > - - - - )} ))} diff --git a/src/components/mui/tables/sortable-table-v2/mui-table-sortable-v2.js b/src/components/mui/tables/sortable-table-v2/mui-table-sortable-v2.js index aaf1225a..0ecc463c 100644 --- a/src/components/mui/tables/sortable-table-v2/mui-table-sortable-v2.js +++ b/src/components/mui/tables/sortable-table-v2/mui-table-sortable-v2.js @@ -25,7 +25,6 @@ import Paper from "@mui/material/Paper"; import UnfoldMoreIcon from "@mui/icons-material/UnfoldMore"; import { IconButton } from "@mui/material"; import EditIcon from "@mui/icons-material/Edit"; -import DeleteIcon from "@mui/icons-material/Delete"; import { DndContext } from "@dnd-kit/core"; import { SortableContext, verticalListSortingStrategy } from "@dnd-kit/sortable"; import { visuallyHidden } from "@mui/utils"; @@ -35,6 +34,7 @@ import styles from "./styles.module.less"; import showConfirmDialog from "../../showConfirmDialog"; import SortableRow from "./sortable-row"; import TableCellContent from "../components/table-cell-content"; +import RowActionsMenu from "../components/row-actions-menu"; import CustomTablePagination from "../components/CustomTablePagination"; import parsePaginationPosition from "../components/pagination-position"; import useDndKitReorder from "../../DragNDropList/hooks/useDndKitReorder"; @@ -142,6 +142,7 @@ const MuiTableSortableV2 = ({ {/* TABLE HEADER */} + {onDelete && } {columns.map((col) => ( ))} {onEdit && } - {onDelete && } {onReorder && } @@ -195,6 +195,17 @@ const MuiTableSortableV2 = ({ > {({ dragHandleProps }) => ( <> + {/* Delete column */} + {onDelete && ( + + handleDelete(row) + }]} + /> + + )} {/* Main content columns */} {columns.map((col) => ( )} - {/* Delete column */} - {onDelete && ( - - handleDelete(row)} - sx={{ padding: 0 }} - > - - - - )} {/* Re order column */} {onReorder && ( + {onDelete && } {columns.map((col) => ( ))} {onEdit && } - {onDelete && } {onReorder && } @@ -163,6 +163,17 @@ const MuiTableSortable = ({ }) }} > + {/* Delete column */} + {onDelete && ( + + handleDelete(row) + }]} + /> + + )} {/* Main content columns */} {columns.map((col) => ( )} - {/* Delete column */} - {onDelete && ( - - handleDelete(row)} - sx={{ padding: 0 }} - > - - - - )} {/* Re order column */} {onReorder && ( Date: Thu, 22 Oct 2026 16:39:35 -0500 Subject: [PATCH 04/20] fix: add tests for delete/edit button rearrangement --- .../mui/__tests__/mui-table-editable.test.js | 15 ++--- .../__tests__/mui-table-sortable-v2.test.js | 10 ++-- .../mui/__tests__/mui-table-sortable.test.js | 5 +- .../mui/__tests__/mui-table.test.js | 55 ++++++++++++------- 4 files changed, 48 insertions(+), 37 deletions(-) diff --git a/src/components/mui/__tests__/mui-table-editable.test.js b/src/components/mui/__tests__/mui-table-editable.test.js index 8dd3f6a0..ae7ed230 100644 --- a/src/components/mui/__tests__/mui-table-editable.test.js +++ b/src/components/mui/__tests__/mui-table-editable.test.js @@ -30,11 +30,12 @@ jest.mock("@mui/material/IconButton", () => { const React = require("react"); return { __esModule: true, - default: ({ children, onClick, ...rest }) => ( - - ) + )) }; }); jest.mock("@mui/material/ButtonBase/TouchRipple", () => ({ @@ -211,8 +212,8 @@ describe("MuiTableEditable", () => { const user = userEvent.setup(); const { onDelete } = setup(); showConfirmDialog.mockResolvedValueOnce(true); // ✅ use the exported mock - const [btn] = screen.getAllByLabelText("general.delete"); - await user.click(btn); + await user.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await user.click(screen.getByRole("menuitem", { name: "general.delete" })); expect(showConfirmDialog).toHaveBeenCalled(); expect(onDelete).toHaveBeenCalledWith(1); }); @@ -221,8 +222,8 @@ describe("MuiTableEditable", () => { const user = userEvent.setup(); const { onDelete } = setup(); showConfirmDialog.mockResolvedValueOnce(false); - const [btn] = screen.getAllByLabelText("general.delete"); - await user.click(btn); + await user.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await user.click(screen.getByRole("menuitem", { name: "general.delete" })); expect(showConfirmDialog).toHaveBeenCalled(); expect(onDelete).not.toHaveBeenCalled(); }); diff --git a/src/components/mui/__tests__/mui-table-sortable-v2.test.js b/src/components/mui/__tests__/mui-table-sortable-v2.test.js index f1a7ae39..6ad6fa12 100644 --- a/src/components/mui/__tests__/mui-table-sortable-v2.test.js +++ b/src/components/mui/__tests__/mui-table-sortable-v2.test.js @@ -132,9 +132,8 @@ describe("MuiTableSortableV2", () => { const onDelete = jest.fn(); showConfirmDialog.mockResolvedValueOnce(true); setup({ onDelete }); - const buttons = within(screen.getByRole("table")).getAllByRole("button"); - // buttons[0] is the sort label button; buttons[1] is the first delete button (row 1) - await userEvent.click(buttons[1]); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await userEvent.click(screen.getByRole("menuitem", { name: "general.delete" })); await new Promise((r) => setTimeout(r, 0)); expect(showConfirmDialog).toHaveBeenCalled(); expect(onDelete).toHaveBeenCalledWith(1); @@ -239,9 +238,8 @@ describe("MuiTableSortableV2", () => { 2 ); - const buttons = within(screen.getByRole("table")).getAllByRole("button"); - // buttons[0] is the sort label button; buttons[1] is the first delete button (row 1) - await userEvent.click(buttons[1]); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await userEvent.click(screen.getByRole("menuitem", { name: "general.delete" })); await new Promise((r) => setTimeout(r, 0)); expect(onDelete).toHaveBeenCalledWith("a-1"); }); diff --git a/src/components/mui/__tests__/mui-table-sortable.test.js b/src/components/mui/__tests__/mui-table-sortable.test.js index fa5ff099..69fc1b71 100644 --- a/src/components/mui/__tests__/mui-table-sortable.test.js +++ b/src/components/mui/__tests__/mui-table-sortable.test.js @@ -113,9 +113,8 @@ describe("MuiTableSortable", () => { const onDelete = jest.fn(); showConfirmDialog.mockResolvedValueOnce(true); setup({ onDelete }); - const buttons = within(screen.getByRole("table")).getAllByRole("button"); - // buttons[0] is the sort label button; buttons[1] is the first delete button (row 1) - await userEvent.click(buttons[1]); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await userEvent.click(screen.getByRole("menuitem", { name: "general.delete" })); await new Promise((r) => setTimeout(r, 0)); expect(showConfirmDialog).toHaveBeenCalled(); expect(onDelete).toHaveBeenCalledWith(1); diff --git a/src/components/mui/__tests__/mui-table.test.js b/src/components/mui/__tests__/mui-table.test.js index b5c06775..53f9cabc 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -74,23 +74,32 @@ describe("MuiTable", () => { expect(screen.getByText("mui_table.no_items")).toBeInTheDocument(); }); - test("renders edit button when onEdit is provided", () => { - setup({ onEdit: jest.fn() }); - expect(screen.getAllByTestId("action-edit")).toHaveLength(2); + test("calls onEdit with the row when the row is clicked", async () => { + const onEdit = jest.fn(); + setup({ onEdit }); + await userEvent.click(screen.getByText("Bob")); + expect(onEdit).toHaveBeenCalledTimes(1); + expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 2 })); }); - test("calls onEdit when edit button is clicked", async () => { + test("action clicks do not trigger the row's onEdit", async () => { const onEdit = jest.fn(); - setup({ onEdit }); - await userEvent.click(screen.getAllByTestId("action-edit")[0]); - expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); + const onSelect = jest.fn(); + showConfirmDialog.mockResolvedValueOnce(false); + setup({ onEdit, onSelect, onDelete: jest.fn() }); + await userEvent.click(screen.getAllByTestId("action-select")[0]); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await userEvent.click(screen.getByRole("menuitem", { name: "general.delete" })); + expect(onSelect).toHaveBeenCalledTimes(1); + expect(onEdit).not.toHaveBeenCalled(); }); test("calls showConfirmDialog and then onDelete when delete confirmed", async () => { const onDelete = jest.fn(); showConfirmDialog.mockResolvedValueOnce(true); setup({ onDelete }); - await userEvent.click(screen.getAllByTestId("action-delete")[0]); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await userEvent.click(screen.getByRole("menuitem", { name: "general.delete" })); await new Promise((r) => setTimeout(r, 0)); expect(showConfirmDialog).toHaveBeenCalled(); expect(onDelete).toHaveBeenCalledWith(1); @@ -100,7 +109,8 @@ describe("MuiTable", () => { const onDelete = jest.fn(); showConfirmDialog.mockResolvedValueOnce(false); setup({ onDelete }); - await userEvent.click(screen.getAllByTestId("action-delete")[0]); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await userEvent.click(screen.getByRole("menuitem", { name: "general.delete" })); await new Promise((r) => setTimeout(r, 0)); expect(onDelete).not.toHaveBeenCalled(); }); @@ -130,10 +140,10 @@ describe("MuiTable", () => { expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); }); - test("hides delete button when canDelete returns false", () => { + test("hides delete menu when canDelete returns false", () => { const onDelete = jest.fn(); setup({ onDelete, canDelete: (row) => row.id !== 1 }); - expect(screen.getAllByTestId("action-delete")).toHaveLength(1); + expect(screen.getAllByTestId("row-actions-menu-trigger")).toHaveLength(1); }); test("renders pagination (top and bottom) when perPage and currentPage are set", () => { @@ -171,18 +181,21 @@ describe("MuiTable", () => { ]; const disabledOptions = { sortCol: "", sortDir: 1, disableProp: "is_disabled" }; - test("disables edit button for rows where disableProp is truthy", () => { - setup({ data: disabledData, onEdit: jest.fn(), options: disabledOptions }); - const buttons = screen.getAllByTestId("action-edit"); - expect(buttons[0]).toBeDisabled(); - expect(buttons[1]).not.toBeDisabled(); + test("row click skips onEdit for rows where disableProp is truthy", async () => { + const onEdit = jest.fn(); + setup({ data: disabledData, onEdit, options: disabledOptions }); + await userEvent.click(screen.getByText("Alice")); + await userEvent.click(screen.getByText("Bob")); + expect(onEdit).toHaveBeenCalledTimes(1); + expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 2 })); }); - test("disables delete button for rows where disableProp is truthy", () => { + test("disables delete menu item for rows where disableProp is truthy", async () => { setup({ data: disabledData, onDelete: jest.fn(), options: disabledOptions }); - const buttons = screen.getAllByTestId("action-delete"); - expect(buttons[0]).toBeDisabled(); - expect(buttons[1]).not.toBeDisabled(); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + expect( + screen.getByRole("menuitem", { name: "general.delete" }) + ).toHaveAttribute("aria-disabled", "true"); }); test("disables select button for rows where disableProp is truthy", () => { @@ -280,7 +293,7 @@ describe("MuiTable", () => { test("passes the total column count, including action columns", () => { setup({ data: [customRow], onDelete: jest.fn() }); expect(customRow.renderRow).toHaveBeenCalledWith(columns.length + 1); - expect(screen.queryByTestId("action-delete")).not.toBeInTheDocument(); + expect(screen.queryByTestId("row-actions-menu-trigger")).not.toBeInTheDocument(); }); }); }); From 7c0f24a0a6f2eebfe1d01aa3ab1273c28e6640db Mon Sep 17 00:00:00 2001 From: anna Date: Thu, 22 Oct 2026 16:41:57 -0500 Subject: [PATCH 05/20] fix: adjust the column actions on bulk edit table --- .../mui/BulkEditTable/BulkEditTable.js | 2 +- .../mui/BulkEditTable/components/Row.js | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/components/mui/BulkEditTable/BulkEditTable.js b/src/components/mui/BulkEditTable/BulkEditTable.js index b2be12c9..fa3a525a 100644 --- a/src/components/mui/BulkEditTable/BulkEditTable.js +++ b/src/components/mui/BulkEditTable/BulkEditTable.js @@ -161,6 +161,7 @@ const BulkEditTable = ({ + {onDelete && } - {onDelete && } {columns.map((col, i) => { const sortable = !!col.sortable; diff --git a/src/components/mui/BulkEditTable/components/Row.js b/src/components/mui/BulkEditTable/components/Row.js index 5a3a1d60..5b26e606 100644 --- a/src/components/mui/BulkEditTable/components/Row.js +++ b/src/components/mui/BulkEditTable/components/Row.js @@ -58,7 +58,17 @@ const Row = (props) => { }; return ( - + + {onDelete && ( + + onDelete(row) + }]} + /> + + )} { slotProps={{ input: { "aria-label": `Select row ${row[idKey]}` } }} /> - {onDelete && ( - - onDelete(row) - }]} - /> - - )} {columns.map((col) => ( Date: Thu, 22 Oct 2026 22:12:21 -0500 Subject: [PATCH 06/20] fix: a button click doesn't trigger an edit on mui table --- .../mui/__tests__/mui-table.test.js | 26 +++++++++++++++++++ src/components/mui/tables/mui-table/index.js | 9 ++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/components/mui/__tests__/mui-table.test.js b/src/components/mui/__tests__/mui-table.test.js index 53f9cabc..3ac3454a 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -296,4 +296,30 @@ describe("MuiTable", () => { expect(screen.queryByTestId("row-actions-menu-trigger")).not.toBeInTheDocument(); }); }); + + test("clicking a button inside a cell runs its handler, not onEdit", async () => { + const onEdit = jest.fn(); + const onManage = jest.fn(); + setup({ + onEdit, + columns: [ + ...columns, + { + columnKey: "manage", + header: "", + render: (row) => ( + + ) + } + ] + }); + + await userEvent.click(screen.getAllByRole("button", { name: "Manage" })[0]); + + expect(onManage).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); + expect(onEdit).not.toHaveBeenCalled(); + }); + }); diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index 05738ce7..88d48c47 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -93,7 +93,14 @@ const MuiTable = ({ // whole row edits on click; disabled rows stay inert const getRowEditProps = (row) => onEdit && !(options.disableProp && row[options.disableProp]) - ? { hover: true, onClick: () => onEdit(row), sx: { cursor: "pointer" } } + ? { + hover: true, + onClick: (ev) => { + // ignore clicks on controls rendered inside cells + if (!ev.target.closest("button, a, input, [role=button]")) onEdit(row); + }, + sx: { cursor: "pointer" } + } : {}; const handleDelete = createDeleteHandler({ From a69ab2fec4de06f9bf8baa2ca7bc61807f49941f Mon Sep 17 00:00:00 2001 From: anna Date: Fri, 23 Oct 2026 15:39:19 -0500 Subject: [PATCH 07/20] fix: make row click editing keyboard accesible --- src/components/mui/__tests__/mui-table.test.js | 9 +++++++++ src/components/mui/tables/mui-table/index.js | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/src/components/mui/__tests__/mui-table.test.js b/src/components/mui/__tests__/mui-table.test.js index 3ac3454a..f62c8d8c 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -82,6 +82,15 @@ describe("MuiTable", () => { expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 2 })); }); + test("calls onEdit when Enter is pressed on a focused row", async () => { + const onEdit = jest.fn(); + setup({ onEdit }); + screen.getByText("Bob").closest("tr").focus(); + await userEvent.keyboard("{Enter}"); + expect(onEdit).toHaveBeenCalledTimes(1); + expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 2 })); + }); + test("action clicks do not trigger the row's onEdit", async () => { const onEdit = jest.fn(); const onSelect = jest.fn(); diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index 88d48c47..4d0b547c 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -95,10 +95,15 @@ const MuiTable = ({ onEdit && !(options.disableProp && row[options.disableProp]) ? { hover: true, + tabIndex: 0, onClick: (ev) => { // ignore clicks on controls rendered inside cells if (!ev.target.closest("button, a, input, [role=button]")) onEdit(row); }, + // Enter on the focused row itself edits, for keyboard users + onKeyDown: (ev) => { + if (ev.key === "Enter" && ev.target === ev.currentTarget) onEdit(row); + }, sx: { cursor: "pointer" } } : {}; From 2bc98dcd4114dd077736da82ecffd5f9cb3b16c5 Mon Sep 17 00:00:00 2001 From: anna Date: Fri, 23 Oct 2026 16:05:13 -0500 Subject: [PATCH 08/20] fix: share delete action across tables, actions disabled on archive, better keyboard user experience --- .../__tests__/BulkEditTable.test.js | 33 +++++++++++++++++++ .../mui/BulkEditTable/components/Row.js | 12 +++---- .../mui/__tests__/mui-table.test.js | 9 +++-- .../mui/tables/components/row-actions-menu.js | 30 ++++++++++++----- .../mui/tables/editable-table/index.js | 7 ++-- src/components/mui/tables/mui-table/index.js | 19 +++++++---- .../mui-table-sortable-v2.js | 7 ++-- .../mui/tables/sortable-table/index.js | 7 ++-- 8 files changed, 81 insertions(+), 43 deletions(-) diff --git a/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js b/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js index c9401ead..7be95bf9 100644 --- a/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js +++ b/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js @@ -21,12 +21,18 @@ import { waitFor } from "@testing-library/react"; import BulkEditTable from "../BulkEditTable"; +import showConfirmDialog from "../../showConfirmDialog"; jest.mock("i18n-react/dist/i18n-react", () => ({ __esModule: true, default: { translate: (key) => key } })); +jest.mock("../../showConfirmDialog", () => ({ + __esModule: true, + default: jest.fn() +})); + describe("BulkEditTable", () => { const baseProps = { options: { @@ -48,6 +54,33 @@ describe("BulkEditTable", () => { jest.clearAllMocks(); }); + test("delete from the row menu confirms, then passes the row's idKey value", async () => { + const user = userEvent.setup(); + const onDelete = jest.fn(); + showConfirmDialog.mockResolvedValueOnce(true); + + render( + + ); + + await user.click(screen.getAllByTestId("row-actions-menu-trigger")[1]); + await user.click(screen.getByRole("menuitem", { name: "general.delete" })); + + await waitFor(() => expect(onDelete).toHaveBeenCalledWith("a-2")); + expect(showConfirmDialog).toHaveBeenCalledWith( + expect.objectContaining({ text: "general.row_remove_warning Event 2" }) + ); + }); + test("applies bulk updates to selected rows", async () => { const user = userEvent.setup(); const onUpdate = jest.fn(() => Promise.resolve()); diff --git a/src/components/mui/BulkEditTable/components/Row.js b/src/components/mui/BulkEditTable/components/Row.js index 5b26e606..f4e04cb2 100644 --- a/src/components/mui/BulkEditTable/components/Row.js +++ b/src/components/mui/BulkEditTable/components/Row.js @@ -19,9 +19,10 @@ import TableCell from "@mui/material/TableCell"; import Checkbox from "@mui/material/Checkbox"; import IconButton from "@mui/material/IconButton"; import EditIcon from "@mui/icons-material/Edit"; -import T from "i18n-react/dist/i18n-react"; import Cell from "./Cell"; -import RowActionsMenu from "../../tables/components/row-actions-menu"; +import RowActionsMenu, { + getDeleteAction +} from "../../tables/components/row-actions-menu"; import { getColumnWidthSx, ACTION_CELL_SX @@ -61,12 +62,7 @@ const Row = (props) => { {onDelete && ( - onDelete(row) - }]} - /> + onDelete(row))]} /> )} { expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 2 })); }); - test("disables delete menu item for rows where disableProp is truthy", async () => { + test("disables the row menu for rows where disableProp is truthy", () => { setup({ data: disabledData, onDelete: jest.fn(), options: disabledOptions }); - await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); - expect( - screen.getByRole("menuitem", { name: "general.delete" }) - ).toHaveAttribute("aria-disabled", "true"); + const triggers = screen.getAllByTestId("row-actions-menu-trigger"); + expect(triggers[0]).toBeDisabled(); + expect(triggers[1]).not.toBeDisabled(); }); test("disables select button for rows where disableProp is truthy", () => { diff --git a/src/components/mui/tables/components/row-actions-menu.js b/src/components/mui/tables/components/row-actions-menu.js index b5773f7a..3b5c4eae 100644 --- a/src/components/mui/tables/components/row-actions-menu.js +++ b/src/components/mui/tables/components/row-actions-menu.js @@ -20,6 +20,14 @@ import MenuItem from "@mui/material/MenuItem"; import Tooltip from "@mui/material/Tooltip"; import MoreVertIcon from "@mui/icons-material/MoreVert"; +// shared Delete entry so every table's menu labels and styles it the same +export const getDeleteAction = (onClick, disabled = false) => ({ + label: T.translate("general.delete"), + onClick, + disabled, + destructive: true +}); + const RowActionsMenu = ({ actions }) => { const [anchorEl, setAnchorEl] = React.useState(null); @@ -28,13 +36,17 @@ const RowActionsMenu = ({ actions }) => { return ( <> - setAnchorEl(e.currentTarget)} - data-testid="row-actions-menu-trigger" - > - - + + setAnchorEl(e.currentTarget)} + data-testid="row-actions-menu-trigger" + aria-label={T.translate("general.more_actions")} + disabled={actions.every((action) => action.disabled)} + > + + + { setAnchorEl(null); action.onClick(); }} + sx={action.destructive ? { color: "error.main" } : undefined} > {action.label} @@ -63,7 +76,8 @@ RowActionsMenu.propTypes = { PropTypes.shape({ label: PropTypes.string.isRequired, onClick: PropTypes.func.isRequired, - disabled: PropTypes.bool + disabled: PropTypes.bool, + destructive: PropTypes.bool }) ).isRequired }; diff --git a/src/components/mui/tables/editable-table/index.js b/src/components/mui/tables/editable-table/index.js index e3bdde4a..2665e117 100644 --- a/src/components/mui/tables/editable-table/index.js +++ b/src/components/mui/tables/editable-table/index.js @@ -29,7 +29,7 @@ import TableCellContent from "../components/table-cell-content"; import TableShell from "../components/table-shell"; import createDeleteHandler from "../components/create-delete-handler"; import SortableHeaderContent from "../components/sortable-header-content"; -import RowActionsMenu from "../components/row-actions-menu"; +import RowActionsMenu, { getDeleteAction } from "../components/row-actions-menu"; import { RESPONSIVE_TABLE_SX, getColumnWidthSx, @@ -261,10 +261,7 @@ const MuiTableEditable = ({ }} > handleDelete(row) - }]} + actions={[getDeleteAction(() => handleDelete(row))]} /> )} diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index 4d0b547c..1069ec68 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -32,7 +32,7 @@ import TableCellContent from "../components/table-cell-content"; import TableShell from "../components/table-shell"; import createDeleteHandler from "../components/create-delete-handler"; import SortableHeaderContent from "../components/sortable-header-content"; -import RowActionsMenu from "../components/row-actions-menu"; +import RowActionsMenu, { getDeleteAction } from "../components/row-actions-menu"; import { RESPONSIVE_TABLE_SX, getColumnWidthSx, @@ -104,7 +104,11 @@ const MuiTable = ({ onKeyDown: (ev) => { if (ev.key === "Enter" && ev.target === ev.currentTarget) onEdit(row); }, - sx: { cursor: "pointer" } + // browsers don't draw focus outlines on rows, so shade it instead + sx: { + cursor: "pointer", + "&:focus-visible": { backgroundColor: "action.selected" } + } } : {}; @@ -167,11 +171,12 @@ const MuiTable = ({ > {canDelete(row) && ( handleDelete(row), - disabled: !!(options.disableProp && row[options.disableProp]) - }]} + actions={[ + getDeleteAction( + () => handleDelete(row), + !!(options.disableProp && row[options.disableProp]) + ) + ]} /> )} diff --git a/src/components/mui/tables/sortable-table-v2/mui-table-sortable-v2.js b/src/components/mui/tables/sortable-table-v2/mui-table-sortable-v2.js index 0ecc463c..085d6f29 100644 --- a/src/components/mui/tables/sortable-table-v2/mui-table-sortable-v2.js +++ b/src/components/mui/tables/sortable-table-v2/mui-table-sortable-v2.js @@ -34,7 +34,7 @@ import styles from "./styles.module.less"; import showConfirmDialog from "../../showConfirmDialog"; import SortableRow from "./sortable-row"; import TableCellContent from "../components/table-cell-content"; -import RowActionsMenu from "../components/row-actions-menu"; +import RowActionsMenu, { getDeleteAction } from "../components/row-actions-menu"; import CustomTablePagination from "../components/CustomTablePagination"; import parsePaginationPosition from "../components/pagination-position"; import useDndKitReorder from "../../DragNDropList/hooks/useDndKitReorder"; @@ -199,10 +199,7 @@ const MuiTableSortableV2 = ({ {onDelete && ( handleDelete(row) - }]} + actions={[getDeleteAction(() => handleDelete(row))]} /> )} diff --git a/src/components/mui/tables/sortable-table/index.js b/src/components/mui/tables/sortable-table/index.js index 3308b4ce..f1670b17 100644 --- a/src/components/mui/tables/sortable-table/index.js +++ b/src/components/mui/tables/sortable-table/index.js @@ -29,7 +29,7 @@ import TableCellContent from "../components/table-cell-content"; import TableShell from "../components/table-shell"; import createDeleteHandler from "../components/create-delete-handler"; import SortableHeaderContent from "../components/sortable-header-content"; -import RowActionsMenu from "../components/row-actions-menu"; +import RowActionsMenu, { getDeleteAction } from "../components/row-actions-menu"; import { RESPONSIVE_TABLE_SX, getColumnWidthSx, @@ -167,10 +167,7 @@ const MuiTableSortable = ({ {onDelete && ( handleDelete(row) - }]} + actions={[getDeleteAction(() => handleDelete(row))]} /> )} From ccd613e8c28fd245f44beb08ae9f79c11eeb8e65 Mon Sep 17 00:00:00 2001 From: anna Date: Fri, 23 Oct 2026 16:11:37 -0500 Subject: [PATCH 09/20] fix: cleanup inconsistencies, unneccesary components, and misleading func name --- .../mui/BulkEditTable/components/Row.js | 19 ++++++++----------- src/components/mui/tables/mui-table/index.js | 6 +++--- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/components/mui/BulkEditTable/components/Row.js b/src/components/mui/BulkEditTable/components/Row.js index f4e04cb2..7a2464c4 100644 --- a/src/components/mui/BulkEditTable/components/Row.js +++ b/src/components/mui/BulkEditTable/components/Row.js @@ -13,7 +13,6 @@ import React from "react"; import PropTypes from "prop-types"; -import Box from "@mui/material/Box"; import TableRow from "@mui/material/TableRow"; import TableCell from "@mui/material/TableCell"; import Checkbox from "@mui/material/Checkbox"; @@ -100,16 +99,14 @@ const Row = (props) => { className={`${styles.actionColumn} ${styles.dottedBorderLeft}`} sx={{ backgroundColor: "#fff" }} > - - onEdit(row)} - sx={{ padding: 0 }} - aria-label={`Edit row ${row[idKey]}`} - > - - - + onEdit(row)} + sx={{ padding: 0 }} + aria-label={`Edit row ${row[idKey]}`} + > + + )} diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index 1069ec68..59190fe5 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -72,10 +72,10 @@ const MuiTable = ({ disableProp: null, // disableProp is the prop that will disable the row ...userOptions }; - const activeActionsCount = + const actionColumnsCount = (onDelete ? 1 : 0) + (onArchive ? 1 : 0) + (onSelect ? 1 : 0); - const totalColumnsCount = columns.length + activeActionsCount; + const totalColumnsCount = columns.length + actionColumnsCount; const {sortCol, sortDir} = options; @@ -196,8 +196,8 @@ const MuiTable = ({ ev.stopPropagation()} className={styles.dottedBorderLeft} + onClick={(ev) => ev.stopPropagation()} > Date: Thu, 29 Oct 2026 13:09:12 -0500 Subject: [PATCH 10/20] fix: guard row clicks from registering from a popup click --- .../mui/__tests__/mui-table.test.js | 25 +++++++++++++++++++ src/components/mui/tables/mui-table/index.js | 2 ++ 2 files changed, 27 insertions(+) diff --git a/src/components/mui/__tests__/mui-table.test.js b/src/components/mui/__tests__/mui-table.test.js index e70df624..f06b9bec 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -25,6 +25,8 @@ import React from "react"; import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import "@testing-library/jest-dom"; +import Menu from "@mui/material/Menu"; +import MenuItem from "@mui/material/MenuItem"; import MuiTable from "../tables/mui-table"; import showConfirmDialog from "../showConfirmDialog"; @@ -330,4 +332,27 @@ describe("MuiTable", () => { expect(onEdit).not.toHaveBeenCalled(); }); + test("clicks inside a popup opened from a cell do not trigger onEdit", async () => { + const onEdit = jest.fn(); + setup({ + onEdit, + columns: [ + ...columns, + { + columnKey: "pick", + header: "", + render: () => ( + + Option A + + ) + } + ] + }); + + await userEvent.click(screen.getAllByRole("menuitem", { name: "Option A" })[0]); + + expect(onEdit).not.toHaveBeenCalled(); + }); + }); diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index 59190fe5..81f50c06 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -97,6 +97,8 @@ const MuiTable = ({ hover: true, tabIndex: 0, onClick: (ev) => { + // ignore clicks from popups (menus, dialogs) opened by a cell + if (!ev.currentTarget.contains(ev.target)) return; // ignore clicks on controls rendered inside cells if (!ev.target.closest("button, a, input, [role=button]")) onEdit(row); }, From 042bd0884d151e026f1717736641aeb3aec984d1 Mon Sep 17 00:00:00 2001 From: anna Date: Thu, 29 Oct 2026 13:19:49 -0500 Subject: [PATCH 11/20] fix: move checkboxes to be the first column in a bulkedittable --- src/components/mui/BulkEditTable/BulkEditTable.js | 2 +- src/components/mui/BulkEditTable/components/Row.js | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/mui/BulkEditTable/BulkEditTable.js b/src/components/mui/BulkEditTable/BulkEditTable.js index fa3a525a..b2be12c9 100644 --- a/src/components/mui/BulkEditTable/BulkEditTable.js +++ b/src/components/mui/BulkEditTable/BulkEditTable.js @@ -161,7 +161,6 @@ const BulkEditTable = ({
- {onDelete && } + {onDelete && } {columns.map((col, i) => { const sortable = !!col.sortable; diff --git a/src/components/mui/BulkEditTable/components/Row.js b/src/components/mui/BulkEditTable/components/Row.js index 7a2464c4..685fdbe2 100644 --- a/src/components/mui/BulkEditTable/components/Row.js +++ b/src/components/mui/BulkEditTable/components/Row.js @@ -59,11 +59,6 @@ const Row = (props) => { return ( - {onDelete && ( - - onDelete(row))]} /> - - )} { slotProps={{ input: { "aria-label": `Select row ${row[idKey]}` } }} /> + {onDelete && ( + + onDelete(row))]} /> + + )} {columns.map((col) => ( Date: Thu, 29 Oct 2026 13:49:10 -0500 Subject: [PATCH 12/20] fix: expand edit click guard --- .../mui/__tests__/mui-table.test.js | 45 ++++++++++++++++++- src/components/mui/tables/mui-table/index.js | 7 ++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/components/mui/__tests__/mui-table.test.js b/src/components/mui/__tests__/mui-table.test.js index f06b9bec..95f7b110 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -22,11 +22,12 @@ jest.mock("../showConfirmDialog", () => ({ })); import React from "react"; -import { render, screen } from "@testing-library/react"; +import { render, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import "@testing-library/jest-dom"; import Menu from "@mui/material/Menu"; import MenuItem from "@mui/material/MenuItem"; +import Select from "@mui/material/Select"; import MuiTable from "../tables/mui-table"; import showConfirmDialog from "../showConfirmDialog"; @@ -332,6 +333,48 @@ describe("MuiTable", () => { expect(onEdit).not.toHaveBeenCalled(); }); + test.each([ + ["a textarea", () =>