diff --git a/packages/react-integration/cypress/integration/tableselectable.spec.ts b/packages/react-integration/cypress/integration/tableselectable.spec.ts index 65563d28f94..258d09d9f3c 100644 --- a/packages/react-integration/cypress/integration/tableselectable.spec.ts +++ b/packages/react-integration/cypress/integration/tableselectable.spec.ts @@ -17,11 +17,11 @@ describe('Table Selectable Test', () => { it('Test selectable checkbox', () => { for (let i = 1; i <= 3; i++) { - cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).check(); + cy.get(`tbody tr:nth-child(${i}) input[type="checkbox"]`).check(); } for (let i = 1; i <= 3; i++) { - cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).should('be.checked'); + cy.get(`tbody tr:nth-child(${i}) input[type="checkbox"]`).should('be.checked'); } }); @@ -30,14 +30,14 @@ describe('Table Selectable Test', () => { cy.get('input[name=selectVariant][value=radio]').click(); for (let i = 1; i <= 3; i++) { - cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).check(); + cy.get(`tbody tr:nth-child(${i}) input[type="radio"]`).check(); } // Only last radio input should be checked in the end of the iteration for (let i = 1; i <= 3; i++) { if (i < 3) { - cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).should('not.be.checked'); + cy.get(`tbody tr:nth-child(${i}) input[type="radio"]`).should('not.be.checked'); } else { - cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).should('be.checked'); + cy.get(`tbody tr:nth-child(${i}) input[type="radio"]`).should('be.checked'); } } }); 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..156dcbe09ec 100644 --- a/packages/react-table/src/components/Table/utils/transformers.test.tsx +++ b/packages/react-table/src/components/Table/utils/transformers.test.tsx @@ -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" > -