Skip to content

Commit a434558

Browse files
gitdallasgitdallas
authored
feat(Table): use pf check/radio for selects (#12045)
Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>
1 parent 0b97915 commit a434558

6 files changed

Lines changed: 240 additions & 81 deletions

File tree

‎packages/react-integration/cypress/integration/tableselectable.spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ describe('Table Selectable Test', () => {
1717

1818
it('Test selectable checkbox', () => {
1919
for (let i = 1; i <= 3; i++) {
20-
cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).check();
20+
cy.get(`tbody tr:nth-child(${i}) input[type="checkbox"]`).check();
2121
}
2222

2323
for (let i = 1; i <= 3; i++) {
24-
cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).should('be.checked');
24+
cy.get(`tbody tr:nth-child(${i}) input[type="checkbox"]`).should('be.checked');
2525
}
2626
});
2727

@@ -30,14 +30,14 @@ describe('Table Selectable Test', () => {
3030
cy.get('input[name=selectVariant][value=radio]').click();
3131

3232
for (let i = 1; i <= 3; i++) {
33-
cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).check();
33+
cy.get(`tbody tr:nth-child(${i}) input[type="radio"]`).check();
3434
}
3535
// Only last radio input should be checked in the end of the iteration
3636
for (let i = 1; i <= 3; i++) {
3737
if (i < 3) {
38-
cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).should('not.be.checked');
38+
cy.get(`tbody tr:nth-child(${i}) input[type="radio"]`).should('not.be.checked');
3939
} else {
40-
cy.get(`tbody tr:nth-child(${i}) .pf-v6-c-table__check > label > input`).should('be.checked');
40+
cy.get(`tbody tr:nth-child(${i}) input[type="radio"]`).should('be.checked');
4141
}
4242
}
4343
});

‎packages/react-table/src/components/Table/SelectColumn.tsx‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import { createRef, Fragment } from 'react';
22
import { Tooltip, TooltipProps } from '@patternfly/react-core/dist/esm/components/Tooltip';
3+
import { Checkbox } from '@patternfly/react-core/dist/esm/components/Checkbox';
4+
import { Radio } from '@patternfly/react-core/dist/esm/components/Radio';
35

46
export enum RowSelectVariant {
57
radio = 'radio',
68
checkbox = 'checkbox'
79
}
810

911
export interface SelectColumnProps {
10-
name?: string;
1112
children?: React.ReactNode;
1213
className?: string;
1314
onSelect?: (event: React.FormEvent<HTMLInputElement>) => void;
@@ -16,6 +17,10 @@ export interface SelectColumnProps {
1617
tooltip?: React.ReactNode;
1718
/** other props to pass to the tooltip */
1819
tooltipProps?: Omit<TooltipProps, 'content'>;
20+
/** id for the input element - required by Checkbox and Radio components */
21+
id?: string;
22+
/** name for the input element - required by Radio component */
23+
name?: string;
1924
}
2025

2126
export const SelectColumn: React.FunctionComponent<SelectColumnProps> = ({
@@ -26,15 +31,30 @@ export const SelectColumn: React.FunctionComponent<SelectColumnProps> = ({
2631
selectVariant,
2732
tooltip,
2833
tooltipProps,
34+
id,
35+
name,
2936
...props
3037
}: SelectColumnProps) => {
31-
const inputRef = createRef<HTMLInputElement>();
38+
const inputRef = createRef<any>();
39+
40+
const handleChange = (event: React.FormEvent<HTMLInputElement>, _checked: boolean) => {
41+
onSelect && onSelect(event);
42+
};
43+
44+
const commonProps = {
45+
...props,
46+
id,
47+
ref: inputRef,
48+
onChange: handleChange
49+
};
3250

3351
const content = (
3452
<Fragment>
35-
<label>
36-
<input {...props} ref={inputRef} type={selectVariant} onChange={onSelect} />
37-
</label>
53+
{selectVariant === RowSelectVariant.checkbox ? (
54+
<Checkbox {...commonProps} />
55+
) : (
56+
<Radio {...commonProps} name={name} />
57+
)}
3858
{children}
3959
</Fragment>
4060
);

‎packages/react-table/src/components/Table/utils/__snapshots__/transformers.test.tsx.snap‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,5 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`Transformer functions selectable unselected 1`] = `
4-
{
5-
"children": <SelectColumn
6-
aria-label="Select row 0"
7-
checked={false}
8-
name="radioGroup"
9-
onSelect={[Function]}
10-
>
11-
12-
</SelectColumn>,
13-
"className": "pf-v6-c-table__check",
14-
"component": "td",
15-
"isVisible": true,
16-
}
17-
`;
18-
193
exports[`Transformer functions sortable asc 1`] = `
204
{
215
"aria-sort": "ascending",

‎packages/react-table/src/components/Table/utils/decorators/selectable.tsx‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,26 +30,27 @@ export const selectable: ITransform = (
3030
* @param {React.FormEvent} event - React form event
3131
*/
3232
function selectClick(event: React.FormEvent<HTMLInputElement>) {
33-
const selected = rowIndex === undefined ? event.currentTarget.checked : rowData && !rowData.selected;
33+
const selected = rowIndex === undefined ? event.currentTarget.checked : !(rowData && rowData.selected);
3434
// tslint:disable-next-line:no-unused-expression
3535
onSelect && onSelect(event, selected, rowId, rowData, extraData);
3636
}
3737
const customProps = {
38+
id: rowId !== -1 ? `select-${rowIndex}` : 'select-all',
3839
...(rowId !== -1
3940
? {
40-
checked: rowData && !!rowData.selected,
41+
isChecked: rowData && !!rowData.selected,
4142
'aria-label': `Select row ${rowIndex}`
4243
}
4344
: {
44-
checked: allRowsSelected,
45+
isChecked: allRowsSelected,
4546
'aria-label': 'Select all rows'
4647
}),
4748
...(rowData &&
4849
(rowData.disableCheckbox || rowData.disableSelection) && {
49-
disabled: true,
50+
isDisabled: true,
5051
className: checkStyles.checkInput
5152
}),
52-
...(!rowData && isHeaderSelectDisabled && { disabled: true })
53+
...(!rowData && isHeaderSelectDisabled && { isDisabled: true })
5354
};
5455
let selectName = 'check-all';
5556
if (rowId !== -1 && selectVariant === RowSelectVariant.checkbox) {

‎packages/react-table/src/components/Table/utils/transformers.test.tsx‎

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ const testCellActions = async ({
8383

8484
describe('Transformer functions', () => {
8585
describe('selectable', () => {
86-
test('main select', async () => {
86+
test('main select (header) - should toggle from unchecked to checked', async () => {
8787
const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId }));
8888
const column = {
8989
extraParams: { onSelect }
@@ -92,37 +92,80 @@ describe('Transformer functions', () => {
9292
expect(returnedData).toMatchObject({ className: tableStyles.tableCheck });
9393

9494
const user = userEvent.setup();
95-
9695
render(returnedData.children as React.ReactElement<any>);
9796

98-
await user.type(screen.getByRole('textbox'), 'a');
97+
// Click the header radio button (should be unchecked initially)
98+
await user.click(screen.getByRole('radio'));
99+
99100
expect(onSelect).toHaveBeenCalledTimes(1);
100-
expect(onSelect.mock.results[0].value).toMatchObject({ rowId: -1, selected: false });
101+
expect(onSelect.mock.results[0].value).toMatchObject({ rowId: -1, selected: true });
101102
});
102103

103-
test('selected', async () => {
104+
test('row select (checkbox) - should toggle from selected to unselected', async () => {
104105
const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId }));
105106
const column = {
106-
extraParams: { onSelect }
107+
extraParams: { onSelect, selectVariant: 'checkbox' }
107108
};
108109
const returnedData = selectable('', { column, rowIndex: 0, rowData: { selected: true } } as IExtra);
109110
expect(returnedData).toMatchObject({ className: tableStyles.tableCheck });
110-
const user = userEvent.setup();
111111

112+
const user = userEvent.setup();
112113
render(returnedData.children as React.ReactElement<any>);
113114

114-
await user.type(screen.getByRole('textbox'), 'a');
115+
// Click the row checkbox (should be checked initially, clicking should uncheck)
116+
await user.click(screen.getByRole('checkbox'));
115117
expect(onSelect).toHaveBeenCalledTimes(1);
116118
expect(onSelect.mock.results[0].value).toMatchObject({ rowId: 0, selected: false });
117119
});
118120

119-
test('unselected', () => {
121+
test('row select (checkbox) - should toggle from unselected to selected', async () => {
120122
const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId }));
121123
const column = {
122-
extraParams: { onSelect }
124+
extraParams: { onSelect, selectVariant: 'checkbox' }
123125
};
124126
const returnedData = selectable('', { column, rowIndex: 0, rowData: { selected: false } } as IExtra);
125-
expect(returnedData).toMatchSnapshot();
127+
expect(returnedData).toMatchObject({ className: tableStyles.tableCheck });
128+
129+
const user = userEvent.setup();
130+
render(returnedData.children as React.ReactElement<any>);
131+
132+
// Click the row checkbox (should be unchecked initially, clicking should check)
133+
await user.click(screen.getByRole('checkbox'));
134+
expect(onSelect).toHaveBeenCalledTimes(1);
135+
expect(onSelect.mock.results[0].value).toMatchObject({ rowId: 0, selected: true });
136+
});
137+
138+
test('row select (radio) - clicking already selected radio should not trigger onSelect', async () => {
139+
const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId }));
140+
const column = {
141+
extraParams: { onSelect, selectVariant: 'radio' }
142+
};
143+
const returnedData = selectable('', { column, rowIndex: 0, rowData: { selected: true } } as IExtra);
144+
expect(returnedData).toMatchObject({ className: tableStyles.tableCheck });
145+
146+
const user = userEvent.setup();
147+
render(returnedData.children as React.ReactElement<any>);
148+
149+
// Click the row radio button (should be checked initially, clicking should not trigger change)
150+
await user.click(screen.getByRole('radio'));
151+
expect(onSelect).toHaveBeenCalledTimes(0);
152+
});
153+
154+
test('row select (radio) - should toggle from unselected to selected', async () => {
155+
const onSelect = jest.fn((_event, selected, rowId) => ({ selected, rowId }));
156+
const column = {
157+
extraParams: { onSelect, selectVariant: 'radio' }
158+
};
159+
const returnedData = selectable('', { column, rowIndex: 0, rowData: { selected: false } } as IExtra);
160+
expect(returnedData).toMatchObject({ className: tableStyles.tableCheck });
161+
162+
const user = userEvent.setup();
163+
render(returnedData.children as React.ReactElement<any>);
164+
165+
// Click the row radio button (should be unchecked initially, clicking should check)
166+
await user.click(screen.getByRole('radio'));
167+
expect(onSelect).toHaveBeenCalledTimes(1);
168+
expect(onSelect.mock.results[0].value).toMatchObject({ rowId: 0, selected: true });
126169
});
127170
});
128171

0 commit comments

Comments
 (0)