From 347ed3b0ae49d58dd931b502676a3ac749de8a02 Mon Sep 17 00:00:00 2001 From: anna Date: Fri, 30 Oct 2026 16:50:04 -0500 Subject: [PATCH 1/7] refactor: replace row edit/delete icons with a row actions menu --- .../mui/BulkEditTable/BulkEditTable.js | 16 +- .../__tests__/BulkEditTable.test.js | 51 +++++++ .../mui/BulkEditTable/components/Row.js | 73 ++------- .../mui/__tests__/mui-table-editable.test.js | 38 +++-- .../__tests__/mui-table-sortable-v2.test.js | 33 +++-- .../mui/__tests__/mui-table-sortable.test.js | 28 ++-- .../mui/__tests__/mui-table.test.js | 73 ++++++--- .../mui/tables/components/row-actions-menu.js | 60 ++++++-- .../mui/tables/components/table-styles.js | 4 - .../mui/tables/editable-table/index.js | 43 ++---- src/components/mui/tables/mui-table/index.js | 140 +++--------------- .../mui-table-sortable-v2.js | 42 ++---- .../mui/tables/sortable-table/index.js | 39 ++--- src/i18n/en.json | 1 + 14 files changed, 284 insertions(+), 357 deletions(-) diff --git a/src/components/mui/BulkEditTable/BulkEditTable.js b/src/components/mui/BulkEditTable/BulkEditTable.js index 925c6d4b..2d19111f 100644 --- a/src/components/mui/BulkEditTable/BulkEditTable.js +++ b/src/components/mui/BulkEditTable/BulkEditTable.js @@ -31,10 +31,7 @@ import styles from "./BulkEditTable.module.less"; import CustomTablePagination from "../tables/components/CustomTablePagination"; import parsePaginationPosition from "../tables/components/pagination-position"; import showConfirmDialog from "../showConfirmDialog"; -import { - RESPONSIVE_TABLE_SX, - getActionsMenuBreakpoint -} from "../tables/components/table-styles"; +import { RESPONSIVE_TABLE_SX } from "../tables/components/table-styles"; const BulkEditTable = ({ options, @@ -52,6 +49,7 @@ const BulkEditTable = ({ idKey, onEdit, onDelete, + getRowActions, getName, deleteDialogTitle, deleteDialogBody, @@ -71,9 +69,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 @@ -196,7 +191,7 @@ const BulkEditTable = ({ ); })} - {(onEdit || onDelete) && ( + {(onEdit || onDelete || getRowActions) && ( ))} @@ -253,6 +247,7 @@ BulkEditTable.propTypes = { pageSliderVisible: PropTypes.bool, onEdit: PropTypes.func, onDelete: PropTypes.func, + getRowActions: PropTypes.func, getName: PropTypes.func, deleteDialogTitle: PropTypes.string, deleteDialogBody: PropTypes.oneOfType([PropTypes.func, PropTypes.string]), @@ -264,6 +259,7 @@ BulkEditTable.defaultProps = { idKey: "id", onEdit: null, onDelete: null, + getRowActions: null, getName: (item) => item.name, deleteDialogTitle: null, deleteDialogBody: null, diff --git a/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js b/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js index c9401ead..08dd79a3 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,51 @@ 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("shows getRowActions items in the row menu and runs their onClick", async () => { + const user = userEvent.setup(); + const onArchive = jest.fn(); + + render( + [{ label: "Archive", onClick: () => onArchive(row) }]} + /> + ); + + await user.click(screen.getAllByTestId("row-actions-menu-trigger")[1]); + await user.click(screen.getByRole("menuitem", { name: "Archive" })); + + expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 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 e20c7718..68722b3e 100644 --- a/src/components/mui/BulkEditTable/components/Row.js +++ b/src/components/mui/BulkEditTable/components/Row.js @@ -13,14 +13,9 @@ 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"; -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"; @@ -45,31 +40,19 @@ const Row = (props) => { onFieldChange, onEdit, onDelete, - idKey, - collapseActions, - actionsBreakpoint + getRowActions, + 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); }; return ( - + { /> ))} - {(onEdit || onDelete) && ( + {(onEdit || onDelete || getRowActions) && ( - - {onEdit && ( - onEdit(row)} - sx={{ padding: 0 }} - aria-label={`Edit row ${row[idKey]}`} - > - - - )} - {onDelete && ( - onDelete(row)} - sx={{ padding: 0 }} - aria-label={`Delete row ${row[idKey]}`} - > - - - )} - - {collapseActions && ( - - - - )} + onEdit(row))} + onDelete={onDelete && (() => onDelete(row))} + /> )} @@ -156,17 +109,15 @@ Row.propTypes = { onFieldChange: PropTypes.func, onEdit: PropTypes.func, onDelete: PropTypes.func, - idKey: PropTypes.string, - collapseActions: PropTypes.bool, - actionsBreakpoint: PropTypes.string + getRowActions: PropTypes.func, + idKey: PropTypes.string }; Row.defaultProps = { idKey: "id", onEdit: null, onDelete: null, - collapseActions: false, - actionsBreakpoint: "md" + getRowActions: null }; export default Row; diff --git a/src/components/mui/__tests__/mui-table-editable.test.js b/src/components/mui/__tests__/mui-table-editable.test.js index 8dd3f6a0..4862c3b6 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", () => ({ @@ -197,22 +198,35 @@ describe("MuiTableEditable", () => { expect(screen.queryByRole("textbox")).not.toBeInTheDocument(); }); - test("edit button calls onEdit", async () => { + test("menu Edit calls onEdit", async () => { const user = userEvent.setup(); const { onEdit } = setup(); - const [btn] = screen.getAllByLabelText("general.edit"); - await user.click(btn); + await user.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await user.click(screen.getByRole("menuitem", { name: "general.edit" })); expect(onEdit).toHaveBeenCalledWith( expect.objectContaining({ id: 1, name: "Alice" }) ); }); + test("getRowActions items show in the row menu and run their onClick", async () => { + const user = userEvent.setup(); + const onArchive = jest.fn(); + setup({ + onEdit: undefined, + onDelete: undefined, + getRowActions: (row) => [{ label: "Archive", onClick: () => onArchive(row) }] + }); + await user.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await user.click(screen.getByRole("menuitem", { name: "Archive" })); + expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); + }); + test("delete confirmed calls onDelete", async () => { 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 +235,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(); }); @@ -286,7 +300,7 @@ describe("MuiTableEditable", () => { expect(newDir).toBe(1); }); - test("applies archived styles to content, edit and delete cells when disableProp matches", () => { + test("applies archived styles to content, row menu and archive cells when disableProp matches", () => { setup({ options: { sortCol: "name", sortDir: -1, disableProp: "is_archived" }, data: [ @@ -299,7 +313,7 @@ describe("MuiTableEditable", () => { const aliceRow = screen.getByText("Alice").closest("tr"); const cells = within(aliceRow).getAllByTestId("mui-table-cell"); - const archivedCellIndexes = [0, 1, 2, 3, 5]; + const archivedCellIndexes = [0, 1, 2, 3, 4]; archivedCellIndexes.forEach((index) => { const sx = getCellSx(cells[index]); expect(sx.backgroundColor).toBe("background.light"); 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..75d5192f 100644 --- a/src/components/mui/__tests__/mui-table-sortable-v2.test.js +++ b/src/components/mui/__tests__/mui-table-sortable-v2.test.js @@ -58,7 +58,7 @@ jest.mock("@dnd-kit/utilities", () => ({ })); import React from "react"; -import { render, screen, within } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import "@testing-library/jest-dom"; import MuiTableSortableV2 from "../tables/sortable-table-v2/mui-table-sortable-v2"; @@ -111,30 +111,34 @@ describe("MuiTableSortableV2", () => { expect(screen.getByText("mui_table.no_items")).toBeInTheDocument(); }); - test("renders edit button when onEdit is provided", () => { + test("renders the row menu when onEdit is provided", () => { const onEdit = jest.fn(); setup({ onEdit }); - expect(screen.getAllByRole("button").length).toBeGreaterThan(0); + expect(screen.getAllByTestId("row-actions-menu-trigger")).toHaveLength(2); }); - test("calls onEdit when edit button is clicked", async () => { + test("calls onEdit from the row menu's Edit item", async () => { const onEdit = jest.fn(); setup({ onEdit }); - // scoped to the table itself, since pagination (top+bottom) also renders buttons - const buttons = within(screen.getByRole("table")).getAllByRole("button"); - // buttons[0] is the sort label button for the sortable "Name" column; - // buttons[1] is the first edit 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.edit" })); expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); }); + test("shows getRowActions items in the row menu and runs their onClick", async () => { + const onArchive = jest.fn(); + setup({ getRowActions: (row) => [{ label: "Archive", onClick: () => onArchive(row) }] }); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await userEvent.click(screen.getByRole("menuitem", { name: "Archive" })); + expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); + }); + test("calls showConfirmDialog and onDelete when delete is confirmed", async () => { 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 +243,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..80077702 100644 --- a/src/components/mui/__tests__/mui-table-sortable.test.js +++ b/src/components/mui/__tests__/mui-table-sortable.test.js @@ -39,7 +39,7 @@ jest.mock("react-beautiful-dnd", () => { }); import React from "react"; -import { render, screen, within } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import "@testing-library/jest-dom"; import MuiTableSortable from "../tables/sortable-table"; @@ -92,30 +92,34 @@ describe("MuiTableSortable", () => { expect(screen.getByText("mui_table.no_items")).toBeInTheDocument(); }); - test("renders edit button when onEdit is provided", () => { + test("renders the row menu when onEdit is provided", () => { const onEdit = jest.fn(); setup({ onEdit }); - expect(screen.getAllByRole("button").length).toBeGreaterThan(0); + expect(screen.getAllByTestId("row-actions-menu-trigger")).toHaveLength(2); }); - test("calls onEdit when edit button is clicked", async () => { + test("calls onEdit from the row menu's Edit item", async () => { const onEdit = jest.fn(); setup({ onEdit }); - // scoped to the table itself, since pagination (top+bottom) also renders buttons - const buttons = within(screen.getByRole("table")).getAllByRole("button"); - // buttons[0] is the sort label button for the sortable "Name" column; - // buttons[1] is the first edit 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.edit" })); expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); }); + test("shows getRowActions items in the row menu and runs their onClick", async () => { + const onArchive = jest.fn(); + setup({ getRowActions: (row) => [{ label: "Archive", onClick: () => onArchive(row) }] }); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await userEvent.click(screen.getByRole("menuitem", { name: "Archive" })); + expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); + }); + test("calls showConfirmDialog and onDelete when delete is confirmed", async () => { 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..1c5185bb 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -74,23 +74,29 @@ 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 when edit button is clicked", async () => { + test("calls onEdit once from the row menu's Edit item", async () => { const onEdit = jest.fn(); setup({ onEdit }); - await userEvent.click(screen.getAllByTestId("action-edit")[0]); - expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[1]); + await userEvent.click(screen.getByRole("menuitem", { name: "general.edit" })); + expect(onEdit).toHaveBeenCalledTimes(1); + expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 2 })); + }); + + test("shows getRowActions items in the row menu and runs their onClick", async () => { + const onArchive = jest.fn(); + setup({ getRowActions: (row) => [{ label: "Archive", onClick: () => onArchive(row) }] }); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + await userEvent.click(screen.getByRole("menuitem", { name: "Archive" })); + expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); }); 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 +106,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 +137,17 @@ 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("keeps Edit in the menu when canDelete returns false", async () => { + setup({ onEdit: jest.fn(), onDelete: jest.fn(), canDelete: (row) => row.id !== 1 }); + await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); + expect(screen.getByRole("menuitem", { name: "general.edit" })).toBeInTheDocument(); + expect(screen.queryByRole("menuitem", { name: "general.delete" })).not.toBeInTheDocument(); }); test("renders pagination (top and bottom) when perPage and currentPage are set", () => { @@ -171,18 +185,30 @@ 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("disables the row menu for rows where disableProp is truthy", () => { + setup({ data: disabledData, onEdit: jest.fn(), onDelete: jest.fn(), options: disabledOptions }); + const triggers = screen.getAllByTestId("row-actions-menu-trigger"); + expect(triggers[0]).toBeDisabled(); + expect(triggers[1]).not.toBeDisabled(); }); - test("disables delete button for rows where disableProp is truthy", () => { - setup({ data: disabledData, onDelete: jest.fn(), options: disabledOptions }); - const buttons = screen.getAllByTestId("action-delete"); - expect(buttons[0]).toBeDisabled(); - expect(buttons[1]).not.toBeDisabled(); + test("custom actions stay clickable on disabled rows while Edit is greyed out", async () => { + const onUnarchive = jest.fn(); + setup({ + data: disabledData, + onEdit: jest.fn(), + options: disabledOptions, + getRowActions: (row) => [{ label: "Unarchive", onClick: () => onUnarchive(row) }] + }); + const [trigger] = screen.getAllByTestId("row-actions-menu-trigger"); + expect(trigger).not.toBeDisabled(); + await userEvent.click(trigger); + expect(screen.getByRole("menuitem", { name: "general.edit" })).toHaveAttribute( + "aria-disabled", + "true" + ); + await userEvent.click(screen.getByRole("menuitem", { name: "Unarchive" })); + expect(onUnarchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); }); test("disables select button for rows where disableProp is truthy", () => { @@ -280,7 +306,8 @@ 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(); }); }); + }); diff --git a/src/components/mui/tables/components/row-actions-menu.js b/src/components/mui/tables/components/row-actions-menu.js index 1fc1d4f8..a658caea 100644 --- a/src/components/mui/tables/components/row-actions-menu.js +++ b/src/components/mui/tables/components/row-actions-menu.js @@ -13,30 +13,54 @@ 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 Divider from "@mui/material/Divider"; +import Tooltip from "@mui/material/Tooltip"; import MoreVertIcon from "@mui/icons-material/MoreVert"; -const RowActionsMenu = ({ actions }) => { +// built-in Edit/Delete keep every table's menu consistent; `actions` holds any extra items +const RowActionsMenu = ({ actions = [], onEdit, onDelete, disabled = false }) => { const [anchorEl, setAnchorEl] = React.useState(null); - if (!actions.length) return null; + const hasBuiltIns = !!(onEdit || onDelete); + if (!hasBuiltIns && !actions.length) return null; 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={ + (!hasBuiltIns || disabled) && actions.every((action) => action.disabled) + } + > + + + + setAnchorEl(null)} > + {onEdit && ( + { + setAnchorEl(null); + onEdit(); + }} + > + {T.translate("general.edit")} + + )} {actions.map((action) => ( { {action.label} ))} + {onDelete && (onEdit || actions.length > 0) && } + {onDelete && ( + { + setAnchorEl(null); + onDelete(); + }} + sx={{ color: "error.main" }} + > + {T.translate("general.delete")} + + )} ); @@ -61,7 +98,10 @@ RowActionsMenu.propTypes = { onClick: PropTypes.func.isRequired, disabled: PropTypes.bool }) - ).isRequired + ), + onEdit: PropTypes.func, + onDelete: PropTypes.func, + disabled: PropTypes.bool }; export default RowActionsMenu; 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 && { diff --git a/src/components/mui/tables/editable-table/index.js b/src/components/mui/tables/editable-table/index.js index 8563ff42..98758b98 100644 --- a/src/components/mui/tables/editable-table/index.js +++ b/src/components/mui/tables/editable-table/index.js @@ -21,15 +21,15 @@ import TableBody from "@mui/material/TableBody"; import TableCell from "@mui/material/TableCell"; import TableHead from "@mui/material/TableHead"; import TableRow from "@mui/material/TableRow"; -import { IconButton, TextField } from "@mui/material"; +import { 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, @@ -154,6 +154,7 @@ const MuiTableEditable = ({ onEdit, onArchive, onDelete, + getRowActions, onCellChange, // New prop for handling cell value changes deleteDialogBody, tableSx = {}, @@ -229,9 +230,6 @@ const MuiTableEditable = ({ /> ))} - {onEdit && ( - - )} {onArchive && ( )} - {onDelete && ( + {(onEdit || onDelete || getRowActions) && ( )} @@ -288,23 +286,6 @@ const MuiTableEditable = ({ )} ))} - {onEdit && ( - - onEdit(row)} - size="small" - aria-label={T.translate("general.edit")} - > - - - - )} {onArchive && ( )} - {onDelete && ( + {(onEdit || onDelete || getRowActions) && ( - handleDelete(row)} - size="small" - aria-label={T.translate("general.delete")} - > - - + onEdit(row))} + onDelete={onDelete && (() => handleDelete(row))} + /> )} @@ -390,6 +370,7 @@ MuiTableEditable.propTypes = { onEdit: PropTypes.func, onArchive: PropTypes.func, onDelete: PropTypes.func, + getRowActions: PropTypes.func, onCellChange: PropTypes.func, deleteDialogBody: PropTypes.func, confirmButtonColor: PropTypes.string diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index fa34da32..24e1ac12 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -23,8 +23,6 @@ import { TableRow, Tooltip } from "@mui/material"; -import EditIcon from "@mui/icons-material/Edit"; -import DeleteIcon from "@mui/icons-material/Delete"; import ArchiveIcon from "@mui/icons-material/Archive"; import UnarchiveIcon from "@mui/icons-material/Unarchive"; import styles from "../components/styles.module.less"; @@ -40,7 +38,6 @@ import { getColumnWidthSx, getArchivedRowSx, getActionCellSx, - getActionsMenuBreakpoint, ACTION_CELL_SX } from "../components/table-styles"; @@ -61,6 +58,7 @@ const MuiTable = ({ onEdit, onArchive, onDelete, + getRowActions, onSelect, canDelete = () => true, deleteDialogTitle = null, @@ -75,17 +73,11 @@ const MuiTable = ({ disableProp: null, // disableProp is the prop that will disable the row ...userOptions }; - const activeActionsCount = - (onEdit ? 1 : 0) + (onDelete ? 1 : 0) + (onArchive ? 1 : 0) + (onSelect ? 1 : 0); - // responsive:false opts out of collapsing actions into a menu — needed when - // extra rows (e.g. TotalRow) are passed as children with a static column - // count that can't track the collapse breakpoint - const collapseActions = - options.responsive !== false && !children && activeActionsCount >= 2; - const collapseBreakpoint = getActionsMenuBreakpoint(columns.length); + const hasRowMenu = !!(onEdit || onDelete || getRowActions); + const actionColumnsCount = + (hasRowMenu ? 1 : 0) + (onArchive ? 1 : 0) + (onSelect ? 1 : 0); - const totalColumnsCount = - columns.length + activeActionsCount + (collapseActions ? 1 : 0); + const totalColumnsCount = columns.length + actionColumnsCount; const {sortCol, sortDir} = options; @@ -94,43 +86,6 @@ 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 || {}), @@ -174,11 +129,9 @@ const MuiTable = ({ /> ))} - {onEdit && } - {onArchive && } - {onDelete && } - {onSelect && } - {collapseActions && } + {onArchive && } + {hasRowMenu && } + {onSelect && } @@ -188,7 +141,7 @@ const MuiTable = ({ {data.map((row) => row.renderRow ? ( {row.renderRow(totalColumnsCount)} ) : ( - + {/* 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 && ( )} - {/* Delete column */} - {onDelete && ( + {/* Row actions menu column */} + {hasRowMenu && ( - {canDelete(row) && ( - - - handleDelete(row)} - data-testid="action-delete" - sx={{ padding: 0 }} - disabled={options.disableProp && row[options.disableProp]} - > - - - - - )} + onEdit(row))} + onDelete={onDelete && canDelete(row) ? () => handleDelete(row) : undefined} + disabled={!!(options.disableProp && row[options.disableProp])} + /> )} {onSelect && ( @@ -315,15 +225,6 @@ const MuiTable = ({ )} - {collapseActions && ( - - - - )} ))} {/* Here we inject extra rows passed as children */} @@ -358,6 +259,7 @@ MuiTable.propTypes = { onEdit: PropTypes.func, onArchive: PropTypes.func, onDelete: PropTypes.func, + getRowActions: PropTypes.func, onSelect: PropTypes.func, canDelete: PropTypes.func, deleteDialogTitle: PropTypes.string, 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..8ba60e68 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 @@ -24,8 +24,6 @@ import TableRow from "@mui/material/TableRow"; 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 +33,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"; @@ -59,6 +58,7 @@ const MuiTableSortableV2 = ({ getName = (item) => item.name, onEdit, onDelete, + getRowActions, deleteDialogTitle = null, deleteDialogBody = null, onReorder, @@ -176,8 +176,7 @@ const MuiTableSortableV2 = ({ )} ))} - {onEdit && } - {onDelete && } + {(onEdit || onDelete || getRowActions) && } {onReorder && } @@ -206,36 +205,18 @@ const MuiTableSortableV2 = ({ ))} - {/* Edit column */} - {onEdit && ( + {/* Row actions menu column */} + {(onEdit || onDelete || getRowActions) && ( - onEdit(row)} - sx={{ padding: 0 }} - > - - - - )} - {/* Delete column */} - {onDelete && ( - - handleDelete(row)} - sx={{ padding: 0 }} - > - - + onEdit(row))} + onDelete={onDelete && (() => handleDelete(row))} + /> )} {/* Re order column */} @@ -259,8 +240,7 @@ const MuiTableSortableV2 = ({ item.name, onEdit, onDelete, + getRowActions, deleteDialogTitle = null, deleteDialogBody = null, onReorder, @@ -120,8 +120,7 @@ const MuiTableSortable = ({ /> ))} - {onEdit && } - {onDelete && } + {(onEdit || onDelete || getRowActions) && } {onReorder && } @@ -176,36 +175,18 @@ const MuiTableSortable = ({ ))} - {/* Edit column */} - {onEdit && ( + {/* Row actions menu column */} + {(onEdit || onDelete || getRowActions) && ( - onEdit(row)} - sx={{ padding: 0 }} - > - - - - )} - {/* Delete column */} - {onDelete && ( - - handleDelete(row)} - sx={{ padding: 0 }} - > - - + onEdit(row))} + onDelete={onDelete && (() => handleDelete(row))} + /> )} {/* Re order column */} diff --git a/src/i18n/en.json b/src/i18n/en.json index f891fed5..3e18da9e 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -34,6 +34,7 @@ "edit": "Edit", "delete": "Delete", "view": "View", + "more_actions": "More actions", "cancel": "Cancel", "apply": "Apply", "n_a": "N/A", From ec6fcddf6030d63b31d2ea0d87f3c992146f3a66 Mon Sep 17 00:00:00 2001 From: anna Date: Tue, 3 Nov 2026 11:55:19 -0600 Subject: [PATCH 2/7] fix: revert row actions change on tabels not used in summit admin --- .../mui/BulkEditTable/BulkEditTable.js | 16 ++-- .../__tests__/BulkEditTable.test.js | 51 ------------- .../mui/BulkEditTable/components/Row.js | 73 ++++++++++++++++--- .../__tests__/mui-table-sortable-v2.test.js | 33 ++++----- .../mui/tables/components/table-styles.js | 4 + .../mui-table-sortable-v2.js | 42 ++++++++--- 6 files changed, 121 insertions(+), 98 deletions(-) diff --git a/src/components/mui/BulkEditTable/BulkEditTable.js b/src/components/mui/BulkEditTable/BulkEditTable.js index 2d19111f..925c6d4b 100644 --- a/src/components/mui/BulkEditTable/BulkEditTable.js +++ b/src/components/mui/BulkEditTable/BulkEditTable.js @@ -31,7 +31,10 @@ import styles from "./BulkEditTable.module.less"; import CustomTablePagination from "../tables/components/CustomTablePagination"; import parsePaginationPosition from "../tables/components/pagination-position"; import showConfirmDialog from "../showConfirmDialog"; -import { RESPONSIVE_TABLE_SX } from "../tables/components/table-styles"; +import { + RESPONSIVE_TABLE_SX, + getActionsMenuBreakpoint +} from "../tables/components/table-styles"; const BulkEditTable = ({ options, @@ -49,7 +52,6 @@ const BulkEditTable = ({ idKey, onEdit, onDelete, - getRowActions, getName, deleteDialogTitle, deleteDialogBody, @@ -69,6 +71,9 @@ 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 @@ -191,7 +196,7 @@ const BulkEditTable = ({ ); })} - {(onEdit || onDelete || getRowActions) && ( + {(onEdit || onDelete) && ( ))} @@ -247,7 +253,6 @@ BulkEditTable.propTypes = { pageSliderVisible: PropTypes.bool, onEdit: PropTypes.func, onDelete: PropTypes.func, - getRowActions: PropTypes.func, getName: PropTypes.func, deleteDialogTitle: PropTypes.string, deleteDialogBody: PropTypes.oneOfType([PropTypes.func, PropTypes.string]), @@ -259,7 +264,6 @@ BulkEditTable.defaultProps = { idKey: "id", onEdit: null, onDelete: null, - getRowActions: null, getName: (item) => item.name, deleteDialogTitle: null, deleteDialogBody: null, diff --git a/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js b/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js index 08dd79a3..c9401ead 100644 --- a/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js +++ b/src/components/mui/BulkEditTable/__tests__/BulkEditTable.test.js @@ -21,18 +21,12 @@ 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: { @@ -54,51 +48,6 @@ 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("shows getRowActions items in the row menu and runs their onClick", async () => { - const user = userEvent.setup(); - const onArchive = jest.fn(); - - render( - [{ label: "Archive", onClick: () => onArchive(row) }]} - /> - ); - - await user.click(screen.getAllByTestId("row-actions-menu-trigger")[1]); - await user.click(screen.getByRole("menuitem", { name: "Archive" })); - - expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 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 68722b3e..e20c7718 100644 --- a/src/components/mui/BulkEditTable/components/Row.js +++ b/src/components/mui/BulkEditTable/components/Row.js @@ -13,9 +13,14 @@ 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"; +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"; @@ -40,19 +45,31 @@ const Row = (props) => { onFieldChange, onEdit, onDelete, - getRowActions, - idKey + idKey, + collapseActions, + actionsBreakpoint } = 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); }; return ( - + { /> ))} - {(onEdit || onDelete || getRowActions) && ( + {(onEdit || onDelete) && ( - onEdit(row))} - onDelete={onDelete && (() => onDelete(row))} - /> + + {onEdit && ( + onEdit(row)} + sx={{ padding: 0 }} + aria-label={`Edit row ${row[idKey]}`} + > + + + )} + {onDelete && ( + onDelete(row)} + sx={{ padding: 0 }} + aria-label={`Delete row ${row[idKey]}`} + > + + + )} + + {collapseActions && ( + + + + )} )} @@ -109,15 +156,17 @@ Row.propTypes = { onFieldChange: PropTypes.func, onEdit: PropTypes.func, onDelete: PropTypes.func, - getRowActions: PropTypes.func, - idKey: PropTypes.string + idKey: PropTypes.string, + collapseActions: PropTypes.bool, + actionsBreakpoint: PropTypes.string }; Row.defaultProps = { idKey: "id", onEdit: null, onDelete: null, - getRowActions: null + collapseActions: false, + actionsBreakpoint: "md" }; export default Row; 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 75d5192f..f1a7ae39 100644 --- a/src/components/mui/__tests__/mui-table-sortable-v2.test.js +++ b/src/components/mui/__tests__/mui-table-sortable-v2.test.js @@ -58,7 +58,7 @@ jest.mock("@dnd-kit/utilities", () => ({ })); 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 MuiTableSortableV2 from "../tables/sortable-table-v2/mui-table-sortable-v2"; @@ -111,34 +111,30 @@ describe("MuiTableSortableV2", () => { expect(screen.getByText("mui_table.no_items")).toBeInTheDocument(); }); - test("renders the row menu when onEdit is provided", () => { + test("renders edit button when onEdit is provided", () => { const onEdit = jest.fn(); setup({ onEdit }); - expect(screen.getAllByTestId("row-actions-menu-trigger")).toHaveLength(2); + expect(screen.getAllByRole("button").length).toBeGreaterThan(0); }); - test("calls onEdit from the row menu's Edit item", async () => { + test("calls onEdit when edit button is clicked", async () => { const onEdit = jest.fn(); setup({ onEdit }); - await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); - await userEvent.click(screen.getByRole("menuitem", { name: "general.edit" })); + // scoped to the table itself, since pagination (top+bottom) also renders buttons + const buttons = within(screen.getByRole("table")).getAllByRole("button"); + // buttons[0] is the sort label button for the sortable "Name" column; + // buttons[1] is the first edit button (row 1) + await userEvent.click(buttons[1]); expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); }); - test("shows getRowActions items in the row menu and runs their onClick", async () => { - const onArchive = jest.fn(); - setup({ getRowActions: (row) => [{ label: "Archive", onClick: () => onArchive(row) }] }); - await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); - await userEvent.click(screen.getByRole("menuitem", { name: "Archive" })); - expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); - }); - test("calls showConfirmDialog and onDelete when delete is confirmed", async () => { const onDelete = jest.fn(); showConfirmDialog.mockResolvedValueOnce(true); setup({ onDelete }); - await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); - await userEvent.click(screen.getByRole("menuitem", { name: "general.delete" })); + 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 new Promise((r) => setTimeout(r, 0)); expect(showConfirmDialog).toHaveBeenCalled(); expect(onDelete).toHaveBeenCalledWith(1); @@ -243,8 +239,9 @@ describe("MuiTableSortableV2", () => { 2 ); - await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); - await userEvent.click(screen.getByRole("menuitem", { name: "general.delete" })); + 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 new Promise((r) => setTimeout(r, 0)); expect(onDelete).toHaveBeenCalledWith("a-1"); }); diff --git a/src/components/mui/tables/components/table-styles.js b/src/components/mui/tables/components/table-styles.js index 36fa9cd3..004713e8 100644 --- a/src/components/mui/tables/components/table-styles.js +++ b/src/components/mui/tables/components/table-styles.js @@ -1,3 +1,4 @@ +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 = { @@ -18,6 +19,9 @@ 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 && { 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 8ba60e68..aaf1225a 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 @@ -24,6 +24,8 @@ import TableRow from "@mui/material/TableRow"; 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"; @@ -33,7 +35,6 @@ 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"; @@ -58,7 +59,6 @@ const MuiTableSortableV2 = ({ getName = (item) => item.name, onEdit, onDelete, - getRowActions, deleteDialogTitle = null, deleteDialogBody = null, onReorder, @@ -176,7 +176,8 @@ const MuiTableSortableV2 = ({ )} ))} - {(onEdit || onDelete || getRowActions) && } + {onEdit && } + {onDelete && } {onReorder && } @@ -205,18 +206,36 @@ const MuiTableSortableV2 = ({ ))} - {/* Row actions menu column */} - {(onEdit || onDelete || getRowActions) && ( + {/* Edit column */} + {onEdit && ( - onEdit(row))} - onDelete={onDelete && (() => handleDelete(row))} - /> + onEdit(row)} + sx={{ padding: 0 }} + > + + + + )} + {/* Delete column */} + {onDelete && ( + + handleDelete(row)} + sx={{ padding: 0 }} + > + + )} {/* Re order column */} @@ -240,7 +259,8 @@ const MuiTableSortableV2 = ({ Date: Tue, 3 Nov 2026 12:56:36 -0600 Subject: [PATCH 3/7] fix: disable edit and delete on editable tables --- .../mui/__tests__/mui-table-editable.test.js | 13 +++++++++++++ src/components/mui/tables/editable-table/index.js | 1 + 2 files changed, 14 insertions(+) diff --git a/src/components/mui/__tests__/mui-table-editable.test.js b/src/components/mui/__tests__/mui-table-editable.test.js index 4862c3b6..3b7dd4ce 100644 --- a/src/components/mui/__tests__/mui-table-editable.test.js +++ b/src/components/mui/__tests__/mui-table-editable.test.js @@ -300,6 +300,19 @@ describe("MuiTableEditable", () => { expect(newDir).toBe(1); }); + test("disables the row menu for rows where disableProp is truthy", () => { + setup({ + options: { sortCol: "name", sortDir: -1, disableProp: "is_disabled" }, + data: [ + { id: 1, name: "Alice", role: "Dev", age: 35, is_disabled: true }, + { id: 2, name: "Bob", role: "PM", age: 41, is_disabled: false } + ] + }); + const triggers = screen.getAllByTestId("row-actions-menu-trigger"); + expect(triggers[0]).toBeDisabled(); + expect(triggers[1]).not.toBeDisabled(); + }); + test("applies archived styles to content, row menu and archive cells when disableProp matches", () => { setup({ options: { sortCol: "name", sortDir: -1, disableProp: "is_archived" }, diff --git a/src/components/mui/tables/editable-table/index.js b/src/components/mui/tables/editable-table/index.js index 98758b98..45ac22c8 100644 --- a/src/components/mui/tables/editable-table/index.js +++ b/src/components/mui/tables/editable-table/index.js @@ -327,6 +327,7 @@ const MuiTableEditable = ({ actions={getRowActions?.(row)} onEdit={onEdit && (() => onEdit(row))} onDelete={onDelete && (() => handleDelete(row))} + disabled={!!(options.disableProp && row[options.disableProp])} /> )} From 3f8f274f8989609c8a6b49b9b04911d4b26e064d Mon Sep 17 00:00:00 2001 From: anna Date: Tue, 3 Nov 2026 13:01:57 -0600 Subject: [PATCH 4/7] fix: remove leftover hover, make extra actions button accessible, guard a null row actions --- src/components/mui/tables/components/row-actions-menu.js | 6 +++++- src/components/mui/tables/mui-table/index.js | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/mui/tables/components/row-actions-menu.js b/src/components/mui/tables/components/row-actions-menu.js index a658caea..47832391 100644 --- a/src/components/mui/tables/components/row-actions-menu.js +++ b/src/components/mui/tables/components/row-actions-menu.js @@ -22,8 +22,10 @@ import Tooltip from "@mui/material/Tooltip"; import MoreVertIcon from "@mui/icons-material/MoreVert"; // built-in Edit/Delete keep every table's menu consistent; `actions` holds any extra items -const RowActionsMenu = ({ actions = [], onEdit, onDelete, disabled = false }) => { +const RowActionsMenu = ({ actions: actionsProp, onEdit, onDelete, disabled = false }) => { const [anchorEl, setAnchorEl] = React.useState(null); + // getRowActions may return null for rows without extra actions + const actions = actionsProp ?? []; const hasBuiltIns = !!(onEdit || onDelete); if (!hasBuiltIns && !actions.length) return null; @@ -37,6 +39,8 @@ const RowActionsMenu = ({ actions = [], onEdit, onDelete, disabled = false }) => onClick={(e) => setAnchorEl(e.currentTarget)} data-testid="row-actions-menu-trigger" aria-label={T.translate("general.more_actions")} + aria-haspopup="true" + aria-expanded={anchorEl ? "true" : undefined} disabled={ (!hasBuiltIns || disabled) && actions.every((action) => action.disabled) } diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index 24e1ac12..8aaf7bd8 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -141,7 +141,7 @@ const MuiTable = ({ {data.map((row) => row.renderRow ? ( {row.renderRow(totalColumnsCount)} ) : ( - + {/* Main content columns */} {columns.map((col) => ( Date: Tue, 3 Nov 2026 13:22:42 -0600 Subject: [PATCH 5/7] Fix: JSX syntax for row actions in sortable table Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/components/mui/tables/sortable-table/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/mui/tables/sortable-table/index.js b/src/components/mui/tables/sortable-table/index.js index a94f30b4..c46eab62 100644 --- a/src/components/mui/tables/sortable-table/index.js +++ b/src/components/mui/tables/sortable-table/index.js @@ -186,6 +186,7 @@ const MuiTableSortable = ({ actions={getRowActions?.(row)} onEdit={onEdit && (() => onEdit(row))} onDelete={onDelete && (() => handleDelete(row))} + disabled={!!(options.disableProp && row[options.disableProp])} /> )} From 1e2edea20c3e2d19f4f6b68fb3ba525d9de8a0d8 Mon Sep 17 00:00:00 2001 From: anna Date: Tue, 3 Nov 2026 15:34:06 -0600 Subject: [PATCH 6/7] fix: allow tables to take in the number of rows --- src/components/mui/__tests__/mui-table.test.js | 14 ++++++++++++++ src/components/mui/tables/mui-table/index.js | 6 +++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/components/mui/__tests__/mui-table.test.js b/src/components/mui/__tests__/mui-table.test.js index 1c5185bb..642af3b1 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -74,6 +74,20 @@ describe("MuiTable", () => { expect(screen.getByText("mui_table.no_items")).toBeInTheDocument(); }); + test.each([ + ["without action columns", {}, 2], + ["with the row menu column", { onEdit: jest.fn(), onDelete: jest.fn() }, 3] + ])("passes the column count to function children %s", (_, overrides, expected) => { + const children = jest.fn((colCount) => ( + + {`extra row spans ${colCount}`} + + )); + setup({ ...overrides, children }); + expect(children).toHaveBeenCalledWith(expected); + expect(screen.getByText(`extra row spans ${expected}`)).toBeInTheDocument(); + }); + test("calls onEdit once from the row menu's Edit item", async () => { const onEdit = jest.fn(); setup({ onEdit }); diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index 8aaf7bd8..81312aa7 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -227,8 +227,8 @@ const MuiTable = ({ )} ))} - {/* Here we inject extra rows passed as children */} - {children} + {/* Here we inject extra rows passed as children; a function child receives the column count */} + {typeof children === "function" ? children(totalColumnsCount) : children} {data.length === 0 && ( @@ -245,7 +245,7 @@ const MuiTable = ({ MuiTable.propTypes = { columns: PropTypes.array, data: PropTypes.array, - children: PropTypes.node, + children: PropTypes.oneOfType([PropTypes.node, PropTypes.func]), totalRows: PropTypes.number, perPage: PropTypes.number, currentPage: PropTypes.number, From 5ebd416ba764c55fa0a5218b14042e34b2c99f5a Mon Sep 17 00:00:00 2001 From: anna Date: Wed, 4 Nov 2026 12:54:59 -0600 Subject: [PATCH 7/7] fix: remove row actions --- .../mui/__tests__/mui-table-editable.test.js | 13 --------- .../mui/__tests__/mui-table-sortable.test.js | 8 ------ .../mui/__tests__/mui-table.test.js | 27 ------------------- .../mui/tables/editable-table/index.js | 7 ++--- src/components/mui/tables/mui-table/index.js | 5 +--- .../mui/tables/sortable-table/index.js | 6 ++--- 6 files changed, 5 insertions(+), 61 deletions(-) diff --git a/src/components/mui/__tests__/mui-table-editable.test.js b/src/components/mui/__tests__/mui-table-editable.test.js index 3b7dd4ce..70d088ca 100644 --- a/src/components/mui/__tests__/mui-table-editable.test.js +++ b/src/components/mui/__tests__/mui-table-editable.test.js @@ -208,19 +208,6 @@ describe("MuiTableEditable", () => { ); }); - test("getRowActions items show in the row menu and run their onClick", async () => { - const user = userEvent.setup(); - const onArchive = jest.fn(); - setup({ - onEdit: undefined, - onDelete: undefined, - getRowActions: (row) => [{ label: "Archive", onClick: () => onArchive(row) }] - }); - await user.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); - await user.click(screen.getByRole("menuitem", { name: "Archive" })); - expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); - }); - test("delete confirmed calls onDelete", async () => { const user = userEvent.setup(); const { onDelete } = setup(); diff --git a/src/components/mui/__tests__/mui-table-sortable.test.js b/src/components/mui/__tests__/mui-table-sortable.test.js index 80077702..709d3716 100644 --- a/src/components/mui/__tests__/mui-table-sortable.test.js +++ b/src/components/mui/__tests__/mui-table-sortable.test.js @@ -106,14 +106,6 @@ describe("MuiTableSortable", () => { expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); }); - test("shows getRowActions items in the row menu and runs their onClick", async () => { - const onArchive = jest.fn(); - setup({ getRowActions: (row) => [{ label: "Archive", onClick: () => onArchive(row) }] }); - await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); - await userEvent.click(screen.getByRole("menuitem", { name: "Archive" })); - expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); - }); - test("calls showConfirmDialog and onDelete when delete is confirmed", async () => { const onDelete = jest.fn(); showConfirmDialog.mockResolvedValueOnce(true); diff --git a/src/components/mui/__tests__/mui-table.test.js b/src/components/mui/__tests__/mui-table.test.js index 642af3b1..ff113793 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -97,14 +97,6 @@ describe("MuiTable", () => { expect(onEdit).toHaveBeenCalledWith(expect.objectContaining({ id: 2 })); }); - test("shows getRowActions items in the row menu and runs their onClick", async () => { - const onArchive = jest.fn(); - setup({ getRowActions: (row) => [{ label: "Archive", onClick: () => onArchive(row) }] }); - await userEvent.click(screen.getAllByTestId("row-actions-menu-trigger")[0]); - await userEvent.click(screen.getByRole("menuitem", { name: "Archive" })); - expect(onArchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); - }); - test("calls showConfirmDialog and then onDelete when delete confirmed", async () => { const onDelete = jest.fn(); showConfirmDialog.mockResolvedValueOnce(true); @@ -206,25 +198,6 @@ describe("MuiTable", () => { expect(triggers[1]).not.toBeDisabled(); }); - test("custom actions stay clickable on disabled rows while Edit is greyed out", async () => { - const onUnarchive = jest.fn(); - setup({ - data: disabledData, - onEdit: jest.fn(), - options: disabledOptions, - getRowActions: (row) => [{ label: "Unarchive", onClick: () => onUnarchive(row) }] - }); - const [trigger] = screen.getAllByTestId("row-actions-menu-trigger"); - expect(trigger).not.toBeDisabled(); - await userEvent.click(trigger); - expect(screen.getByRole("menuitem", { name: "general.edit" })).toHaveAttribute( - "aria-disabled", - "true" - ); - await userEvent.click(screen.getByRole("menuitem", { name: "Unarchive" })); - expect(onUnarchive).toHaveBeenCalledWith(expect.objectContaining({ id: 1 })); - }); - test("disables select button for rows where disableProp is truthy", () => { setup({ data: disabledData, onSelect: jest.fn(), options: disabledOptions }); const buttons = screen.getAllByTestId("action-select"); diff --git a/src/components/mui/tables/editable-table/index.js b/src/components/mui/tables/editable-table/index.js index 45ac22c8..b175c8e5 100644 --- a/src/components/mui/tables/editable-table/index.js +++ b/src/components/mui/tables/editable-table/index.js @@ -154,7 +154,6 @@ const MuiTableEditable = ({ onEdit, onArchive, onDelete, - getRowActions, onCellChange, // New prop for handling cell value changes deleteDialogBody, tableSx = {}, @@ -241,7 +240,7 @@ const MuiTableEditable = ({ }} /> )} - {(onEdit || onDelete || getRowActions) && ( + {(onEdit || onDelete) && ( )} @@ -314,7 +313,7 @@ const MuiTableEditable = ({ )} - {(onEdit || onDelete || getRowActions) && ( + {(onEdit || onDelete) && ( onEdit(row))} onDelete={onDelete && (() => handleDelete(row))} disabled={!!(options.disableProp && row[options.disableProp])} @@ -371,7 +369,6 @@ MuiTableEditable.propTypes = { onEdit: PropTypes.func, onArchive: PropTypes.func, onDelete: PropTypes.func, - getRowActions: PropTypes.func, onCellChange: PropTypes.func, deleteDialogBody: PropTypes.func, confirmButtonColor: PropTypes.string diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index 81312aa7..797cc9c2 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -58,7 +58,6 @@ const MuiTable = ({ onEdit, onArchive, onDelete, - getRowActions, onSelect, canDelete = () => true, deleteDialogTitle = null, @@ -73,7 +72,7 @@ const MuiTable = ({ disableProp: null, // disableProp is the prop that will disable the row ...userOptions }; - const hasRowMenu = !!(onEdit || onDelete || getRowActions); + const hasRowMenu = !!(onEdit || onDelete); const actionColumnsCount = (hasRowMenu ? 1 : 0) + (onArchive ? 1 : 0) + (onSelect ? 1 : 0); @@ -197,7 +196,6 @@ const MuiTable = ({ className={styles.dottedBorderLeft} > onEdit(row))} onDelete={onDelete && canDelete(row) ? () => handleDelete(row) : undefined} disabled={!!(options.disableProp && row[options.disableProp])} @@ -259,7 +257,6 @@ MuiTable.propTypes = { onEdit: PropTypes.func, onArchive: PropTypes.func, onDelete: PropTypes.func, - getRowActions: PropTypes.func, onSelect: PropTypes.func, canDelete: PropTypes.func, deleteDialogTitle: PropTypes.string, diff --git a/src/components/mui/tables/sortable-table/index.js b/src/components/mui/tables/sortable-table/index.js index c46eab62..a3e78ecb 100644 --- a/src/components/mui/tables/sortable-table/index.js +++ b/src/components/mui/tables/sortable-table/index.js @@ -51,7 +51,6 @@ const MuiTableSortable = ({ getName = (item) => item.name, onEdit, onDelete, - getRowActions, deleteDialogTitle = null, deleteDialogBody = null, onReorder, @@ -120,7 +119,7 @@ const MuiTableSortable = ({ /> ))} - {(onEdit || onDelete || getRowActions) && } + {(onEdit || onDelete) && } {onReorder && } @@ -176,14 +175,13 @@ const MuiTableSortable = ({ ))} {/* Row actions menu column */} - {(onEdit || onDelete || getRowActions) && ( + {(onEdit || onDelete) && ( onEdit(row))} onDelete={onDelete && (() => handleDelete(row))} disabled={!!(options.disableProp && row[options.disableProp])}