Skip to content

feat(Table): use pf check/radio for selects #12045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 24, 2025
Merged

feat(Table): use pf check/radio for selects #12045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72ac0d1
feat(Table): use pf check/radio for selects
gitdallas Oct 8, 2025
5d506bf
fix cypress
gitdallas Oct 8, 2025
0a9d7a6
remove unused import
gitdallas Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/react-integration/cypress/integration/tableselectable.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
});

Expand All @@ -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');
}
}
});
Expand Down
30 changes: 25 additions & 5 deletions packages/react-table/src/components/Table/SelectColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
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',
checkbox = 'checkbox'
}

export interface SelectColumnProps {
name?: string;
children?: React.ReactNode;
className?: string;
onSelect?: (event: React.FormEvent<HTMLInputElement>) => void;
Expand All @@ -16,6 +17,10 @@ export interface SelectColumnProps {
tooltip?: React.ReactNode;
/** other props to pass to the tooltip */
tooltipProps?: Omit<TooltipProps, 'content'>;
/** id for the input element - required by Checkbox and Radio components */
id?: string;
/** name for the input element - required by Radio component */
name?: string;
Comment on lines +20 to +23
Copy link
Contributor

@wise-king-sullyman wise-king-sullyman Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's something extra we could/should do with the types here to make these required depending on the passed selectVariant 🤔

Though that might be really hard to do without a breaking change, so this isn't a blocker.

Copy link
Contributor Author

@gitdallas gitdallas Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈

}

export const SelectColumn: React.FunctionComponent<SelectColumnProps> = ({
Expand All @@ -26,15 +31,30 @@ export const SelectColumn: React.FunctionComponent<SelectColumnProps> = ({
selectVariant,
tooltip,
tooltipProps,
id,
name,
...props
}: SelectColumnProps) => {
const inputRef = createRef<HTMLInputElement>();
const inputRef = createRef<any>();

const handleChange = (event: React.FormEvent<HTMLInputElement>, _checked: boolean) => {
onSelect && onSelect(event);
};

const commonProps = {
...props,
id,
ref: inputRef,
onChange: handleChange
};

const content = (
<Fragment>
<label>
<input {...props} ref={inputRef} type={selectVariant} onChange={onSelect} />
</label>
{selectVariant === RowSelectVariant.checkbox ? (
<Checkbox {...commonProps} />
) : (
<Radio {...commonProps} name={name} />
)}
{children}
</Fragment>
);
Expand Down
16 changes: 0 additions & 16 deletions packages/react-table/src/components/Table/utils/__snapshots__/transformers.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Transformer functions selectable unselected 1`] = `
{
"children": <SelectColumn
aria-label="Select row 0"
checked={false}
name="radioGroup"
onSelect={[Function]}
>

</SelectColumn>,
"className": "pf-v6-c-table__check",
"component": "td",
"isVisible": true,
}
`;

exports[`Transformer functions sortable asc 1`] = `
{
"aria-sort": "ascending",
Expand Down
11 changes: 6 additions & 5 deletions packages/react-table/src/components/Table/utils/decorators/selectable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,27 @@ export const selectable: ITransform = (
* @param {React.FormEvent} event - React form event
*/
function selectClick(event: React.FormEvent<HTMLInputElement>) {
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) {
Expand Down
65 changes: 54 additions & 11 deletions packages/react-table/src/components/Table/utils/transformers.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -92,37 +92,80 @@ describe('Transformer functions', () => {
expect(returnedData).toMatchObject({ className: tableStyles.tableCheck });

const user = userEvent.setup();

render(returnedData.children as React.ReactElement<any>);

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<any>);

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<any>);

// 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<any>);

// 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<any>);

// 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 });
});
});

Expand Down
Loading
Loading