Refactor/row actions

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 }) => (
-      <button type="button" onClick={onClick} {...rest}>
+    // forwardRef so the menu trigger's Tooltip can attach
+    default: React.forwardRef(({ children, onClick, ...rest }, ref) => (
+      <button type="button" ref={ref} onClick={onClick} {...rest}>
         {children}
       </button>
-    )
+    ))
   };
 });
 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");

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,26 @@ 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("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);

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) => (
+      <tr>
+        <td>{`extra row spans ${colCount}`}</td>
+      </tr>
+    ));
+    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();
     });
   });
+
 });

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 (
     <>
-      <IconButton
-        size="medium"
-        onClick={(e) => setAnchorEl(e.currentTarget)}
-        data-testid="row-actions-menu-trigger"
-      >
-        <MoreVertIcon />
-      </IconButton>
+      <Tooltip title={T.translate("general.more_actions")}>
+        <span>
+          <IconButton
+            size="medium"
+            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)
+            }
+          >
+            <MoreVertIcon />
+          </IconButton>
+        </span>
+      </Tooltip>
       <Menu
         anchorEl={anchorEl}
         open={Boolean(anchorEl)}
         onClose={() => setAnchorEl(null)}
       >
+        {onEdit && (
+          <MenuItem
+            disabled={disabled}
+            onClick={() => {
+              setAnchorEl(null);
+              onEdit();
+            }}
+          >
+            {T.translate("general.edit")}
+          </MenuItem>
+        )}
         {actions.map((action) => (
           <MenuItem
             key={action.label}
@@ -49,6 +77,19 @@ const RowActionsMenu = ({ actions }) => {
             {action.label}
           </MenuItem>
         ))}
+        {onDelete && (onEdit || actions.length > 0) && <Divider />}
+        {onDelete && (
+          <MenuItem
+            disabled={disabled}
+            onClick={() => {
+              setAnchorEl(null);
+              onDelete();
+            }}
+            sx={{ color: "error.main" }}
+          >
+            {T.translate("general.delete")}
+          </MenuItem>
+        )}
       </Menu>
     </>
   );
@@ -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;