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..f205fb5e 100644 --- a/src/components/mui/__tests__/mui-table.test.js +++ b/src/components/mui/__tests__/mui-table.test.js @@ -22,7 +22,7 @@ jest.mock("../showConfirmDialog", () => ({ })); import React from "react"; -import { render, screen } from "@testing-library/react"; +import { render, screen, fireEvent } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import "@testing-library/jest-dom"; import MuiTable from "../tables/mui-table"; @@ -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", () => { @@ -255,6 +281,59 @@ describe("MuiTable", () => { }); }); + describe("getHref link columns", () => { + // hash URLs: jsdom can't follow real page navigations + const linkColumns = [ + { ...columns[0], getHref: (row) => `#/people/${row.id}` }, + columns[1] + ]; + + test("renders the cell as a link to the row's href", () => { + setup({ columns: linkColumns }); + expect(screen.getByRole("link", { name: "Alice" })).toHaveAttribute("href", "#/people/1"); + }); + + test("a plain click calls onNavigate with the href and row", async () => { + const onNavigate = jest.fn(); + setup({ columns: linkColumns, onNavigate }); + await userEvent.click(screen.getByRole("link", { name: "Bob" })); + expect(onNavigate).toHaveBeenCalledWith("#/people/2", expect.objectContaining({ id: 2 })); + }); + + test("ctrl-click leaves the browser default so the link opens in a new tab", () => { + const onNavigate = jest.fn(); + setup({ columns: linkColumns, onNavigate }); + const notCancelled = fireEvent.click(screen.getByRole("link", { name: "Alice" }), { + ctrlKey: true + }); + expect(notCancelled).toBe(true); + expect(onNavigate).not.toHaveBeenCalled(); + }); + + test("a click that ends a text selection doesn't navigate", () => { + const onNavigate = jest.fn(); + const selection = jest + .spyOn(window, "getSelection") + .mockReturnValue({ toString: () => "Ali" }); + setup({ columns: linkColumns, onNavigate }); + const notCancelled = fireEvent.click(screen.getByRole("link", { name: "Alice" })); + expect(notCancelled).toBe(false); + expect(onNavigate).not.toHaveBeenCalled(); + selection.mockRestore(); + }); + + test("renders plain text when getHref returns nothing for a row", () => { + setup({ + columns: [ + { ...columns[0], getHref: (row) => (row.id === 2 ? "#/people/2" : null) }, + columns[1] + ] + }); + expect(screen.queryByRole("link", { name: "Alice" })).not.toBeInTheDocument(); + expect(screen.getByRole("link", { name: "Bob" })).toBeInTheDocument(); + }); + }); + describe("renderRow", () => { const customRow = { id: "custom-1", @@ -280,7 +359,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-cell-link.js b/src/components/mui/tables/components/table-cell-link.js new file mode 100644 index 00000000..cbc45623 --- /dev/null +++ b/src/components/mui/tables/components/table-cell-link.js @@ -0,0 +1,56 @@ +/** + * Copyright 2026 OpenStack Foundation + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * */ + +import * as React from "react"; +import PropTypes from "prop-types"; +import Link from "@mui/material/Link"; + +// real link so new tab / copy link work; plain clicks can be routed in-app via onNavigate +const TableCellLink = ({ href, row, onNavigate, children }) => { + const handleClick = (ev) => { + // finishing a text selection shouldn't navigate + if (window.getSelection()?.toString()) { + ev.preventDefault(); + return; + } + // new-tab / new-window clicks keep the browser's default + if (ev.button !== 0 || ev.metaKey || ev.ctrlKey || ev.shiftKey || ev.altKey) return; + if (onNavigate) { + ev.preventDefault(); + onNavigate(href, row); + } + }; + + return ( + + {children} + + ); +}; + +TableCellLink.propTypes = { + href: PropTypes.string.isRequired, + row: PropTypes.object.isRequired, + onNavigate: PropTypes.func, + children: PropTypes.node +}; + +export default TableCellLink; 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..081c0667 100644 --- a/src/components/mui/tables/mui-table/index.js +++ b/src/components/mui/tables/mui-table/index.js @@ -23,14 +23,13 @@ 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"; import ArrowForwardIcon from "@mui/icons-material/ArrowForward"; import PropTypes from "prop-types"; import TableCellContent from "../components/table-cell-content"; +import TableCellLink from "../components/table-cell-link"; import TableShell from "../components/table-shell"; import createDeleteHandler from "../components/create-delete-handler"; import SortableHeaderContent from "../components/sortable-header-content"; @@ -40,7 +39,6 @@ import { getColumnWidthSx, getArchivedRowSx, getActionCellSx, - getActionsMenuBreakpoint, ACTION_CELL_SX } from "../components/table-styles"; @@ -61,7 +59,9 @@ const MuiTable = ({ onEdit, onArchive, onDelete, + getRowActions, onSelect, + onNavigate, canDelete = () => true, deleteDialogTitle = null, deleteDialogBody = null, @@ -75,17 +75,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,49 +88,25 @@ const MuiTable = ({ ...(col.headSx || {}) }); - // individual action cells hide below collapseBreakpoint once actions collapse into a menu - const getIndividualActionSx = () => - collapseActions ? { display: { xs: "none", [collapseBreakpoint]: "table-cell" } } : {}; - - const menuCellSx = { display: { xs: "table-cell", [collapseBreakpoint]: "none" } }; - - const getRowActions = (row) => - [ - onEdit && { - label: T.translate("general.edit"), - onClick: () => onEdit(row), - disabled: !!(options.disableProp && row[options.disableProp]) - }, - onArchive && { - label: row.is_archived - ? T.translate("general.unarchive") - : T.translate("general.archive"), - onClick: () => onArchive(row), - disabled: !!( - options.disableProp && - options.disableProp !== "is_archived" && - row[options.disableProp] - ) - }, - onDelete && - canDelete(row) && { - label: T.translate("general.delete"), - onClick: () => handleDelete(row), - disabled: !!(options.disableProp && row[options.disableProp]) - }, - onSelect && { - label: T.translate("general.view"), - onClick: () => onSelect(row), - disabled: !!(options.disableProp && row[options.disableProp]) - } - ].filter(Boolean); - const getCellSx = (row, col) => ({ ...getColumnWidthSx(col), ...(col.cellSx || {}), ...getArchivedRowSx(row, options.disableProp) }); + // columns with getHref render as a link; no href for a row falls back to plain text + const renderCellContent = (row, col) => { + const content = ; + const href = col.getHref?.(row); + return href ? ( + + {content} + + ) : ( + content + ); + }; + const handleDelete = createDeleteHandler({ onDelete, getName, @@ -174,11 +144,9 @@ const MuiTable = ({ /> ))} - {onEdit && } - {onArchive && } - {onDelete && } - {onSelect && } - {collapseActions && } + {onArchive && } + {hasRowMenu && } + {onSelect && } @@ -188,7 +156,7 @@ const MuiTable = ({ {data.map((row) => row.renderRow ? ( {row.renderRow(totalColumnsCount)} ) : ( - + {/* Main content columns */} {columns.map((col) => ( - + {renderCellContent(row, 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 +240,6 @@ const MuiTable = ({ )} - {collapseActions && ( - - - - )} ))} {/* Here we inject extra rows passed as children */} @@ -358,7 +274,9 @@ MuiTable.propTypes = { onEdit: PropTypes.func, onArchive: PropTypes.func, onDelete: PropTypes.func, + getRowActions: PropTypes.func, onSelect: PropTypes.func, + onNavigate: PropTypes.func, canDelete: PropTypes.func, deleteDialogTitle: PropTypes.string, deleteDialogBody: PropTypes.oneOfType([PropTypes.func, 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 a3ebbaa6..7e19f986 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -31,6 +31,7 @@ "edit": "Edit", "delete": "Delete", "view": "View", + "more_actions": "More actions", "cancel": "Cancel", "apply": "Apply", "n_a": "N/A",