From d7aa936b806b0ae6fbffd5714fdb4b0c630db545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 7 Aug 2025 16:25:54 +0100 Subject: [PATCH 01/31] refactor(web): remove unnecessary wrapping section In the latest version of the Agama interface, Page.Sections elements are used to wrap specific regions of a page, not the entire page itself. This commit removes an outdated full-page wrapper at DASD page to align with that practice. --- web/src/components/storage/dasd/DASDPage.tsx | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/web/src/components/storage/dasd/DASDPage.tsx b/web/src/components/storage/dasd/DASDPage.tsx index 7f47a3c489..81a2276682 100644 --- a/web/src/components/storage/dasd/DASDPage.tsx +++ b/web/src/components/storage/dasd/DASDPage.tsx @@ -21,7 +21,7 @@ */ import React from "react"; -import { Content, Stack } from "@patternfly/react-core"; +import { Content } from "@patternfly/react-core"; import { Page } from "~/components/core"; import DASDTable from "./DASDTable"; import DASDFormatProgress from "./DASDFormatProgress"; @@ -40,12 +40,7 @@ export default function DASDPage() { - {/** TRANSLATORS: DASD devices selection table */} - - - - - + From 903f1696bd295e381c240d59ac3686f0a9c56954 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Fri, 8 Aug 2025 15:23:01 +0100 Subject: [PATCH 02/31] fix(web): support value-based item deselection Ensures that item deselection works even when the deselected object is equal in value but not the same instance (i.e. different memory reference). This can happen when operations like sorting produce a new list containing objects with the same content but different references. Previously, strict reference comparison prevented proper deselection in these cases. Now component uses Radashi's `isEqual` method, which relies on same-value equality using `Object.is()`[1], to correctly identify selected items. [1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Equality_comparisons_and_sameness#same-value_equality_using_object.is --- web/src/components/core/SelectableDataTable.test.tsx | 2 +- web/src/components/core/SelectableDataTable.tsx | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/web/src/components/core/SelectableDataTable.test.tsx b/web/src/components/core/SelectableDataTable.test.tsx index e800cbdf7c..3b7d2222f3 100644 --- a/web/src/components/core/SelectableDataTable.test.tsx +++ b/web/src/components/core/SelectableDataTable.test.tsx @@ -432,7 +432,7 @@ describe("SelectableDataTable", () => { describe("and user selects an already selected item", () => { it("triggers the `onSelectionChange` callback with a collection not including the item", async () => { const { user } = plainRender( - , + , ); const sda1row = screen.getByRole("row", { name: /dev\/sda1/ }); const sda1radio = within(sda1row).getByRole("checkbox"); diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index 7a66c9f450..f9f1f3aeb4 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -34,6 +34,7 @@ import { ThProps, TdProps, } from "@patternfly/react-table"; +import { isEqual } from "radashi"; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -279,7 +280,7 @@ export default function SelectableDataTable({ } if (isItemSelected(item)) { - onSelectionChange(selection.filter((i) => i !== item)); + onSelectionChange(selection.filter((i) => !isEqual(i, item))); } else { onSelectionChange([...selection, item]); } From a20612f349525311a193126ac4a92d406a9b5890 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sun, 10 Aug 2025 20:02:44 +0100 Subject: [PATCH 03/31] feat(web): add sorting support to core SelectableDataTable This makes it possible to use the centralized table wrapper with tables that require sorting behavior, such as the one listing DASD devices. By centralizing table logic, we ensure consistent look & feel, improve code reuse, and automatically benefit from enhancements applied to the shared component. --- .../core/SelectableDataTable.test.tsx | 67 +++++++- .../components/core/SelectableDataTable.tsx | 151 +++++++++++++++--- 2 files changed, 190 insertions(+), 28 deletions(-) diff --git a/web/src/components/core/SelectableDataTable.test.tsx b/web/src/components/core/SelectableDataTable.test.tsx index 3b7d2222f3..5d65fcc739 100644 --- a/web/src/components/core/SelectableDataTable.test.tsx +++ b/web/src/components/core/SelectableDataTable.test.tsx @@ -120,7 +120,7 @@ const vg = { const columns: SelectableDataTableColumn[] = [ // FIXME: do not use any but the right types once storage part is rewritten. // Or even better, write a test not coupled to storage - { name: "Device", value: (item: any) => item.name }, + { name: "Device", value: (item: any) => item.name, sortingKey: "name" }, { name: "Content", value: (item: any) => { @@ -130,7 +130,7 @@ const columns: SelectableDataTableColumn[] = [ return item.content; }, }, - { name: "Size", value: (item: any) => item.size }, + { name: "Size", value: (item: any) => item.size, sortingKey: "size" }, ]; const onChangeFn = jest.fn(); @@ -451,4 +451,67 @@ describe("SelectableDataTable", () => { }); }); }); + + describe("sorting", () => { + const updateSorting = jest.fn(); + + beforeEach(() => updateSorting.mockClear()); + + it("calls updateSorting with correct next direction when clicking the currently sorted column", async () => { + const { user } = plainRender( + , + ); + + const deviceHeader = screen.getByRole("columnheader", { name: "Device" }); + const deviceHeaderButton = within(deviceHeader).getByRole("button", { name: "Device" }); + await user.click(deviceHeaderButton); + + expect(updateSorting).toHaveBeenCalledWith({ + index: 0, + direction: "desc", + }); + }); + + it("does not call updateSorting when clicking on a non-sortable column", async () => { + const { user } = plainRender( + , + ); + + const contentHeader = screen.getByRole("columnheader", { name: "Content" }); + expect(within(contentHeader).queryByRole("button", { name: "Content" })).toBeNull(); + await user.click(contentHeader); + + expect(updateSorting).not.toHaveBeenCalled(); + }); + + it("calls updateSorting with ascending direction when clicking a new sortable column", async () => { + const { user } = plainRender( + , + ); + + const sizeHeader = screen.getByRole("columnheader", { name: "Size" }); + const sizeHeaderButton = within(sizeHeader).getByRole("button", { name: "Size" }); + await user.click(sizeHeaderButton); + + expect(updateSorting).toHaveBeenCalledWith({ + index: 2, + direction: "asc", + }); + }); + }); }); diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index f9f1f3aeb4..d5e025e1e7 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -34,22 +34,22 @@ import { ThProps, TdProps, } from "@patternfly/react-table"; -import { isEqual } from "radashi"; +import { isEqual, isFunction } from "radashi"; /* eslint-disable @typescript-eslint/no-explicit-any */ /** - * An object for sharing data across nested maps - * - * Since function arguments are always passed by value, an object passed by - * sharing is needed for sharing data that might be mutated from different - * places, as it is the case of the rowIndex prop here. - * - * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions#passing_arguments + * Represents the current sorting configuration. */ - -type SharedData = { - rowIndex: number; +export type SortedBy = { + /** + * Index of the column being sorted (given by PatternFly/TableTypes#onShort). + */ + index?: number; + /** + * Direction of the sort: ascending ("asc") or descending ("desc"). + */ + direction?: "asc" | "desc"; }; /** @@ -81,6 +81,12 @@ export type SelectableDataTableColumn = { */ value: (item: object) => React.ReactNode; + /** + * If defined, marks the column as sortable and specifies the key used for + * sorting. + */ + sortingKey?: string; + /** * A space-separated string of additional CSS class names to apply to the column's cells. * Useful for custom styling or conditional formatting. @@ -170,6 +176,17 @@ export type SelectableDataTableProps = { */ initialExpandedKeys?: any[]; + /** + * Current column index and direction used for sorting. + */ + sortedBy?: SortedBy; + + /** + * Optional callback to update sorting. If not provided, sorting is disabled. + * Called when a sortable column header is clicked. + */ + updateSorting?: (v: SortedBy) => void; + /** * Callback fired when the selection changes. * @@ -178,22 +195,98 @@ export type SelectableDataTableProps = { onSelectionChange?: (selection: T[]) => void; } & TableProps; +/** + * An internal utility object used to pass context and state between deeply + * nested render functions, such as those involved in building table rows and + * headers. + * + * JavaScript function arguments are passed by value, but objects are passed + * by reference. This allows `SharedData` to expose shared and optionally mutable + * data across nested calls. + * + * - `rowIndex` is intentionally mutable and is updated as rows are rendered. + * - `sortedBy` and `updateSorting` are read-only references to external sorting state. + * + * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions#passing_arguments + */ +type SharedData = { + /** + * Mutable counter tracking the current row index during rendering. Updated + * in-place to avoid recomputation or prop drilling. + */ + rowIndex: number; + /** + * Current column index and direction used for sorting. + */ + readonly sortedBy: SortedBy; + /** + * Callback to update sorting. If not provided, sorting is skipped. + */ + readonly updateSorting: (v: SortedBy) => void; +}; + +/** + * Build sorting properties for a given column header, enabling PatternFly table sorting. + */ +const buildSorting = ( + columnIndex: number, + column: SelectableDataTableColumn, + sharedData: SharedData, +): ThProps["sort"] | undefined => { + const { sortedBy, updateSorting } = sharedData; + const { sortingKey } = column; + + if (!sortedBy || !isFunction(updateSorting)) return undefined; + + if (!sortingKey) { + process.env.NODE_ENV === "development" && + console.error( + `Column ${column.name} (index ${columnIndex}) does not provide 'sortingKey', skipping sorting props`, + ); + return undefined; + } + + return { + sortBy: { + ...sortedBy, + defaultDirection: "asc", + }, + onSort: (_event, index, direction) => { + updateSorting({ index, direction }); + }, + columnIndex, + }; +}; + /** * Internal component for building the table header */ -const TableHeader = ({ columns }: { columns: SelectableDataTableColumn[] }) => ( - - - - - {columns?.map((c, i) => ( - - {c.name} - - ))} - - -); +const TableHeader = ({ + columns, + sharedData, +}: { + columns: SelectableDataTableColumn[]; + sharedData: SharedData; +}) => { + return ( + + + + + {columns?.map((c, i) => { + const sortProp = + sharedData.sortedBy && c.sortingKey ? buildSorting(i, c, sharedData) : undefined; + + return ( + + {c.name} + + ); + })} + + + ); +}; /** * Helper function to sanitize the `itemsSelected` prop value for the @@ -250,6 +343,8 @@ export default function SelectableDataTable({ itemsSelected = [], initialExpandedKeys = [], onSelectionChange, + sortedBy = {}, + updateSorting = undefined, ...tableProps }: SelectableDataTableProps) { const [expandedItemsKeys, setExpandedItemsKeys] = useState(initialExpandedKeys); @@ -364,13 +459,17 @@ export default function SelectableDataTable({ }; // @see SharedData - const sharedData = { rowIndex: 0 }; + const sharedData = { + rowIndex: 0, + sortedBy, + updateSorting, + }; const TableBody = () => items?.map((item) => renderItem(item, sharedData)); return ( - +
); From 8f61fb358660d626a45f7d55ace7dc96c41af6f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sun, 10 Aug 2025 23:37:44 +0100 Subject: [PATCH 04/31] feat(web): add select-all support to core SelectableDataTable Enables selecting or unselecting all items at once when using the "multiple" selection mode, by rendering a checkbox in the selection column header. --- .../core/SelectableDataTable.test.tsx | 69 +++++++++++++++++++ .../components/core/SelectableDataTable.tsx | 55 ++++++++++++++- 2 files changed, 122 insertions(+), 2 deletions(-) diff --git a/web/src/components/core/SelectableDataTable.test.tsx b/web/src/components/core/SelectableDataTable.test.tsx index 5d65fcc739..3fea0d4b04 100644 --- a/web/src/components/core/SelectableDataTable.test.tsx +++ b/web/src/components/core/SelectableDataTable.test.tsx @@ -514,4 +514,73 @@ describe("SelectableDataTable", () => { }); }); }); + + describe("select/unselect all checkbox", () => { + const onSelectionChange = jest.fn(); + + beforeEach(() => onSelectionChange.mockClear()); + + it("renders it only when selectionMode is multiple and allowSelectAll is true", () => { + const { rerender } = plainRender( + , + ); + + expect(screen.queryByRole("checkbox", { name: /select all/i })).toBeNull(); + + rerender(); + expect(screen.queryByRole("checkbox", { name: /select all/i })).toBeNull(); + + rerender(); + screen.getByRole("checkbox", { name: "Select all rows" }); + }); + + it("selects all items if it is clicked and not all items are selected", async () => { + const { user, rerender } = plainRender( + , + ); + + const selectAllCheckbox = screen.getByRole("checkbox", { name: "Select all rows" }); + await user.click(selectAllCheckbox); + + expect(onSelectionChange).toHaveBeenCalledWith(props.items); + + rerender( + , + ); + + await user.click(selectAllCheckbox); + + expect(onSelectionChange).toHaveBeenCalledWith(props.items); + }); + + it("unselects all items if it is clicked when all items are selected", async () => { + const { user } = plainRender( + , + ); + + // FIXME: label should be "Unselect" + const selectAllCheckbox = screen.getByRole("checkbox", { name: "Select all rows" }); + await user.click(selectAllCheckbox); + + expect(onSelectionChange).toHaveBeenCalledWith([]); + }); + }); }); diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index d5e025e1e7..8725e0e8d1 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -193,6 +193,20 @@ export type SelectableDataTableProps = { * Receives the updated array of selected items. */ onSelectionChange?: (selection: T[]) => void; + + /** + * If true, allows the user to select all rows via the header checkbox. + * + * Only applicable when `allowMultiple` is also true. When enabled, a checkbox + * will appear in the header row, letting users select or deselect all rows in + * one action. + * + * When toggled, `onSelectionChange` will be called with either: + * - the full `items` array (when selecting all), or + * - an empty array (when deselecting all). + * + */ + allowSelectAll?: boolean; } & TableProps; /** @@ -223,10 +237,31 @@ type SharedData = { * Callback to update sorting. If not provided, sorting is skipped. */ readonly updateSorting: (v: SortedBy) => void; + /** + * Whether the select/unselect-all feature is enabled and the header checkbox + * is shown. + */ + readonly allowSelectAll: boolean; + /** + * Whether multiple item selection is allowed. + */ + readonly allowMultiple: boolean; + /** + * Whether all items in the table are currently selected. + * Used to reflect checkbox state in the header. + */ + readonly isAllSelected: boolean; + /** + * Handles toggling selection of all items in the table. + * If `isSelecting` is `true`, all items are passed to `onSelectionChange`. + * If `false`, an empty array is passed instead (deselect all). + */ + readonly selectAll: (isSelecting: boolean) => void; }; /** - * Build sorting properties for a given column header, enabling PatternFly table sorting. + * Build sorting props for a given column header, enabling PatternFly table + * sorting. */ const buildSorting = ( columnIndex: number, @@ -268,11 +303,21 @@ const TableHeader = ({ columns: SelectableDataTableColumn[]; sharedData: SharedData; }) => { + const { allowMultiple, allowSelectAll, isAllSelected, selectAll } = sharedData; + + const selectAllProps = + allowMultiple && allowSelectAll + ? { + onSelect: (_event, isSelecting: boolean) => selectAll(isSelecting), + isSelected: isAllSelected, + } + : undefined; + return ( - + {columns?.map((c, i) => { const sortProp = sharedData.sortedBy && c.sortingKey ? buildSorting(i, c, sharedData) : undefined; @@ -345,6 +390,7 @@ export default function SelectableDataTable({ onSelectionChange, sortedBy = {}, updateSorting = undefined, + allowSelectAll = false, ...tableProps }: SelectableDataTableProps) { const [expandedItemsKeys, setExpandedItemsKeys] = useState(initialExpandedKeys); @@ -463,6 +509,11 @@ export default function SelectableDataTable({ rowIndex: 0, sortedBy, updateSorting, + allowMultiple, + allowSelectAll, + isAllSelected: itemsSelected.length === items.length, + selectAll: (isSelecting: boolean) => + isSelecting ? onSelectionChange(items) : onSelectionChange([]), }; const TableBody = () => items?.map((item) => renderItem(item, sharedData)); From 992f61d5a53bb53bc1f82f108b60518667330a36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Aug 2025 16:12:23 +0100 Subject: [PATCH 05/31] refactor(web): support custom item equality comparison Add the `itemEqualityFn` prop to core/SelectableDataTable to allow custom logic for comparing items during selection. This helps avoid issues with deep comparisons that might fail when a property changes in an object while the previous value is still held in the selected state. It also supports cases where items don't have a stable ID or require alternative comparison logic. By default, the component compares items using the `itemIdKey` (defaults to `"id"`). If the key is missing, strict equality (`===`) is used as a fallback. --- .../core/SelectableDataTable.test.tsx | 124 ++++++++++++++++++ .../components/core/SelectableDataTable.tsx | 32 ++++- 2 files changed, 151 insertions(+), 5 deletions(-) diff --git a/web/src/components/core/SelectableDataTable.test.tsx b/web/src/components/core/SelectableDataTable.test.tsx index 3fea0d4b04..75342692aa 100644 --- a/web/src/components/core/SelectableDataTable.test.tsx +++ b/web/src/components/core/SelectableDataTable.test.tsx @@ -259,6 +259,130 @@ describe("SelectableDataTable", () => { expect(sdaChild).not.toBeNull(); }); + describe("when not providing a custom item equality function", () => { + const onSelectionChange = jest.fn(); + + beforeEach(() => onSelectionChange.mockClear()); + + describe("and items has the given id property", () => { + it("selects an item correctly", async () => { + const { user } = plainRender( + , + ); + + const table = screen.getByRole("grid"); + const sdaRow = within(table).getByRole("row", { name: /dev\/sda 1024/ }); + const sdaCheckbox = within(sdaRow).getByRole("checkbox"); + await user.click(sdaCheckbox); + expect(onSelectionChange).toHaveBeenCalledWith([sda]); + }); + + it("unselects an item correctly", async () => { + const { user } = plainRender( + , + ); + + const table = screen.getByRole("grid"); + const sdaRow = within(table).getByRole("row", { name: /dev\/sda 1024/ }); + const sdaCheckbox = within(sdaRow).getByRole("checkbox"); + await user.click(sdaCheckbox); + expect(onSelectionChange).toHaveBeenCalledWith([]); + }); + }); + + describe("but items does not have the given id property", () => { + it("selects an item correctly", async () => { + const { user } = plainRender( + , + ); + + const table = screen.getByRole("grid"); + const sdaRow = within(table).getByRole("row", { name: /dev\/sda 1024/ }); + const sdaCheckbox = within(sdaRow).getByRole("checkbox"); + await user.click(sdaCheckbox); + expect(onSelectionChange).toHaveBeenCalledWith([sda]); + }); + + it("unselects an item correctly", async () => { + const { user } = plainRender( + , + ); + + const table = screen.getByRole("grid"); + const sdaRow = within(table).getByRole("row", { name: /dev\/sda 1024/ }); + const sdaCheckbox = within(sdaRow).getByRole("checkbox"); + await user.click(sdaCheckbox); + expect(onSelectionChange).toHaveBeenCalledWith([]); + }); + }); + }); + + describe("when providing a custom item equality function", () => { + const onSelectionChange = jest.fn(); + const itemEqualityFn = (a, b) => a.type === b.type && a.name === b.name; + + beforeEach(() => onSelectionChange.mockClear()); + + it("selects an item correctly", async () => { + const { user } = plainRender( + , + ); + + const table = screen.getByRole("grid"); + const sdaRow = within(table).getByRole("row", { name: /dev\/sda 1024/ }); + const sdaCheckbox = within(sdaRow).getByRole("checkbox"); + await user.click(sdaCheckbox); + expect(onSelectionChange).toHaveBeenCalledWith([sda]); + }); + + it("unselects an item correctly", async () => { + const { user } = plainRender( + , + ); + + const table = screen.getByRole("grid"); + const sdaRow = within(table).getByRole("row", { name: /dev\/sda 1024/ }); + const sdaCheckbox = within(sdaRow).getByRole("checkbox"); + await user.click(sdaCheckbox); + expect(onSelectionChange).toHaveBeenCalledWith([]); + }); + }); + describe("when `itemsSelected` is given", () => { it("renders nothing as checked if value is an empty array", () => { plainRender(); diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index 8725e0e8d1..6a81428fec 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -34,7 +34,7 @@ import { ThProps, TdProps, } from "@patternfly/react-table"; -import { isEqual, isFunction } from "radashi"; +import { isFunction } from "radashi"; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -166,6 +166,16 @@ export type SelectableDataTableProps = { */ itemClassNames?: (item: T) => string | undefined; + /** + * A function used to determine if two items are equal. + * + * It helps correctly identify and manipulate selected items, especially when + * items are objects that may not be referentially equal. The function should + * return `true` if `a` and `b` are considered the same item; `false` + * otherwise. + */ + itemEqualityFn?: (a: T, b: T) => boolean; + /** * Array of currently selected items. */ @@ -375,6 +385,13 @@ const sanitizeSelection = ( * For consistency, the selection API (`itemsSelected` and `onSelectionChange`) * always uses arrays, even when `selectionMode` is set to `"single"`. * + * By default, item equality is determined by comparing each item's `itemIdKey` + * property (which defaults to `"id"`). If the item does not have that key, a + * strict equality check (`===`) between the two items is performed. + * + * Such a comparasion can be overriden by providing a custom `itemEqualityFn` + * prop, for example, to perform deep comparison or other alternative logic. + * * @note It only accepts one nesting level. */ export default function SelectableDataTable({ @@ -385,12 +402,19 @@ export default function SelectableDataTable({ itemChildren = () => [], itemSelectable = () => true, itemClassNames = () => "", + itemEqualityFn = (a, b) => { + if (Object.hasOwn(a, itemIdKey)) { + return a[itemIdKey] === b[itemIdKey]; + } + return a === b; + }, itemsSelected = [], initialExpandedKeys = [], onSelectionChange, sortedBy = {}, updateSorting = undefined, allowSelectAll = false, + ...tableProps }: SelectableDataTableProps) { const [expandedItemsKeys, setExpandedItemsKeys] = useState(initialExpandedKeys); @@ -398,9 +422,7 @@ export default function SelectableDataTable({ const allowMultiple = selectionMode === "multiple"; const isItemSelected = (item: object) => { const selected = selection.find((selectionItem) => { - return ( - Object.hasOwn(selectionItem, itemIdKey) && selectionItem[itemIdKey] === item[itemIdKey] - ); + return itemEqualityFn(item, selectionItem); }); return selected !== undefined || selection.includes(item); @@ -421,7 +443,7 @@ export default function SelectableDataTable({ } if (isItemSelected(item)) { - onSelectionChange(selection.filter((i) => !isEqual(i, item))); + onSelectionChange(selection.filter((i) => !itemEqualityFn(i, item))); } else { onSelectionChange([...selection, item]); } From 01ba104d9ead5f331b9e2d01b51e29aab93b4010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Aug 2025 16:25:31 +0100 Subject: [PATCH 06/31] refactor(web): rebuild DASD devices table using SelectableDataTable Replaces the custom table implementation with the generic SelectableDataTable component, preserving the existing structure and behavior. This change removes the complex logic that handled selection persistence across filtering. That approach was not only hard to maintain but also introduced cognitive overhead and edge cases. Instead, selection is now cleared when filters are applied, leading to simpler, more predictable behavior. --- web/src/components/storage/dasd/DASDTable.tsx | 186 +++++++----------- 1 file changed, 68 insertions(+), 118 deletions(-) diff --git a/web/src/components/storage/dasd/DASDTable.tsx b/web/src/components/storage/dasd/DASDTable.tsx index e0363814d6..c09ea462a3 100644 --- a/web/src/components/storage/dasd/DASDTable.tsx +++ b/web/src/components/storage/dasd/DASDTable.tsx @@ -40,49 +40,67 @@ import { ToolbarGroup, ToolbarItem, } from "@patternfly/react-core"; -import { Page, Popup } from "~/components/core"; -import { Table, Thead, Tr, Th, Tbody, Td } from "@patternfly/react-table"; +import { Page, Popup, SelectableDataTable } from "~/components/core"; import { Icon } from "~/components/layout"; import { _ } from "~/i18n"; import { hex } from "~/utils"; import { sort } from "fast-sort"; import { DASDDevice } from "~/types/dasd"; import { useDASDDevices, useDASDMutation, useFormatDASDMutation } from "~/queries/storage/dasd"; - -// FIXME: please, note that this file still requiring refinements until reach a -// reasonable stable version -const columnData = (device: DASDDevice, column: { id: string; sortId?: string; label: string }) => { - let data = device[column.id]; - - switch (column.id) { - case "formatted": - case "diag": - if (!device.enabled) data = ""; - break; - case "partitionInfo": - data = data.split(",").map((d: string) =>
{d}
); - break; - } - - if (typeof data === "boolean") { - return data ? _("Yes") : _("No"); - } - - return data; -}; +import { SortedBy } from "~/components/core/SelectableDataTable"; const columns = [ - { id: "id", sortId: "hexId", label: _("Channel ID") }, - { id: "status", label: _("Status") }, - { id: "deviceName", label: _("Device") }, - { id: "deviceType", label: _("Type") }, - // TRANSLATORS: table header, the column contains "Yes"/"No" values - // for the DIAG access mode (special disk access mode on IBM mainframes), - // usually keep untranslated - { id: "diag", label: _("DIAG") }, - { id: "formatted", label: _("Formatted") }, - { id: "partitionInfo", label: _("Partition Info") }, + { + // TRANSLATORS: table header for a DASD devices table + name: _("Channel ID"), + sortingKey: "hexId", + value: (d: DASDDevice) => d.id, + }, + + { + // TRANSLATORS: table header for a DASD devices table + name: _("Status"), + value: (d: DASDDevice) => d.status, + sortingKey: "status", + }, + { + // TRANSLATORS: table header for a DASD devices table + name: _("Device"), + value: (d: DASDDevice) => d.deviceName, + sortingKey: "deviceName", + }, + { + // TRANSLATORS: table header for a DASD devices table + name: _("Type"), + value: (d: DASDDevice) => d.deviceType, + sortingKey: "deviceType", + }, + { + // TRANSLATORS: table header for `DIAG access mode` on DASD devices table. + // It refers to an special disk access mode on IBM mainframes. Keep + // untranslated. + name: _("DIAG"), + value: (d: DASDDevice) => { + if (!d.enabled) return; + d.diag ? _("Yes") : _("No"); + }, + sortingKey: "diag", + }, + { + // TRANSLATORS: table header for a column in a DASD devices table that + // usually contents Yes or No values + name: _("Formatted"), + value: (d: DASDDevice) => (d.formatted ? _("Yes") : _("No")), + sortingKey: "formatted", + }, + { + // TRANSLATORS: table header for a DASD devices table + name: _("Partition Info"), + value: (d: DASDDevice) => d.partitionInfo.split(",").map((d: string) =>
{d}
), + sortingKey: "partitionInfo", + }, ]; + const DevicesList = ({ devices }) => ( {devices.map((d: DASDDevice) => ( @@ -215,65 +233,24 @@ type FilterOptions = { minChannel?: string; maxChannel?: string; }; -type SelectionOptions = { - unselect?: boolean; - device?: DASDDevice; - devices?: DASDDevice[]; -}; export default function DASDTable() { const devices = useDASDDevices(); + const [sortedBy, updateSortedBy] = useState({ index: 0, direction: "asc" }); + const [selectedDASD, setSelectedDASD] = useState([]); const [{ minChannel, maxChannel }, setFilters] = useState({ minChannel: "", maxChannel: "", }); - const [sortingColumn, setSortingColumn] = useState(columns[0]); - const [sortDirection, setSortDirection] = useState<"asc" | "desc">("asc"); - - const sortColumnIndex = () => columns.findIndex((c) => c.id === sortingColumn.id); const filteredDevices = filterDevices(devices, minChannel, maxChannel); - const selectedDevicesIds = selectedDASD.map((d) => d.id); - - const changeSelected = (newSelection: SelectionOptions) => { - setSelectedDASD((prevSelection) => { - if (newSelection.unselect) { - if (newSelection.device) - return prevSelection.filter((d) => d.id !== newSelection.device.id); - if (newSelection.devices) return []; - } else { - if (newSelection.device) return [...prevSelection, newSelection.device]; - if (newSelection.devices) return newSelection.devices; - } - }); - }; - - // Selecting - const selectAll = (isSelecting = true) => { - changeSelected({ unselect: !isSelecting, devices: filteredDevices }); - }; - - const selectDevice = (device, isSelecting = true) => { - changeSelected({ unselect: !isSelecting, device }); - }; // Sorting // See https://github.com/snovakovic/fast-sort - const sortBy = sortingColumn.sortId || sortingColumn.id; - const sortedDevices = sort(filteredDevices)[sortDirection]((d) => d[sortBy]); - - // FIXME: this can be improved and even extracted to be used with other tables. - const getSortParams = (columnIndex) => { - return { - sortBy: { index: sortColumnIndex(), direction: sortDirection }, - onSort: (_event, index, direction) => { - setSortingColumn(columns[index]); - setSortDirection(direction); - }, - columnIndex, - }; - }; + const sortedDevices = sort(filteredDevices)[sortedBy.direction]( + (d) => d[columns[sortedBy.index].sortingKey], + ); const updateFilter = (newFilters: FilterOptions) => { setFilters((currentFilters) => ({ ...currentFilters, ...newFilters })); @@ -281,44 +258,17 @@ export default function DASDTable() { const PageContent = () => { return ( - - - - - ))} - - - - {sortedDevices.map((device, rowIndex) => ( - - - ))} - - ))} - -
selectAll(isSelecting), - isSelected: filteredDevices.length === selectedDASD.length, - }} - /> - {columns.map((column, index) => ( - - {column.label} -
selectDevice(device, isSelecting), - isSelected: selectedDevicesIds.includes(device.id), - isDisabled: false, - }} - /> - {columns.map((column) => ( - - {columnData(device, column)} -
+ ); }; From 4a5fd1dc86201d26a8e3163e06cd54735b3f0071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 11 Aug 2025 23:38:04 +0100 Subject: [PATCH 07/31] fix(web): return empty string to please type checking Since returning "void" does not fullfill the requirement of return a ReactNode. --- web/src/components/storage/dasd/DASDTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/storage/dasd/DASDTable.tsx b/web/src/components/storage/dasd/DASDTable.tsx index c09ea462a3..a1f5b5593b 100644 --- a/web/src/components/storage/dasd/DASDTable.tsx +++ b/web/src/components/storage/dasd/DASDTable.tsx @@ -81,7 +81,7 @@ const columns = [ // untranslated. name: _("DIAG"), value: (d: DASDDevice) => { - if (!d.enabled) return; + if (!d.enabled) return ""; d.diag ? _("Yes") : _("No"); }, sortingKey: "diag", From 5db12229865bb5fba6251920f6716dbd27cfed02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 12 Aug 2025 10:33:01 +0100 Subject: [PATCH 08/31] feat(web): allow row-level contextual actions Adds optional support for displaying per-row action menus in the SelectableDataTable component. When provided, an additional column is rendered containing a menu with actions specific to each item. --- .../core/SelectableDataTable.test.tsx | 25 +++++++ .../components/core/SelectableDataTable.tsx | 70 ++++++++++++++++++- 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/web/src/components/core/SelectableDataTable.test.tsx b/web/src/components/core/SelectableDataTable.test.tsx index 75342692aa..1e3ef448a5 100644 --- a/web/src/components/core/SelectableDataTable.test.tsx +++ b/web/src/components/core/SelectableDataTable.test.tsx @@ -192,6 +192,31 @@ describe("SelectableDataTable", () => { within(table).getByRole("row", { name: /Personal Data/ }); }); + it("renders actions column when itemActions is provided", async () => { + const editFn = jest.fn(); + const formatFn = jest.fn(); + + const { user } = plainRender( + [ + { title: `Edit ${d.name}`, onClick: editFn }, + { title: `Format ${d.name}`, onClick: formatFn }, + ]} + itemActionsLabel="Actions" + />, + ); + + const table = screen.getByRole("grid"); + const sda = within(table).getByRole("row", { name: /dev\/sda 1024/ }); + const sdaActions = within(sda).getByRole("button", { name: "Actions" }); + await user.click(sdaActions); + const menu = screen.getByRole("menu"); + const edit = within(menu).getByRole("menuitem", { name: "Edit /dev/sda" }); + await user.click(edit); + expect(editFn).toHaveBeenCalled(); + }); + it("renders a expand toggler in items with children", () => { plainRender(); const table = screen.getByRole("grid"); diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index 6a81428fec..efc01d6d73 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -21,6 +21,7 @@ */ import React, { useState } from "react"; +import { MenuToggle } from "@patternfly/react-core"; import { Table, TableProps, @@ -33,8 +34,11 @@ import { RowSelectVariant, ThProps, TdProps, + IAction, + ActionsColumn, } from "@patternfly/react-table"; import { isFunction } from "radashi"; +import Icon from "~/components/layout/Icon"; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -176,6 +180,34 @@ export type SelectableDataTableProps = { */ itemEqualityFn?: (a: T, b: T) => boolean; + /** + * Function to generate a list of actions for a given row. + * + * If provided, an additional column is rendered to display contextual actions. + * + * @example + * itemActions={(item) => [ + * { + * title: 'Edit', + * onClick: () => handleEdit(item), + * }, + * { + * title: 'Delete', + * onClick: () => handleDelete(item), + * isDanger: true, + * } + * ]} + */ + itemActions?: (d: T) => IAction[]; + + /** + * Accessible label for the actions menu toggle button. + * + * Used as the `aria-label` for the row action menu's trigger to improve + * accessibility. + */ + itemActionsLabel?: (d: T) => string | string; + /** * Array of currently selected items. */ @@ -267,6 +299,17 @@ type SharedData = { * If `false`, an empty array is passed instead (deselect all). */ readonly selectAll: (isSelecting: boolean) => void; + + /** + * Function to generate a list of row-level actions for a given item. + * If defined, a new actions column is rendered for contextual row menus. + */ + readonly itemActions: SelectableDataTableProps["itemActions"]; + /** + * Accessible label for the actions menu toggle in each row. + * Used as the `aria-label` for improved accessibility. + */ + readonly itemActionsLabel: SelectableDataTableProps["itemActionsLabel"]; }; /** @@ -313,7 +356,7 @@ const TableHeader = ({ columns: SelectableDataTableColumn[]; sharedData: SharedData; }) => { - const { allowMultiple, allowSelectAll, isAllSelected, selectAll } = sharedData; + const { allowMultiple, allowSelectAll, isAllSelected, selectAll, itemActions } = sharedData; const selectAllProps = allowMultiple && allowSelectAll @@ -338,6 +381,7 @@ const TableHeader = ({ ); })} + {itemActions && } ); @@ -414,7 +458,8 @@ export default function SelectableDataTable({ sortedBy = {}, updateSorting = undefined, allowSelectAll = false, - + itemActions, + itemActionsLabel, ...tableProps }: SelectableDataTableProps) { const [expandedItemsKeys, setExpandedItemsKeys] = useState(initialExpandedKeys); @@ -520,6 +565,25 @@ export default function SelectableDataTable({ {c.value(item)} ))} + {itemActions && ( + + ( + + + + )} + /> + + )} {renderChildren()} @@ -533,6 +597,8 @@ export default function SelectableDataTable({ updateSorting, allowMultiple, allowSelectAll, + itemActions, + itemActionsLabel, isAllSelected: itemsSelected.length === items.length, selectAll: (isSelecting: boolean) => isSelecting ? onSelectionChange(items) : onSelectionChange([]), From c9a692a8b5278379d818f1f05be110f9d29c0358 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 12 Aug 2025 10:42:33 +0100 Subject: [PATCH 09/31] refactor(web): improve typing By using Readonly and Pick utility types to avoid duplicating definition and documentation of some properties. --- .../components/core/SelectableDataTable.tsx | 34 +++++-------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index efc01d6d73..763695a5ad 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -271,46 +271,30 @@ type SharedData = { * in-place to avoid recomputation or prop drilling. */ rowIndex: number; - /** - * Current column index and direction used for sorting. - */ - readonly sortedBy: SortedBy; - /** - * Callback to update sorting. If not provided, sorting is skipped. - */ - readonly updateSorting: (v: SortedBy) => void; - /** - * Whether the select/unselect-all feature is enabled and the header checkbox - * is shown. - */ - readonly allowSelectAll: boolean; + /** * Whether multiple item selection is allowed. */ readonly allowMultiple: boolean; + /** * Whether all items in the table are currently selected. * Used to reflect checkbox state in the header. */ readonly isAllSelected: boolean; + /** * Handles toggling selection of all items in the table. * If `isSelecting` is `true`, all items are passed to `onSelectionChange`. * If `false`, an empty array is passed instead (deselect all). */ readonly selectAll: (isSelecting: boolean) => void; - - /** - * Function to generate a list of row-level actions for a given item. - * If defined, a new actions column is rendered for contextual row menus. - */ - readonly itemActions: SelectableDataTableProps["itemActions"]; - /** - * Accessible label for the actions menu toggle in each row. - * Used as the `aria-label` for improved accessibility. - */ - readonly itemActionsLabel: SelectableDataTableProps["itemActionsLabel"]; -}; +} & Readonly< + Pick< + SelectableDataTableProps, + "sortedBy" | "updateSorting" | "allowSelectAll" | "itemActions" | "itemActionsLabel" + > +>; /** * Build sorting props for a given column header, enabling PatternFly table From 75208307e2c2f19f3cee5fc0d23598d3fffed4cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 12 Aug 2025 16:30:18 +0100 Subject: [PATCH 10/31] feat(web): add shortcut for dangerous actions in core/Popup --- web/src/components/core/Popup.test.tsx | 9 +++++++++ web/src/components/core/Popup.tsx | 17 +++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/web/src/components/core/Popup.test.tsx b/web/src/components/core/Popup.test.tsx index 36e063b38f..0ce8ad30ef 100644 --- a/web/src/components/core/Popup.test.tsx +++ b/web/src/components/core/Popup.test.tsx @@ -180,6 +180,15 @@ describe("Popup.AncillaryAction", () => { }); }); +describe("Popup.DangerousAction", () => { + it("renders a 'danger' button with given children as content", async () => { + installerRender(Format everything); + + const button = screen.queryByRole("button", { name: "Format everything" }); + expect(button.classList.contains("pf-m-danger")).toBe(true); + }); +}); + describe("Popup.Confirm", () => { describe("when holding no children", () => { it("renders a 'primary' button using 'Confirm' text as content", async () => { diff --git a/web/src/components/core/Popup.tsx b/web/src/components/core/Popup.tsx index 30a3978454..83c2584d76 100644 --- a/web/src/components/core/Popup.tsx +++ b/web/src/components/core/Popup.tsx @@ -164,6 +164,22 @@ const AncillaryAction = ({ children, ...actionsProps }: PredefinedAction) => ( ); +/** + * A Popup action with danger variant + * + * It always set `variant` {@link https://www.patternfly.org/components/button PF/Button} + * prop to "danger", no matter what given in `props`. + * + * @example Simple usage + * Format + * + */ +const DangerousAction = ({ children, ...actionProps }: PredefinedAction) => ( + + {children} + +); + /** * Agama component for displaying a popup * @@ -245,6 +261,7 @@ const Popup = ({ Popup.Actions = Actions; Popup.PrimaryAction = PrimaryAction; +Popup.DangerousAction = DangerousAction; Popup.Confirm = Confirm; Popup.SecondaryAction = SecondaryAction; Popup.Cancel = Cancel; From 757725cdea9f3d0603f522eb2be7eb797f94d26d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 12 Aug 2025 17:35:45 +0100 Subject: [PATCH 11/31] refactor(web): change how DASD actions are shown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moved device actions to a toolbar above the table that only shows up when there’s at least one device selected. A hint is shown when nothing is selected to let users know actions will be available there. This replaces the former "Perform an action" menu, which was disabled until something was selected and could be confusing at first glance. This should improve usability by making all available actions visible upfront and saving clicks. Additionally, per-device actions are now accessible via an actions menu in the "Actions" column, making it easier to perform actions individually. A better code organization is expected in upcoming commits. --- .../storage/dasd/DASDTable.test.tsx | 62 +-- web/src/components/storage/dasd/DASDTable.tsx | 503 ++++++++++++------ 2 files changed, 353 insertions(+), 212 deletions(-) diff --git a/web/src/components/storage/dasd/DASDTable.test.tsx b/web/src/components/storage/dasd/DASDTable.test.tsx index 78d765527b..a151d6a10e 100644 --- a/web/src/components/storage/dasd/DASDTable.test.tsx +++ b/web/src/components/storage/dasd/DASDTable.test.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2024] SUSE LLC + * Copyright (c) [2024-2025] SUSE LLC * * All Rights Reserved. * @@ -70,49 +70,37 @@ describe("DASDTable", () => { screen.getByText("active"); }); - it("does not allow to perform any action if not selected any device", () => { - installerRender(); - const button = screen.getByRole("button", { name: "Perform an action" }); - expect(button).toHaveAttribute("disabled"); + it("does not offer bulk actions until a device is selected", async () => { + const { user } = installerRender(); + screen.getByText("Select devices to enable bulk actions."); + expect(screen.queryByRole("button", { name: "Activate" })).toBeNull(); + const selection = screen.getByRole("checkbox", { name: "Select row 0" }); + await user.click(selection); + expect(screen.queryByText("Select devices to enable bulk actions.")).toBeNull(); + screen.getByRole("button", { name: "Activate" }); }); - describe("when there are some DASD selected", () => { - it("allows to perform a set of actions over them", async () => { + describe("when format action is requested", () => { + // TODO: Add more scenarios to individually test these cases + // for both situations: when a single device is selected and when multiple devices are selected. + it("shows a confirmation dialog if all the devices are online", async () => { const { user } = installerRender(); - const selection = screen.getByRole("checkbox", { name: "Select row 0" }); + const selection = screen.getByRole("checkbox", { name: "Select row 1" }); await user.click(selection); - const button = screen.getByRole("button", { name: "Perform an action" }); - expect(button).not.toHaveAttribute("disabled"); + const button = screen.getByRole("button", { name: "Format" }); await user.click(button); - screen.getByRole("menuitem", { name: "Format" }); + screen.getByRole("dialog", { name: /Format/ }); }); - describe("and the user click on format", () => { - it("shows a confirmation dialog if all the devices are online", async () => { - const { user } = installerRender(); - const selection = screen.getByRole("checkbox", { name: "Select row 1" }); - await user.click(selection); - const button = screen.getByRole("button", { name: "Perform an action" }); - expect(button).not.toHaveAttribute("disabled"); - await user.click(button); - const format = screen.getByRole("menuitem", { name: "Format" }); - await user.click(format); - screen.getByRole("dialog", { name: "Format selected devices?" }); - }); - - it("shows a warning dialog if some device is offline", async () => { - const { user } = installerRender(); - let selection = screen.getByRole("checkbox", { name: "Select row 0" }); - await user.click(selection); - selection = screen.getByRole("checkbox", { name: "Select row 1" }); - await user.click(selection); - const button = screen.getByRole("button", { name: "Perform an action" }); - expect(button).not.toHaveAttribute("disabled"); - await user.click(button); - const format = screen.getByRole("menuitem", { name: "Format" }); - await user.click(format); - screen.getByRole("dialog", { name: "Cannot format all selected devices" }); - }); + it("shows a warning dialog if some device is offline", async () => { + const { user } = installerRender(); + let selection = screen.getByRole("checkbox", { name: "Select row 0" }); + await user.click(selection); + selection = screen.getByRole("checkbox", { name: "Select row 1" }); + await user.click(selection); + const button = screen.getByRole("button", { name: "Format" }); + await user.click(button); + screen.getByRole("dialog", { name: /Cannot/ }); }); }); }); diff --git a/web/src/components/storage/dasd/DASDTable.tsx b/web/src/components/storage/dasd/DASDTable.tsx index a1f5b5593b..7cb7796d87 100644 --- a/web/src/components/storage/dasd/DASDTable.tsx +++ b/web/src/components/storage/dasd/DASDTable.tsx @@ -20,17 +20,13 @@ * find current contact information at www.suse.com. */ -import React, { useState } from "react"; +import React, { useReducer, useState } from "react"; import { Button, Content, Divider, - Dropdown, - DropdownItem, - DropdownList, List, ListItem, - MenuToggle, Stack, TextInputGroup, TextInputGroupMain, @@ -40,14 +36,16 @@ import { ToolbarGroup, ToolbarItem, } from "@patternfly/react-core"; -import { Page, Popup, SelectableDataTable } from "~/components/core"; +import { Popup, SelectableDataTable } from "~/components/core"; import { Icon } from "~/components/layout"; -import { _ } from "~/i18n"; +import { _, n_ } from "~/i18n"; import { hex } from "~/utils"; import { sort } from "fast-sort"; import { DASDDevice } from "~/types/dasd"; import { useDASDDevices, useDASDMutation, useFormatDASDMutation } from "~/queries/storage/dasd"; -import { SortedBy } from "~/components/core/SelectableDataTable"; +import type { SortedBy } from "~/components/core/SelectableDataTable"; +import { sprintf } from "sprintf-js"; +import Text from "~/components/core/Text"; const columns = [ { @@ -108,119 +106,122 @@ const DevicesList = ({ devices }) => ( ))} ); -const FormatNotPossible = ({ devices, onAccept }) => ( - - - - {_( - "Offline devices must be activated before formatting them. Please, unselect or activate the devices listed below and try it again", - )} - - - - - {_("Accept")} - - -); - -const FormatConfirmation = ({ devices, onCancel, onConfirm }) => ( - - - - {_( - "This action could destroy any data stored on the devices listed below. Please, confirm that you really want to continue.", - )} - - - - - - - - -); - -const Actions = ({ devices, isDisabled }: { devices: DASDDevice[]; isDisabled: boolean }) => { - const { mutate: updateDASD } = useDASDMutation(); - const { mutate: formatDASD } = useFormatDASDMutation(); - const [isOpen, setIsOpen] = useState(false); - const [requestFormat, setRequestFormat] = useState(false); - const onToggle = () => setIsOpen(!isOpen); - const onSelect = () => setIsOpen(false); - const cancelFormatRequest = () => setRequestFormat(false); +/** + * Renders a popup indicating a specific device is offline. + * Used when attempting to format a single device that is disabled. + */ +const DeviceOffline = ({ device, onCancel }) => { + return ( + + + {_("It is offline and must be activated before formatting it.")} + + + {_("Accept")} + + + ); +}; - const deviceIds = devices.map((d) => d.id); +/** + * Shows a popup listing multiple offline devices, + * preventing a format action on them. + */ +const SomeDevicesOffline = ({ devices, onCancel }) => { const offlineDevices = devices.filter((d) => !d.enabled); - const offlineDevicesSelected = offlineDevices.length > 0; - const activate = () => updateDASD({ action: "enable", devices: deviceIds }); - const deactivate = () => updateDASD({ action: "disable", devices: deviceIds }); - const setDiagOn = () => updateDASD({ action: "diagOn", devices: deviceIds }); - const setDiagOff = () => updateDASD({ action: "diagOff", devices: deviceIds }); - const format = () => formatDASD(devices.map((d) => d.id)); + const totalOffline = offlineDevices.length; - const Action = ({ children, ...props }) => ( - - {children} - + return ( + + + + {sprintf(_("Below %s devices are offline and cannot be formatted."), totalOffline)} + + {_("Unselect or activate them and try it again.")} + + + + {_("Accept")} + + ); +}; +/** + * Renders a format confirmation dialog for formatting a single device. + */ +const DeviceFormatConfirmation = ({ device, onAccept, onCancel }) => { return ( - <> - {requestFormat && offlineDevicesSelected && ( - - )} + + + + {_("This action could destroy any data stored on the device.")} + {_("Confirm that you really want to continue.")} + + + + {_("Format now")} + + + + ); +}; - {requestFormat && !offlineDevicesSelected && ( - { - cancelFormatRequest(); - format(); - }} - /> - )} - ( - - {/* TRANSLATORS: drop down menu label */} - {_("Perform an action")} - - )} - > - - {/** TRANSLATORS: drop down menu action, activate the device */} - - {_("Activate")} - - {/** TRANSLATORS: drop down menu action, deactivate the device */} - - {_("Deactivate")} - - - {/** TRANSLATORS: drop down menu action, enable DIAG access method */} - - {_("Set DIAG On")} - - {/** TRANSLATORS: drop down menu action, disable DIAG access method */} - - {_("Set DIAG Off")} - - - {/** TRANSLATORS: drop down menu action, format the disk */} - setRequestFormat(true)}> - {_("Format")} - - - - +/** + * Renders a confirmation dialog for formatting multiple selected devices. + */ +const MultipleDevicesFormatConfirmation = ({ devices, onAccept, onCancel }) => { + return ( + + + + + {_("This action could destroy any data stored on the devices listed below.")} + + + {_("Confirm that you really want to continue.")} + + + + {_("Format now")} + + + ); }; +/** + * Central dispatcher component for rendering the appropriate format dialog, + * based on whether the selected devices are online/offline and how many are + * selected. + */ +const FormatRequestHandler = ({ devices, onAccept, onCancel }) => { + const { mutate: formatDASD } = useFormatDASDMutation(); + const format = () => { + formatDASD(devices.map((d) => d.id)); + onAccept(); + }; + + if (devices.length === 1) { + const device = devices[0]; + + if (device.enabled) { + return ; + } else { + return ; + } + } + + if (devices.some((d) => !d.enabled)) { + return ; + } else { + return ( + + ); + } +}; + const filterDevices = (devices: DASDDevice[], from: string, to: string): DASDDevice[] => { const allChannels = devices.map((d) => d.hexId); const min = hex(from) || Math.min(...allChannels); @@ -234,8 +235,124 @@ type FilterOptions = { maxChannel?: string; }; +/** + * Represents the component state. + */ +type DASDTableState = { + formatRequested: boolean; + selectedDevices: DASDDevice[]; +}; + +/** + * Supported actions. + */ +type DASDTableAction = + | { type: "REQUEST_FORMAT"; devices: DASDTableState["selectedDevices"] } + | { type: "CANCEL_FORMAT_REQUEST" }; + +/** + * Reducer for triggering actions. + */ +const reducer = (state: DASDTableState, action: DASDTableAction): DASDTableState => { + switch (action.type) { + case "REQUEST_FORMAT": { + return { ...state, formatRequested: true, selectedDevices: action.devices }; + } + + case "CANCEL_FORMAT_REQUEST": { + return { ...state, formatRequested: false, selectedDevices: [] }; + } + } +}; + +/** + * Provides individual action buttons for a group of selected DASD devices. + */ +const BulkActions = ({ devices, onFormatRequest }) => { + const { mutate: updateDASD } = useDASDMutation(); + + const devicesIds = devices.map((d) => d.id); + const actions = [ + { + title: _("Activate"), + onClick: () => updateDASD({ action: "enable", devices: devicesIds }), + }, + { + title: _("Deactivate"), + onClick: () => updateDASD({ action: "disable", devices: devicesIds }), + }, + { + isSeparator: true, + }, + { + title: _("Set DIAG on"), + onClick: () => updateDASD({ action: "diagOn", devices: devicesIds }), + }, + { + title: _("Set DIAG off"), + onClick: () => updateDASD({ action: "diagOff", devices: devicesIds }), + }, + { + isSeparator: true, + }, + { + title: _("Format"), + onClick: () => onFormatRequest(devices), + }, + ]; + + return actions + .filter((a) => !a.isSeparator) + .map(({ onClick, title }, i) => ( + + + + )); +}; +/** + * Toolbar section displaying available bulk actions for selected devices. + * Dynamically adjusted based on selection count. + */ +const ActionsToolbar = ({ devices, onFormatRequest }) => { + const text = sprintf( + n_( + // TRANSLATORS: message shown in bulk action toolbar when just one device + // is selected + "Apply to the selected device", + // TRANSLATORS: message shown in bulk action toolbar when some devices are + // selected. %s is replaced with the amount of devices + "Apply to the %s selected devices", + devices.length, + ), + devices.length, + ); + + return ( + + + + {devices.length ? ( + <> + {text} + + ) : ( + _("Select devices to enable bulk actions.") + )} + + + + ); +}; + export default function DASDTable() { const devices = useDASDDevices(); + const { mutate: updateDASD } = useDASDMutation(); + const [state, dispatch] = useReducer(reducer, { + formatRequested: false, + selectedDevices: [], + }); const [sortedBy, updateSortedBy] = useState({ index: 0, direction: "asc" }); const [selectedDASD, setSelectedDASD] = useState([]); @@ -256,8 +373,74 @@ export default function DASDTable() { setFilters((currentFilters) => ({ ...currentFilters, ...newFilters })); }; - const PageContent = () => { - return ( + return ( + <> + + + + + + + updateFilter({ minChannel })} + /> + {minChannel !== "" && ( + + - - )); +/** + * Props for the FiltersToolbar component used in the DASD table. + */ +type FiltersToolbarProps = { + /** Current filter state */ + filters: DASDDevicesFilters; + /** Callback invoked when a filter value changes. */ + onFilterChange: (filter: keyof DASDDevicesFilters, value: string | number) => void; }; + /** - * Toolbar section displaying available bulk actions for selected devices. - * Dynamically adjusted based on selection count. + * Renders the toolbar used to filter DASD devices. */ -const ActionsToolbar = ({ devices, onFormatRequest }) => { - const text = sprintf( +const FiltersToolbar = ({ filters, onFilterChange }: FiltersToolbarProps) => ( + + + + + onFilterChange("status", v)} /> + + + onFilterChange("formatted", v)} + /> + + + onFilterChange("minChannel", v)} + /> + + + onFilterChange("maxChannel", v)} + /> + + + + +); + +/** + * Displays a toolbar containing bulk action buttons for selected DASD devices. + * + * If devices are selected, shows available actions; otherwise, displays an + * instructional message. Depends on the same props as `buildActions`. + */ +const BulkActionsToolbar = ({ devices, updater, dispatcher }: DASDActionsBuilderProps) => { + const applyText = sprintf( n_( // TRANSLATORS: message shown in bulk action toolbar when just one device // is selected @@ -164,7 +227,16 @@ const ActionsToolbar = ({ devices, onFormatRequest }) => { {devices.length ? ( <> - {text} + {applyText}{" "} + {buildActions({ devices, updater, dispatcher }) + .filter((a) => !a.isSeparator) + .map(({ onClick, title }, i) => ( + + + + ))} ) : ( _("Select devices to enable bulk actions.") @@ -192,10 +264,10 @@ type DASDTableState = { const initialState: DASDTableState = { sortedBy: { index: 0, direction: "asc" }, filters: { - minChannel: "", - maxChannel: "", status: "all", formatted: "all", + minChannel: "", + maxChannel: "", }, selectedDevices: [], devicesToFormat: [], @@ -341,51 +413,13 @@ export default function DASDTable() { ); return ( - <> - - - - - - onFilterChange("status", v)} - /> - - - onFilterChange("formatted", v)} - /> - - - onFilterChange("minChannel", v)} - /> - - - onFilterChange("maxChannel", v)} - /> - - - - - - - dispatch({ type: "REQUEST_FORMAT", payload: state.selectedDevices }) - } - /> - - + + + {!isEmpty(state.devicesToFormat) && ( dispatch({ type: "CANCEL_FORMAT_REQUEST" })} /> )} + [ - { - title: _("Activate"), - onClick: () => updateDASD({ action: "enable", devices: [d.id] }), - }, - { - title: _("Deactivate"), - onClick: () => updateDASD({ action: "disable", devices: [d.id] }), - }, - { - isSeparator: true, - }, - { - title: _("Set DIAG on"), - onClick: () => updateDASD({ action: "diagOn", devices: [d.id] }), - }, - { - title: _("Set DIAG off"), - onClick: () => updateDASD({ action: "diagOff", devices: [d.id] }), - }, - { - isSeparator: true, - }, - { - title: _("Format"), - isDanger: true, - onClick: () => { - dispatch({ type: "REQUEST_FORMAT", payload: [d] }); - // formatDASD([d.id]), - }, - }, - ]} + itemActions={(d) => + buildActions({ + devices: [d], + updater: updateDASD, + dispatcher: dispatch, + }) + } itemActionsLabel={(d) => `Actions for ${d.id}`} /> - + ); } diff --git a/web/src/queries/storage/dasd.ts b/web/src/queries/storage/dasd.ts index 85eab88388..c75c2432f6 100644 --- a/web/src/queries/storage/dasd.ts +++ b/web/src/queries/storage/dasd.ts @@ -184,10 +184,17 @@ const useDASDDevicesChanges = () => { return devices; }; +export type DASDMutationFnProps = { + action: "enable" | "disable" | "diagOn" | "diagOff"; + devices: DASDDevice["id"][]; +}; + +export type DASDMutationFn = (props: DASDMutationFnProps) => void; + const useDASDMutation = () => { const queryClient = useQueryClient(); const query = { - mutationFn: ({ action, devices }: { action: string; devices: string[] }) => { + mutationFn: ({ action, devices }: DASDMutationFnProps) => { switch (action) { case "enable": { return enableDASD(devices); From 969b04c5560e56d840a5be8728ecd1091d744a07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sat, 16 Aug 2025 23:37:07 +0100 Subject: [PATCH 19/31] feat(web): add support for EmptyState in core/SelectableDataTable Adds a new `emptyState` prop that allows rendering custom content when the items array is empty. The empty state spans all columns (including selection and actions, if enabled). --- .../core/SelectableDataTable.test.tsx | 32 +++++++++++++++++ .../components/core/SelectableDataTable.tsx | 35 +++++++++++++++++-- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/web/src/components/core/SelectableDataTable.test.tsx b/web/src/components/core/SelectableDataTable.test.tsx index 1e3ef448a5..65d1613f4b 100644 --- a/web/src/components/core/SelectableDataTable.test.tsx +++ b/web/src/components/core/SelectableDataTable.test.tsx @@ -732,4 +732,36 @@ describe("SelectableDataTable", () => { expect(onSelectionChange).toHaveBeenCalledWith([]); }); }); + + describe("EmptyState support", () => { + it("renders no tbody when items is empty and no emptyState is provided", () => { + plainRender( + , + ); + const table = screen.getByRole("grid"); + expect(table.querySelectorAll("tbody").length).toBe(0); + }); + + it("renders emptyState when items is empty", () => { + plainRender( + Resources not found} + />, + ); + + expect(screen.getByText("Resources not found")).toBeInTheDocument(); + }); + + it("does not render emptyState when items are present", () => { + plainRender(Resources not found} />); + + expect(screen.queryByText("Resources not found")).toBeNull(); + const table = screen.getByRole("grid"); + within(table).getByRole("row", { name: /dev\/sda 1024/ }); + }); + }); }); diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index 763695a5ad..5b560d1f15 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -21,7 +21,7 @@ */ import React, { useState } from "react"; -import { MenuToggle } from "@patternfly/react-core"; +import { Bullseye, MenuToggle } from "@patternfly/react-core"; import { Table, TableProps, @@ -37,7 +37,7 @@ import { IAction, ActionsColumn, } from "@patternfly/react-table"; -import { isFunction } from "radashi"; +import { isEmpty, isFunction } from "radashi"; import Icon from "~/components/layout/Icon"; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -249,6 +249,15 @@ export type SelectableDataTableProps = { * */ allowSelectAll?: boolean; + + /** + * Renders a custom empty state when `items` is empty. + * + * If not provided, the table will simply render no rows. When provided, this + * content will be shown inside a full-width row spanning all columns + * (including selection and action columns, if enabled). + */ + emptyState?: React.ReactNode; } & TableProps; /** @@ -444,6 +453,7 @@ export default function SelectableDataTable({ allowSelectAll = false, itemActions, itemActionsLabel, + emptyState, ...tableProps }: SelectableDataTableProps) { const [expandedItemsKeys, setExpandedItemsKeys] = useState(initialExpandedKeys); @@ -588,7 +598,26 @@ export default function SelectableDataTable({ isSelecting ? onSelectionChange(items) : onSelectionChange([]), }; - const TableBody = () => items?.map((item) => renderItem(item, sharedData)); + // TODO: extract to a separate component and inject sharedData as prop + const TableEmptyState = () => { + const columnsCount = + columns.length + (sharedData.allowSelectAll && 1) + (sharedData.itemActions && 1); + + return ( + + + {emptyState} + + + ); + }; + + const TableBody = () => { + if (isEmpty(items) && emptyState) { + return ; + } + return items?.map((item) => renderItem(item, sharedData)); + }; return ( From 92fbdcef04b1d51bf1fcaf00f2ffffee6286e1e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sat, 16 Aug 2025 23:46:31 +0100 Subject: [PATCH 20/31] feat(web): show empty state in DASDTable when there are no devices Renders a friendly interface when the DASD device list is empty, including a custom icon, explanation message, and a "Clear all filters" action. --- web/src/components/layout/Icon.tsx | 2 ++ .../storage/dasd/DASDTable.test.tsx | 12 +++++++ web/src/components/storage/dasd/DASDTable.tsx | 35 +++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/web/src/components/layout/Icon.tsx b/web/src/components/layout/Icon.tsx index 9963a2a4d6..ceacd9ad8a 100644 --- a/web/src/components/layout/Icon.tsx +++ b/web/src/components/layout/Icon.tsx @@ -54,6 +54,7 @@ import NetworkWifi from "@icons/network_wifi.svg?component"; import NetworkWifi1Bar from "@icons/network_wifi_1_bar.svg?component"; import NetworkWifi3Bar from "@icons/network_wifi_3_bar.svg?component"; import Translate from "@icons/translate.svg?component"; +import SearchOff from "@icons/search_off.svg?component"; import SettingsEthernet from "@icons/settings_ethernet.svg?component"; import UnfoldLess from "@icons/unfold_less.svg?component"; import UnfoldMore from "@icons/unfold_more.svg?component"; @@ -92,6 +93,7 @@ const icons = { network_wifi: NetworkWifi, network_wifi_1_bar: NetworkWifi1Bar, network_wifi_3_bar: NetworkWifi3Bar, + search_off: SearchOff, settings_ethernet: SettingsEthernet, translate: Translate, unfold_less: UnfoldLess, diff --git a/web/src/components/storage/dasd/DASDTable.test.tsx b/web/src/components/storage/dasd/DASDTable.test.tsx index c6dfd4c600..6ba67d4a6b 100644 --- a/web/src/components/storage/dasd/DASDTable.test.tsx +++ b/web/src/components/storage/dasd/DASDTable.test.tsx @@ -93,4 +93,16 @@ describe("DASDTable", () => { screen.getByText("FormatActionHandler Mock"); }); }); + + describe("when there is some DASD devices available", () => { + beforeEach(() => { + mockDASDDevices = []; + }); + + it("renders empty state when there are no devices", () => { + installerRender(); + screen.getByRole("heading", { name: "No devices found", level: 2 }); + screen.getByRole("button", { name: "Clear all filters" }); + }); + }); }); diff --git a/web/src/components/storage/dasd/DASDTable.tsx b/web/src/components/storage/dasd/DASDTable.tsx index 0548a3d473..ac4f5d298b 100644 --- a/web/src/components/storage/dasd/DASDTable.tsx +++ b/web/src/components/storage/dasd/DASDTable.tsx @@ -24,11 +24,16 @@ import React, { useReducer } from "react"; import { Button, Content, + EmptyState, + EmptyStateActions, + EmptyStateBody, + EmptyStateFooter, Toolbar, ToolbarContent, ToolbarGroup, ToolbarItem, } from "@patternfly/react-core"; +import Icon from "~/components/layout/Icon"; import FormatActionHandler from "~/components/storage/dasd/FormatActionHandler"; import FormatFilter from "~/components/storage/dasd/FormatFilter"; import SelectableDataTable from "~/components/core/SelectableDataTable"; @@ -247,6 +252,33 @@ const BulkActionsToolbar = ({ devices, updater, dispatcher }: DASDActionsBuilder ); }; +/** + * Empty state UI displayed when the DASD table has no items to show. + * + * This typically occurs when active filters exclude all devices. The component + * provides a clear message and an action to reset filters. + * + */ +const DASDTableEmptyState = ({ clearFilters }) => { + return ( + } + variant="sm" + > + {_("Change filters and try again.")} + + + + + + + ); +}; + /** * Encapsulates all state used by the DASD table component, including filters, * sorting configuration, current selection, and devices to be format. @@ -449,6 +481,9 @@ export default function DASDTable() { }) } itemActionsLabel={(d) => `Actions for ${d.id}`} + emptyState={ + dispatch({ type: "RESET_FILTERS" })} /> + } /> ); From b892a38c5d5f8b5faff2c189c83f17afe2d27a36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sun, 17 Aug 2025 16:03:01 +0100 Subject: [PATCH 21/31] feat(web): improve DASDTable empty state Distinguish between when there are no DASD devices in the system and when no devices match the applied filters. This provides more accurate feedback to the user in the empty state UI. --- .../storage/dasd/DASDTable.test.tsx | 29 ++++-- web/src/components/storage/dasd/DASDTable.tsx | 92 +++++++++++++------ 2 files changed, 88 insertions(+), 33 deletions(-) diff --git a/web/src/components/storage/dasd/DASDTable.test.tsx b/web/src/components/storage/dasd/DASDTable.test.tsx index 6ba67d4a6b..d1927dac9d 100644 --- a/web/src/components/storage/dasd/DASDTable.test.tsx +++ b/web/src/components/storage/dasd/DASDTable.test.tsx @@ -94,15 +94,30 @@ describe("DASDTable", () => { }); }); - describe("when there is some DASD devices available", () => { - beforeEach(() => { - mockDASDDevices = []; + describe("DASDTable/DASDTableEmptyState", () => { + describe("when there are no devices in the system", () => { + beforeEach(() => { + mockDASDDevices = []; + }); + + it("renders informative empty state with no actions", () => { + installerRender(); + screen.getByRole("heading", { name: "No devices available", level: 2 }); + screen.getByText("No DASD devices were found in this machine."); + }); }); - it("renders empty state when there are no devices", () => { - installerRender(); - screen.getByRole("heading", { name: "No devices found", level: 2 }); - screen.getByRole("button", { name: "Clear all filters" }); + describe("when filters results in no matching device", () => { + it.todo("renders empty state with clear all filters option"); + // it("renders empty state with clear all filters option", async () => { + // const { user } = installerRender(); + // const statusFilterToggle = screen.getByRole("button", { name: "Status" }); + // await user.click(statusFilterToggle); + // const readOnlyOption = screen.getByRole("option", { name: "read_only"}); + // await user.click(readOnlyOption); + // screen.getByRole("heading", { name: "No devices found", level: 2 }); + // screen.getByRole("button", { name: "Clear all filters" }); + // }); }); }); }); diff --git a/web/src/components/storage/dasd/DASDTable.tsx b/web/src/components/storage/dasd/DASDTable.tsx index ac4f5d298b..149403adbe 100644 --- a/web/src/components/storage/dasd/DASDTable.tsx +++ b/web/src/components/storage/dasd/DASDTable.tsx @@ -253,30 +253,65 @@ const BulkActionsToolbar = ({ devices, updater, dispatcher }: DASDActionsBuilder }; /** - * Empty state UI displayed when the DASD table has no items to show. - * - * This typically occurs when active filters exclude all devices. The component - * provides a clear message and an action to reset filters. + * Represents the mode of the empty state shown in the DASD table. * + * - "noDevices": No DASD devices are present on the system. + * - "noFilterResults": No matching results after appluing filters. */ -const DASDTableEmptyState = ({ clearFilters }) => { - return ( - } - variant="sm" - > - {_("Change filters and try again.")} - - - - - - - ); +type DASDEmptyStateMode = "noDevices" | "noFilterResults"; + +/** + * Props for the DASDTableEmptyState component. + */ +type DASDTableEmptyStateProps = { + /** + * Determines the type of empty state to display. + */ + mode: DASDEmptyStateMode; + /** + * Callback to reset filters when in "noFilterResults" mode. + */ + resetFilters: () => void; +}; + +/** + * Displays an appropriate empty state interface for the DASD table, + * depending on the mode. + */ +const DASDTableEmptyState = ({ mode, resetFilters }: DASDTableEmptyStateProps) => { + switch (mode) { + case "noDevices": { + return ( + } + variant="sm" + > + {_("No DASD devices were found in this machine.")} + + ); + } + case "noFilterResults": { + return ( + } + variant="sm" + > + {_("Change filters and try again.")} + + + + + + + ); + } + } }; /** @@ -419,7 +454,6 @@ const columns = [ export default function DASDTable() { const devices = useDASDDevices(); const { mutate: updateDASD } = useDASDMutation(); - const [state, dispatch] = useReducer(reducer, initialState); const onSortingChange = (sortedBy: SortedBy) => { @@ -435,6 +469,8 @@ export default function DASDTable() { dispatch({ type: "UPDATE_SELECTION", payload: devices }); }; + const resetFilters = () => dispatch({ type: "RESET_FILTERS" }); + // Filtering const filteredDevices = filterDevices(devices, state.filters); @@ -444,6 +480,12 @@ export default function DASDTable() { (d) => d[columns[state.sortedBy.index].sortingKey], ); + // Determine the appropriate empty state mode, if needed + let emptyMode: DASDEmptyStateMode; + if (isEmpty(filteredDevices)) { + emptyMode = state.filters === initialState.filters ? "noDevices" : "noFilterResults"; + } + return ( @@ -481,9 +523,7 @@ export default function DASDTable() { }) } itemActionsLabel={(d) => `Actions for ${d.id}`} - emptyState={ - dispatch({ type: "RESET_FILTERS" })} /> - } + emptyState={} /> ); From 4c6188c8ac6ba3245c488343c2fe96fc6ce1c7c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sun, 17 Aug 2025 16:05:46 +0100 Subject: [PATCH 22/31] fix(web): add misisng unit tests --- .../storage/dasd/FormatFilter.test.tsx | 53 ++++++++++++++++++ .../storage/dasd/StatusFilter.test.tsx | 54 +++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 web/src/components/storage/dasd/FormatFilter.test.tsx create mode 100644 web/src/components/storage/dasd/StatusFilter.test.tsx diff --git a/web/src/components/storage/dasd/FormatFilter.test.tsx b/web/src/components/storage/dasd/FormatFilter.test.tsx new file mode 100644 index 0000000000..77d5af37f5 --- /dev/null +++ b/web/src/components/storage/dasd/FormatFilter.test.tsx @@ -0,0 +1,53 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import FormatFilter from "./FormatFilter"; + +const onChangeFn = jest.fn(); + +describe("DASD/FormatFilter", () => { + it("renders a select menu to filter DASD devices by format status", async () => { + const { user } = plainRender(); + // Not using the label name to retrieve the MenuToggle button because a bug + // PF/MenuToggle has, check + // https://github.com/patternfly/patternfly-react/issues/11805 + const toggle = screen.getByRole("button"); + await user.click(toggle); + const options = screen.getByRole("listbox"); + within(options).getByRole("option", { name: "all" }); + within(options).getByRole("option", { name: "yes" }); + within(options).getByRole("option", { name: "no" }); + }); + + it("calls onChange when a format option is selected", async () => { + const { user } = plainRender(); + const toggle = screen.getByRole("button"); + await user.click(toggle); + const options = screen.getByRole("listbox"); + const activeOption = within(options).getByRole("option", { name: "yes" }); + await user.click(activeOption); + expect(onChangeFn).toHaveBeenCalledWith(expect.anything(), "yes"); + }); +}); diff --git a/web/src/components/storage/dasd/StatusFilter.test.tsx b/web/src/components/storage/dasd/StatusFilter.test.tsx new file mode 100644 index 0000000000..16205c6eec --- /dev/null +++ b/web/src/components/storage/dasd/StatusFilter.test.tsx @@ -0,0 +1,54 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import StatusFilter from "./StatusFilter"; + +const onChangeFn = jest.fn(); + +describe("DASD/StatusFilter", () => { + it("renders a select menu with available DASD status options", async () => { + const { user } = plainRender(); + // Not using the label name to retrieve the MenuToggle button because a bug + // PF/MenuToggle has, check + // https://github.com/patternfly/patternfly-react/issues/11805 + const toggle = screen.getByRole("button"); + await user.click(toggle); + const options = screen.getByRole("listbox"); + within(options).getByRole("option", { name: "all" }); + within(options).getByRole("option", { name: "active" }); + within(options).getByRole("option", { name: "read_only" }); + within(options).getByRole("option", { name: "offline" }); + }); + + it("calls onChange when a status option is selected", async () => { + const { user } = plainRender(); + const toggle = screen.getByRole("button"); + await user.click(toggle); + const options = screen.getByRole("listbox"); + const activeOption = within(options).getByRole("option", { name: "active" }); + await user.click(activeOption); + expect(onChangeFn).toHaveBeenCalledWith(expect.anything(), "active"); + }); +}); From 0837afe811ef1ba6536e74c3a39bc622b13e9e9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 18 Aug 2025 00:11:12 +0100 Subject: [PATCH 23/31] fix(web): show select all checkbox only when proceed But not if there are no items to show/select/unselect. --- .../components/core/SelectableDataTable.test.tsx | 7 ++++++- web/src/components/core/SelectableDataTable.tsx | 14 +++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/web/src/components/core/SelectableDataTable.test.tsx b/web/src/components/core/SelectableDataTable.test.tsx index 65d1613f4b..5ecb7e51b0 100644 --- a/web/src/components/core/SelectableDataTable.test.tsx +++ b/web/src/components/core/SelectableDataTable.test.tsx @@ -669,7 +669,7 @@ describe("SelectableDataTable", () => { beforeEach(() => onSelectionChange.mockClear()); - it("renders it only when selectionMode is multiple and allowSelectAll is true", () => { + it("renders it only when selectionMode is multiple and allowSelectAll is true and there are items to show", () => { const { rerender } = plainRender( , ); @@ -679,6 +679,11 @@ describe("SelectableDataTable", () => { rerender(); expect(screen.queryByRole("checkbox", { name: /select all/i })).toBeNull(); + rerender( + , + ); + expect(screen.queryByRole("checkbox", { name: /select all/i })).toBeNull(); + rerender(); screen.getByRole("checkbox", { name: "Select all rows" }); }); diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index 5b560d1f15..605c19b030 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -286,6 +286,11 @@ type SharedData = { */ readonly allowMultiple: boolean; + /** + * Whether the select all checkbox is rendered or not. + */ + readonly showSelectAll: boolean; + /** * Whether all items in the table are currently selected. * Used to reflect checkbox state in the header. @@ -349,10 +354,11 @@ const TableHeader = ({ columns: SelectableDataTableColumn[]; sharedData: SharedData; }) => { - const { allowMultiple, allowSelectAll, isAllSelected, selectAll, itemActions } = sharedData; + const { allowMultiple, allowSelectAll, isAllSelected, showSelectAll, selectAll, itemActions } = + sharedData; const selectAllProps = - allowMultiple && allowSelectAll + allowMultiple && allowSelectAll && showSelectAll ? { onSelect: (_event, isSelecting: boolean) => selectAll(isSelecting), isSelected: isAllSelected, @@ -593,7 +599,9 @@ export default function SelectableDataTable({ allowSelectAll, itemActions, itemActionsLabel, - isAllSelected: itemsSelected.length === items.length, + // FIXME: drop showSelectAll once items is part of SharedData + showSelectAll: allowSelectAll && items.length > 0, + isAllSelected: items.length > 0 && items.length === itemsSelected.length, selectAll: (isSelecting: boolean) => isSelecting ? onSelectionChange(items) : onSelectionChange([]), }; From d60ab79ec2ae5fc50840c8faad45310ab206fdd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 18 Aug 2025 08:44:28 +0100 Subject: [PATCH 24/31] fix(web): add missing aria-labels to empty headers --- web/src/components/core/SelectableDataTable.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index 605c19b030..9300ff2dda 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -39,6 +39,7 @@ import { } from "@patternfly/react-table"; import { isEmpty, isFunction } from "radashi"; import Icon from "~/components/layout/Icon"; +import { _ } from "~/i18n"; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -365,11 +366,12 @@ const TableHeader = ({ } : undefined; + // TODO: extract header ariaLabel to props instead of harconding them here. return ( - ); })} - {itemActions && ); From 6f1404bf17ae79b53015c9fd73d3a45ca2712b27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 18 Aug 2025 13:24:20 +0100 Subject: [PATCH 25/31] feat(dasd): show alert while DASD devices are being updated Tracks DASD devices undergoing async mutations using a new `waitingFor` state property. A modal is shown to indicate progress and disappears once all pending actions are complete. Listens for `DASDDeviceChanged` events to update the state accordingly. This should be considered a workaround to provide user feedback and prevent further actions while operations are in progress, until a more robust solution is implemented. --- .../storage/dasd/DASDTable.test.tsx | 51 +++++++++++++- web/src/components/storage/dasd/DASDTable.tsx | 67 ++++++++++++++++--- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/web/src/components/storage/dasd/DASDTable.test.tsx b/web/src/components/storage/dasd/DASDTable.test.tsx index d1927dac9d..8d38180762 100644 --- a/web/src/components/storage/dasd/DASDTable.test.tsx +++ b/web/src/components/storage/dasd/DASDTable.test.tsx @@ -20,17 +20,31 @@ * find current contact information at www.suse.com. */ -import React from "react"; +import React, { act } from "react"; import { screen } from "@testing-library/react"; import { installerRender } from "~/test-utils"; import { DASDDevice } from "~/types/dasd"; import DASDTable from "./DASDTable"; let mockDASDDevices: DASDDevice[] = []; +let eventCallback; +const mockClient = { + onEvent: jest.fn().mockImplementation((cb) => { + eventCallback = cb; + return () => {}; + }), +}; + +jest.mock("~/context/installer", () => ({ + ...jest.requireActual("~/context/installer"), + useInstallerClient: () => mockClient, +})); jest.mock("~/queries/storage/dasd", () => ({ useDASDDevices: () => mockDASDDevices, - useDASDMutation: () => jest.fn(), + useDASDMutation: () => ({ + mutate: jest.fn(), + }), useFormatDASDMutation: () => jest.fn(), })); @@ -92,6 +106,39 @@ describe("DASDTable", () => { await user.click(button); screen.getByText("FormatActionHandler Mock"); }); + + describe("when an action is requested", () => { + it("set component as busy", async () => { + const { user } = installerRender(); + const selection = screen.getByRole("checkbox", { name: "Select row 0" }); + await user.click(selection); + const button = screen.getByRole("button", { name: "Activate" }); + await user.click(button); + screen.getByRole("dialog", { name: "Applying changes" }); + expect(screen.queryByRole("checkbox", { name: "Select row 1" })).toBeNull(); + }); + }); + + describe("when all pending actions are done", () => { + it("set component as idle", async () => { + const { user } = installerRender(); + const selection = screen.getByRole("checkbox", { name: "Select row 0" }); + await user.click(selection); + const button = screen.getByRole("button", { name: "Activate" }); + await user.click(button); + screen.getByRole("dialog", { name: "Applying changes" }); + expect(screen.queryByRole("checkbox", { name: "Select row 0" })).toBeNull(); + + // Simulate a DASDDeviceChanged event + // + act(() => { + eventCallback({ type: "DASDDeviceChanged", device: mockDASDDevices[0] }); + }); + + expect(screen.queryByRole("dialog", { name: "Applying changes" })).toBeNull(); + screen.getByRole("checkbox", { name: "Select row 0" }); + }); + }); }); describe("DASDTable/DASDTableEmptyState", () => { diff --git a/web/src/components/storage/dasd/DASDTable.tsx b/web/src/components/storage/dasd/DASDTable.tsx index 149403adbe..361e3094c1 100644 --- a/web/src/components/storage/dasd/DASDTable.tsx +++ b/web/src/components/storage/dasd/DASDTable.tsx @@ -20,7 +20,7 @@ * find current contact information at www.suse.com. */ -import React, { useReducer } from "react"; +import React, { useEffect, useReducer } from "react"; import { Button, Content, @@ -34,6 +34,8 @@ import { ToolbarItem, } from "@patternfly/react-core"; import Icon from "~/components/layout/Icon"; +import Popup from "~/components/core/Popup"; +import Text from "~/components/core/Text"; import FormatActionHandler from "~/components/storage/dasd/FormatActionHandler"; import FormatFilter from "~/components/storage/dasd/FormatFilter"; import SelectableDataTable from "~/components/core/SelectableDataTable"; @@ -41,12 +43,18 @@ import StatusFilter from "~/components/storage/dasd/StatusFilter"; import TextinputFilter from "~/components/storage/dasd/TextinputFilter"; import { DASDDevice } from "~/types/dasd"; import type { SortedBy } from "~/components/core/SelectableDataTable"; -import { DASDMutationFn, useDASDDevices, useDASDMutation } from "~/queries/storage/dasd"; +import { + DASDMutationFn, + DASDMutationFnProps, + useDASDDevices, + useDASDMutation, +} from "~/queries/storage/dasd"; import { sort } from "fast-sort"; import { isEmpty } from "radashi"; import { hex } from "~/utils"; import { _, n_ } from "~/i18n"; import { sprintf } from "sprintf-js"; +import { useInstallerClient } from "~/context/installer"; /** * Filter options for narrowing down DASD devices shown in the table. @@ -319,10 +327,16 @@ const DASDTableEmptyState = ({ mode, resetFilters }: DASDTableEmptyStateProps) = * sorting configuration, current selection, and devices to be format. */ type DASDTableState = { + /** Current sorting state */ sortedBy: SortedBy; + /** Current active filters applied to the device list */ filters: DASDDevicesFilters; + /** Currently selected devices in the UI */ selectedDevices: DASDDevice[]; + /** Devices selected for formatting */ devicesToFormat: DASDDevice[]; + /** Device IDs currently undergoing an async operation */ + waitingFor: DASDDevice["id"][]; }; /** @@ -338,6 +352,7 @@ const initialState: DASDTableState = { }, selectedDevices: [], devicesToFormat: [], + waitingFor: [], }; /** @@ -350,7 +365,9 @@ type DASDTableAction = | { type: "UPDATE_SELECTION"; payload: DASDTableState["selectedDevices"] } | { type: "RESET_SELECTION" } | { type: "REQUEST_FORMAT"; payload: DASDTableState["devicesToFormat"] } - | { type: "CANCEL_FORMAT_REQUEST" }; + | { type: "CANCEL_FORMAT_REQUEST" } + | { type: "START_WAITING"; payload: DASDDevice["id"][] } + | { type: "UPDATE_WAITING"; payload: DASDDevice["id"] }; /** * Reducer function that handles all DASD table state transitions. @@ -384,6 +401,16 @@ const reducer = (state: DASDTableState, action: DASDTableAction): DASDTableState case "CANCEL_FORMAT_REQUEST": { return { ...state, devicesToFormat: [] }; } + + case "START_WAITING": { + return { ...state, waitingFor: action.payload }; + } + + case "UPDATE_WAITING": { + const prev = state.waitingFor; + const waitingFor = prev.filter((id) => action.payload !== id); + return { ...state, waitingFor }; + } } }; @@ -452,10 +479,21 @@ const columns = [ ]; export default function DASDTable() { + const client = useInstallerClient(); const devices = useDASDDevices(); const { mutate: updateDASD } = useDASDMutation(); const [state, dispatch] = useReducer(reducer, initialState); + useEffect(() => { + if (!client) return; + + return client.onEvent((event) => { + if (event.type === "DASDDeviceChanged") { + dispatch({ type: "UPDATE_WAITING", payload: event.device.id }); + } + }); + }, [client, dispatch]); + const onSortingChange = (sortedBy: SortedBy) => { dispatch({ type: "UPDATE_SORTING", payload: sortedBy }); }; @@ -485,15 +523,28 @@ export default function DASDTable() { if (isEmpty(filteredDevices)) { emptyMode = state.filters === initialState.filters ? "noDevices" : "noFilterResults"; } + /** + * Dispatches a DASD mutation and marks devices as waiting. + * + * @param mutation Parameters describing the DASD update operation + */ + const updater = (mutation: DASDMutationFnProps) => { + updateDASD(mutation); + dispatch({ type: "START_WAITING", payload: mutation.devices }); + }; return ( + {!isEmpty(state.waitingFor) && ( + + {_("Applying changes to the requested DASD devices.")} + + {_("This dialog will dissapear when the work is finished.")} + + + )} - + {!isEmpty(state.devicesToFormat) && ( Date: Mon, 18 Aug 2025 20:56:27 +0200 Subject: [PATCH 26/31] Allow hexadecimal numbers --- web/src/utils.test.ts | 3 ++- web/src/utils.ts | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/web/src/utils.test.ts b/web/src/utils.test.ts index a31009f095..571a66fd89 100644 --- a/web/src/utils.test.ts +++ b/web/src/utils.test.ts @@ -39,11 +39,12 @@ describe("compact", () => { describe("hex", () => { it("parses numeric dot strings as hex", () => { expect(hex("0.0.0160")).toBe(352); // "000160" + expect(hex("0.0.019d")).toBe(413); // "00019d" expect(hex("1.2.3")).toBe(291); // "123" expect(hex("123")).toBe(291); // "123" }); - it("returns 0 for strings with letters or invalid characters", () => { + it("returns 0 for strings with capital letters or invalid characters", () => { expect(hex("1A")).toBe(0); expect(hex("1A.3F")).toBe(0); expect(hex("xyz")).toBe(0); diff --git a/web/src/utils.ts b/web/src/utils.ts index d3cd3c292d..54498fa1b0 100644 --- a/web/src/utils.ts +++ b/web/src/utils.ts @@ -33,8 +33,8 @@ const compact = (collection: Array) => { /** * Parses a "numeric dot string" as a hexadecimal number. * - * Accepts only strings containing digits (`0–9`) and dots (`.`), - * for example: `"0.0.0160"` or `"123"`. Dots are removed before parsing. + * Accepts only strings containing hexadecimal numbers (`0–9a-fA-F`) and dots (`.`), + * for example: `"0.0.0160"` `"0.0.019d"` or `"123"`. Dots are removed before parsing. * * If the cleaned string contains any non-digit characters (such as letters), * or is not a valid integer string, the function returns `0`. @@ -43,8 +43,8 @@ const compact = (collection: Array) => { * * ```ts * hex("0.0.0.160"); // Returns 352 + * hex("0.0.0.19d"); // Returns 352 * hex("1.2.3"); // Returns 291 - * hex("1.A.3"); // Returns 0 (letters are not allowed) * hex(".."); // Returns 0 (empty string before removing dots) * ``` * @@ -53,7 +53,7 @@ const compact = (collection: Array) => { */ const hex = (value: string): number => { const sanitizedValued = value.replaceAll(".", ""); - return /^[0-9]+$/.test(sanitizedValued) ? parseInt(sanitizedValued, 16) : 0; + return /^[0-9a-fA-F]+$/.test(sanitizedValued) ? parseInt(sanitizedValued, 16) : 0; }; /** From e903aa431393dfa989bfee853c98997cfccb1ab7 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Mon, 18 Aug 2025 20:59:43 +0200 Subject: [PATCH 27/31] Fix test using hexadecimal valid strings --- web/src/utils.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/web/src/utils.test.ts b/web/src/utils.test.ts index 571a66fd89..6d3daeacc2 100644 --- a/web/src/utils.test.ts +++ b/web/src/utils.test.ts @@ -37,16 +37,14 @@ describe("compact", () => { }); describe("hex", () => { - it("parses numeric dot strings as hex", () => { + it("parses hexadecimal numeric dot strings as hex", () => { expect(hex("0.0.0160")).toBe(352); // "000160" expect(hex("0.0.019d")).toBe(413); // "00019d" expect(hex("1.2.3")).toBe(291); // "123" expect(hex("123")).toBe(291); // "123" }); - it("returns 0 for strings with capital letters or invalid characters", () => { - expect(hex("1A")).toBe(0); - expect(hex("1A.3F")).toBe(0); + it("returns 0 for strings invalid characters", () => { expect(hex("xyz")).toBe(0); expect(hex("123Z")).toBe(0); }); From e005610ed2c80afc15d0a6c53fd4a292024ea0a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 18 Aug 2025 14:11:08 +0100 Subject: [PATCH 28/31] doc(web): add entry to changes file --- web/package/agama-web-ui.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index e5b99f153d..2bbe841e2a 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Aug 18 13:07:07 UTC 2025 - David Diaz + +- Refactor DASD page to make it more usable and improve its + performance (related to bsc#1247444, gh#agama-project/agama#2648). + ------------------------------------------------------------------- Tue Aug 5 23:28:43 UTC 2025 - David Diaz From 4d6a4a733992194114473a80a88bec7193e4fc22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 18 Aug 2025 14:28:46 +0100 Subject: [PATCH 29/31] fix(web): reduce padding for sorting icon --- web/src/assets/styles/index.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/src/assets/styles/index.scss b/web/src/assets/styles/index.scss index 8826719701..f5e2ab6318 100644 --- a/web/src/assets/styles/index.scss +++ b/web/src/assets/styles/index.scss @@ -498,6 +498,10 @@ label.pf-m-disabled + .pf-v6-c-check__description { } } +.pf-v6-c-table__sort-indicator { + --pf-v6-c-table__sort-indicator--MarginInlineStart: var(--pf-t--global--spacer--xs); +} + // Some utilities not found at PF .w-14ch { inline-size: 14ch; From 1e0c19f7616f7642e7d7b17c107571c22e45224b Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 20 Aug 2025 17:18:20 +0200 Subject: [PATCH 30/31] Fix comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Knut Alejandro Anderssen González --- web/src/components/core/SelectableDataTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/core/SelectableDataTable.tsx b/web/src/components/core/SelectableDataTable.tsx index 9300ff2dda..39b32a6d00 100644 --- a/web/src/components/core/SelectableDataTable.tsx +++ b/web/src/components/core/SelectableDataTable.tsx @@ -434,7 +434,7 @@ const sanitizeSelection = ( * property (which defaults to `"id"`). If the item does not have that key, a * strict equality check (`===`) between the two items is performed. * - * Such a comparasion can be overriden by providing a custom `itemEqualityFn` + * Such comparison can be overridden by providing a custom `itemEqualityFn` * prop, for example, to perform deep comparison or other alternative logic. * * @note It only accepts one nesting level. From 4cdf73b1b7b14ac0bdf68859ef414442114e4cc0 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 20 Aug 2025 17:22:23 +0200 Subject: [PATCH 31/31] web: Adjust dialog wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: David Díaz <1691872+dgdavid@users.noreply.github.com> --- web/src/components/storage/dasd/DASDTable.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/web/src/components/storage/dasd/DASDTable.tsx b/web/src/components/storage/dasd/DASDTable.tsx index 361e3094c1..1698851c5a 100644 --- a/web/src/components/storage/dasd/DASDTable.tsx +++ b/web/src/components/storage/dasd/DASDTable.tsx @@ -35,7 +35,6 @@ import { } from "@patternfly/react-core"; import Icon from "~/components/layout/Icon"; import Popup from "~/components/core/Popup"; -import Text from "~/components/core/Text"; import FormatActionHandler from "~/components/storage/dasd/FormatActionHandler"; import FormatFilter from "~/components/storage/dasd/FormatFilter"; import SelectableDataTable from "~/components/core/SelectableDataTable"; @@ -537,9 +536,11 @@ export default function DASDTable() { {!isEmpty(state.waitingFor) && ( - {_("Applying changes to the requested DASD devices.")} + + {_("This may take a moment while updates complete.")} + - {_("This dialog will dissapear when the work is finished.")} + {_("This message will close automatically when everything is done.")} )}
- + + {columns?.map((c, i) => { const sortProp = sharedData.sortedBy && c.sortingKey ? buildSorting(i, c, sharedData) : undefined; @@ -380,7 +382,7 @@ const TableHeader = ({ } + {itemActions && }