diff --git a/src/components/mui/__tests__/mui-table-editable.test.js b/src/components/mui/__tests__/mui-table-editable.test.js index 8dd3f6a0..70d088ca 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,11 +198,11 @@ 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" }) ); @@ -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(); }); @@ -286,7 +287,20 @@ describe("MuiTableEditable", () => { expect(newDir).toBe(1); }); - test("applies archived styles to content, edit and delete cells when disableProp matches", () => { + 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" }, 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.test.js b/src/components/mui/__tests__/mui-table-sortable.test.js index fa5ff099..709d3716 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,20 +92,17 @@ 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 })); }); @@ -113,9 +110,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..ff113793 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -74,23 +74,35 @@ 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.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 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("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 +112,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 +143,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 +191,11 @@ 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 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("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 select button for rows where disableProp is truthy", () => { @@ -280,7 +293,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..47832391 100644 --- a/src/components/mui/tables/components/row-actions-menu.js +++ b/src/components/mui/tables/components/row-actions-menu.js @@ -13,30 +13,58 @@ 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: actionsProp, onEdit, onDelete, disabled = false }) => { const [anchorEl, setAnchorEl] = React.useState(null); + // getRowActions may return null for rows without extra actions + const actions = actionsProp ?? []; - 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")} + aria-haspopup="true" + aria-expanded={anchorEl ? "true" : undefined} + 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 +102,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/editable-table/index.js b/src/components/mui/tables/editable-table/index.js index 8563ff42..b175c8e5 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, @@ -229,9 +229,6 @@ const MuiTableEditable = ({ /> ))} - {onEdit && ( - - )} {onArchive && ( )} - {onDelete && ( + {(onEdit || onDelete) && ( )} @@ -288,23 +285,6 @@ const MuiTableEditable = ({ )} ))} - {onEdit && ( - - onEdit(row)} - size="small" - aria-label={T.translate("general.edit")} - > - - - - )} {onArchive && ( )} - {onDelete && ( + {(onEdit || onDelete) && ( - handleDelete(row)} - size="small" - aria-label={T.translate("general.delete")} - > - - + onEdit(row))} + onDelete={onDelete && (() => handleDelete(row))} + disabled={!!(options.disableProp && row[options.disableProp])} + /> )} diff --git a/src/components/mui/tables/mui-table/index.js b/src/components/mui/tables/mui-table/index.js index fa34da32..797cc9c2 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"; @@ -75,17 +72,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); + 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 +85,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 +128,9 @@ const MuiTable = ({ /> ))} - {onEdit && } - {onArchive && } - {onDelete && } - {onSelect && } - {collapseActions && } + {onArchive && } + {hasRowMenu && } + {onSelect && } @@ -200,38 +152,10 @@ const MuiTable = ({ ))} - {/* 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,19 +223,10 @@ const MuiTable = ({ )} - {collapseActions && ( - - - - )} ))} - {/* 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 && ( @@ -344,7 +243,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, diff --git a/src/components/mui/tables/sortable-table/index.js b/src/components/mui/tables/sortable-table/index.js index 4f411558..a3e78ecb 100644 --- a/src/components/mui/tables/sortable-table/index.js +++ b/src/components/mui/tables/sortable-table/index.js @@ -20,8 +20,6 @@ import TableHead from "@mui/material/TableHead"; import TableRow from "@mui/material/TableRow"; 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 { DragDropContext, Draggable, Droppable } from "react-beautiful-dnd"; import styles from "../components/styles.module.less"; @@ -30,6 +28,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 { RESPONSIVE_TABLE_SX, getColumnWidthSx, @@ -120,8 +119,7 @@ const MuiTableSortable = ({ /> ))} - {onEdit && } - {onDelete && } + {(onEdit || onDelete) && } {onReorder && } @@ -176,36 +174,18 @@ const MuiTableSortable = ({ ))} - {/* Edit column */} - {onEdit && ( + {/* Row actions menu column */} + {(onEdit || onDelete) && ( - onEdit(row)} - sx={{ padding: 0 }} - > - - - - )} - {/* Delete column */} - {onDelete && ( - - handleDelete(row)} - sx={{ padding: 0 }} - > - - + onEdit(row))} + onDelete={onDelete && (() => handleDelete(row))} + disabled={!!(options.disableProp && row[options.disableProp])} + /> )} {/* 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",