From 72ac0d14b9d33d0606b5ed5823d1e0af8638b2dc Mon Sep 17 00:00:00 2001 From: gitdallas <5322142+gitdallas@users.noreply.github.com> Date: Wed, 8 Oct 2025 15:08:58 -0500 Subject: [PATCH 1/3] feat(Table): use pf check/radio for selects Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com> --- .../src/components/Table/SelectColumn.tsx | 30 ++- .../__snapshots__/transformers.test.tsx.snap | 16 -- .../Table/utils/decorators/selectable.tsx | 11 +- .../Table/utils/transformers.test.tsx | 67 +++++-- .../__snapshots__/Table.test.tsx.snap | 189 ++++++++++++++---- 5 files changed, 236 insertions(+), 77 deletions(-) diff --git a/packages/react-table/src/components/Table/SelectColumn.tsx b/packages/react-table/src/components/Table/SelectColumn.tsx index 46472495423..18f20efef34 100644 --- a/packages/react-table/src/components/Table/SelectColumn.tsx +++ b/packages/react-table/src/components/Table/SelectColumn.tsx @@ -1,5 +1,7 @@ import { createRef, Fragment } from 'react'; import { Tooltip, TooltipProps } from '@patternfly/react-core/dist/esm/components/Tooltip'; +import { Checkbox } from '@patternfly/react-core/dist/esm/components/Checkbox'; +import { Radio } from '@patternfly/react-core/dist/esm/components/Radio'; export enum RowSelectVariant { radio = 'radio', @@ -7,7 +9,6 @@ export enum RowSelectVariant { } export interface SelectColumnProps { - name?: string; children?: React.ReactNode; className?: string; onSelect?: (event: React.FormEvent) => void; @@ -16,6 +17,10 @@ export interface SelectColumnProps { tooltip?: React.ReactNode; /** other props to pass to the tooltip */ tooltipProps?: Omit; + /** id for the input element - required by Checkbox and Radio components */ + id?: string; + /** name for the input element - required by Radio component */ + name?: string; } export const SelectColumn: React.FunctionComponent = ({ @@ -26,15 +31,30 @@ export const SelectColumn: React.FunctionComponent = ({ selectVariant, tooltip, tooltipProps, + id, + name, ...props }: SelectColumnProps) => { - const inputRef = createRef(); + const inputRef = createRef(); + + const handleChange = (event: React.FormEvent, _checked: boolean) => { + onSelect && onSelect(event); + }; + + const commonProps = { + ...props, + id, + ref: inputRef, + onChange: handleChange + }; const content = ( - + {selectVariant === RowSelectVariant.checkbox ? ( + + ) : ( + + )} {children} ); diff --git a/packages/react-table/src/components/Table/utils/__snapshots__/transformers.test.tsx.snap b/packages/react-table/src/components/Table/utils/__snapshots__/transformers.test.tsx.snap index 3b9ee591fa7..794dc9c70d5 100644 --- a/packages/react-table/src/components/Table/utils/__snapshots__/transformers.test.tsx.snap +++ b/packages/react-table/src/components/Table/utils/__snapshots__/transformers.test.tsx.snap @@ -1,21 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Transformer functions selectable unselected 1`] = ` -{ - "children": - - , - "className": "pf-v6-c-table__check", - "component": "td", - "isVisible": true, -} -`; - exports[`Transformer functions sortable asc 1`] = ` { "aria-sort": "ascending", diff --git a/packages/react-table/src/components/Table/utils/decorators/selectable.tsx b/packages/react-table/src/components/Table/utils/decorators/selectable.tsx index a536acd876e..1b4137316bb 100644 --- a/packages/react-table/src/components/Table/utils/decorators/selectable.tsx +++ b/packages/react-table/src/components/Table/utils/decorators/selectable.tsx @@ -30,26 +30,27 @@ export const selectable: ITransform = ( * @param {React.FormEvent} event - React form event */ function selectClick(event: React.FormEvent) { - const selected = rowIndex === undefined ? event.currentTarget.checked : rowData && !rowData.selected; + const selected = rowIndex === undefined ? event.currentTarget.checked : !(rowData && rowData.selected); // tslint:disable-next-line:no-unused-expression onSelect && onSelect(event, selected, rowId, rowData, extraData); } const customProps = { + id: rowId !== -1 ? `select-${rowIndex}` : 'select-all', ...(rowId !== -1 ? { - checked: rowData && !!rowData.selected, + isChecked: rowData && !!rowData.selected, 'aria-label': `Select row ${rowIndex}` } : { - checked: allRowsSelected, + isChecked: allRowsSelected, 'aria-label': 'Select all rows' }), ...(rowData && (rowData.disableCheckbox || rowData.disableSelection) && { - disabled: true, + isDisabled: true, className: checkStyles.checkInput }), - ...(!rowData && isHeaderSelectDisabled && { disabled: true }) + ...(!rowData && isHeaderSelectDisabled && { isDisabled: true }) }; let selectName = 'check-all'; if (rowId !== -1 && selectVariant === RowSelectVariant.checkbox) { diff --git a/packages/react-table/src/components/Table/utils/transformers.test.tsx b/packages/react-table/src/components/Table/utils/transformers.test.tsx index 1b63e92e37b..8e9ec21aed2 100644 --- a/packages/react-table/src/components/Table/utils/transformers.test.tsx +++ b/packages/react-table/src/components/Table/utils/transformers.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor } from '@testing-library/react'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { @@ -83,7 +83,7 @@ const testCellActions = async ({ describe('Transformer functions', () => { describe('selectable', () => { - test('main select', async () => { + test('main select (header) - should toggle from unchecked to checked', async () => { const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId })); const column = { extraParams: { onSelect } @@ -92,37 +92,80 @@ describe('Transformer functions', () => { expect(returnedData).toMatchObject({ className: tableStyles.tableCheck }); const user = userEvent.setup(); - render(returnedData.children as React.ReactElement); - await user.type(screen.getByRole('textbox'), 'a'); + // Click the header radio button (should be unchecked initially) + await user.click(screen.getByRole('radio')); + expect(onSelect).toHaveBeenCalledTimes(1); - expect(onSelect.mock.results[0].value).toMatchObject({ rowId: -1, selected: false }); + expect(onSelect.mock.results[0].value).toMatchObject({ rowId: -1, selected: true }); }); - test('selected', async () => { + test('row select (checkbox) - should toggle from selected to unselected', async () => { const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId })); const column = { - extraParams: { onSelect } + extraParams: { onSelect, selectVariant: 'checkbox' } }; const returnedData = selectable('', { column, rowIndex: 0, rowData: { selected: true } } as IExtra); expect(returnedData).toMatchObject({ className: tableStyles.tableCheck }); - const user = userEvent.setup(); + const user = userEvent.setup(); render(returnedData.children as React.ReactElement); - await user.type(screen.getByRole('textbox'), 'a'); + // Click the row checkbox (should be checked initially, clicking should uncheck) + await user.click(screen.getByRole('checkbox')); expect(onSelect).toHaveBeenCalledTimes(1); expect(onSelect.mock.results[0].value).toMatchObject({ rowId: 0, selected: false }); }); - test('unselected', () => { + test('row select (checkbox) - should toggle from unselected to selected', async () => { const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId })); const column = { - extraParams: { onSelect } + extraParams: { onSelect, selectVariant: 'checkbox' } }; const returnedData = selectable('', { column, rowIndex: 0, rowData: { selected: false } } as IExtra); - expect(returnedData).toMatchSnapshot(); + expect(returnedData).toMatchObject({ className: tableStyles.tableCheck }); + + const user = userEvent.setup(); + render(returnedData.children as React.ReactElement); + + // Click the row checkbox (should be unchecked initially, clicking should check) + await user.click(screen.getByRole('checkbox')); + expect(onSelect).toHaveBeenCalledTimes(1); + expect(onSelect.mock.results[0].value).toMatchObject({ rowId: 0, selected: true }); + }); + + test('row select (radio) - clicking already selected radio should not trigger onSelect', async () => { + const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId })); + const column = { + extraParams: { onSelect, selectVariant: 'radio' } + }; + const returnedData = selectable('', { column, rowIndex: 0, rowData: { selected: true } } as IExtra); + expect(returnedData).toMatchObject({ className: tableStyles.tableCheck }); + + const user = userEvent.setup(); + render(returnedData.children as React.ReactElement); + + // Click the row radio button (should be checked initially, clicking should not trigger change) + await user.click(screen.getByRole('radio')); + expect(onSelect).toHaveBeenCalledTimes(0); + }); + + test('row select (radio) - should toggle from unselected to selected', async () => { + const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId })); + const column = { + extraParams: { onSelect, selectVariant: 'radio' } + }; + const returnedData = selectable('', { column, rowIndex: 0, rowData: { selected: false } } as IExtra); + expect(returnedData).toMatchObject({ className: tableStyles.tableCheck }); + + const user = userEvent.setup(); + render(returnedData.children as React.ReactElement); + + // Click the row radio button (should be unchecked initially, clicking should check) + await user.click(screen.getByRole('radio')); + expect(onSelect).toHaveBeenCalledTimes(1); + expect(onSelect.mock.results[0].value).toMatchObject({ rowId: 0, selected: true }); }); }); diff --git a/packages/react-table/src/deprecated/components/Table/__tests__/__snapshots__/Table.test.tsx.snap b/packages/react-table/src/deprecated/components/Table/__tests__/__snapshots__/Table.test.tsx.snap index 96420adbaac..6a7a59bfaf2 100644 --- a/packages/react-table/src/deprecated/components/Table/__tests__/__snapshots__/Table.test.tsx.snap +++ b/packages/react-table/src/deprecated/components/Table/__tests__/__snapshots__/Table.test.tsx.snap @@ -4216,13 +4216,20 @@ exports[`Table Selectable table 1`] = ` scope="" tabindex="-1" > -