Skip to content

Commit ff9abac

Browse files
rhamiltoclaude
andcommitted
CONSOLE-5091: Upgrade to PatternFly v6.5 with native indeterminate checkbox support
Upgrade to @patternfly/react-table@6.5.0-prerelease.77 which includes native indeterminate checkbox support via PR patternfly/patternfly-react#12411. Replace custom DOM manipulation hook with inline useEffect that sets checkbox indeterminate state directly. This avoids React controlled/uncontrolled input warnings that occur when passing isIndeterminate as a prop. Changes: - Upgrade PatternFly packages to v6.5 prerelease with peer dependency resolutions - Delete useIndeterminateCheckbox custom hook - Add inline useEffect in ConsoleDataView for indeterminate state via DOM manipulation - Fix icon-utils to handle both IconDefinition and IconData formats from PF v6.5 - Add Boolean coercion for checkbox isSelected prop to prevent controlled warnings - Wrap getDataViewRows in useCallback for performance - Remove @ts-expect-error comments now that NotificationBadge types variant prop - Document known reselect dev warning from legacy connect() HOC in kinds.ts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f847537 commit ff9abac

12 files changed

Lines changed: 114 additions & 222 deletions

File tree

frontend/package.json

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@
176176
"@patternfly/react-icons": "~6.4.0",
177177
"@patternfly/react-log-viewer": "~6.3.0",
178178
"@patternfly/react-styles": "~6.4.0",
179-
"@patternfly/react-table": "~6.4.2",
179+
"@patternfly/react-table": "6.5.0-prerelease.77",
180180
"@patternfly/react-templates": "~6.4.2",
181181
"@patternfly/react-tokens": "~6.4.0",
182182
"@patternfly/react-topology": "~6.4.0",
@@ -335,6 +335,11 @@
335335
"hosted-git-info": "^3.0.8",
336336
"lodash-es": "^4.17.23",
337337
"@patternfly/react-component-groups": "6.4.0-prerelease.17",
338+
"@patternfly/react-core": "6.5.0-prerelease.73",
339+
"@patternfly/react-icons": "6.5.0-prerelease.34",
340+
"@patternfly/react-styles": "6.5.0-prerelease.24",
341+
"@patternfly/react-table": "6.5.0-prerelease.77",
342+
"@patternfly/react-tokens": "6.5.0-prerelease.23",
338343
"postcss": "^8.2.13"
339344
},
340345
"lint-staged": {

frontend/packages/console-app/src/components/data-view/BULK_SELECTION_GUIDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -621,9 +621,9 @@ The `useDataViewSelection` hook automatically removes selections for items that
621621

622622
## Related Documentation
623623

624-
- [INDETERMINATE_CHECKBOX.md](./INDETERMINATE_CHECKBOX.md) - Details on the indeterminate checkbox pattern
625624
- [ConsoleDataView](./ConsoleDataView.tsx) - Main data view component
626625
- [dataViewSelectionHelpers.ts](./dataViewSelectionHelpers.ts) - Selection helper functions
626+
- [useDataViewSelection.ts](./useDataViewSelection.ts) - Selection state management hook
627627

628628
## Real-World Examples
629629

frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import { DataViewLabelFilter } from './DataViewLabelFilter';
3737
import { DataViewTextFilter } from './DataViewTextFilter';
3838
import { useConsoleDataViewData } from './useConsoleDataViewData';
3939
import { useConsoleDataViewFilters } from './useConsoleDataViewFilters';
40-
import { useIndeterminateCheckbox } from './useIndeterminateCheckbox';
4140

4241
export const initialFiltersDefault: ResourceFilters = { name: '', label: '' };
4342

@@ -208,11 +207,6 @@ export const ConsoleDataView = <
208207
return { show: shouldShow, allSelected, isIndeterminate };
209208
}, [selection, loaded, filteredData, visibleItems]);
210209

211-
// Set indeterminate state on the select-all checkbox via DOM manipulation
212-
// This is a workaround until PatternFly adds native support for isSelected: null
213-
// See: https://github.com/patternfly/patternfly-react/issues/12404
214-
useIndeterminateCheckbox(bannerState.isIndeterminate);
215-
216210
const handleSelectAllMatching = useCallback(() => {
217211
if (selection?.onSelectAll) {
218212
selection.onSelectAll(true, filteredData);
@@ -225,6 +219,19 @@ export const ConsoleDataView = <
225219
}
226220
}, [selection, filteredData]);
227221

222+
// Set indeterminate state via DOM manipulation since PatternFly's controlled prop
223+
// causes React controlled/uncontrolled warnings when toggling
224+
useEffect(() => {
225+
if (selection && loaded && filteredData.length > 0) {
226+
const checkbox = document.querySelector(
227+
'[data-label=""] input[type="checkbox"]',
228+
) as HTMLInputElement;
229+
if (checkbox) {
230+
checkbox.indeterminate = bannerState.isIndeterminate;
231+
}
232+
}
233+
}, [selection, loaded, filteredData.length, bannerState.isIndeterminate]);
234+
228235
const dataViewFilterNodes = useMemo<React.ReactNode[]>(() => {
229236
const basicFilters: ReactNode[] = [];
230237

frontend/packages/console-app/src/components/data-view/INDETERMINATE_CHECKBOX.md

Lines changed: 0 additions & 95 deletions
This file was deleted.

frontend/packages/console-app/src/components/data-view/dataViewSelectionHelpers.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ export const createSelectionCell = ({
7373
onSelect: (_event: any, isSelecting: boolean) => {
7474
onSelect(itemId, isSelecting);
7575
},
76-
isSelected,
76+
// Ensure isSelected is always a boolean to prevent controlled/uncontrolled warnings
77+
isSelected: Boolean(isSelected),
7778
isDisabled: disabled,
7879
},
7980
},

frontend/packages/console-app/src/components/data-view/useConsoleDataViewData.tsx

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,8 @@ export const useConsoleDataViewData = <
9494
});
9595

9696
const dataViewColumns = useMemo<ConsoleDataViewColumn<TData>[]>(() => {
97-
// Calculate selection state across all filtered items (for indeterminate state)
98-
const selectedCount = selection
99-
? filteredData.filter((item) => selection.selectedItems.has(selection.getItemId(item))).length
100-
: 0;
97+
// Calculate selection state across all filtered items
10198
const totalCount = filteredData.length;
102-
const someSelected = selectedCount > 0 && selectedCount < totalCount;
10399

104100
return activeColumns.map(({ id, title, sort, props, resizableProps }, index) => {
105101
// Filter out custom Console props that aren't valid PatternFly ThProps
@@ -133,11 +129,8 @@ export const useConsoleDataViewData = <
133129
},
134130
isSelected: false, // Will be updated based on visible items
135131
isDisabled: totalCount === 0,
136-
// Pass indeterminate state through props (custom extension until PF supports it)
137-
// See: https://github.com/patternfly/patternfly-react/issues/12404
138-
props: {
139-
isIndeterminate: someSelected,
140-
},
132+
// NOTE: isIndeterminate is set via DOM manipulation in ConsoleDataView to avoid
133+
// React controlled/uncontrolled warnings when the prop value changes
141134
};
142135
}
143136

@@ -230,14 +223,11 @@ export const useConsoleDataViewData = <
230223
},
231224
select: {
232225
...column.props.select,
233-
// Checkbox is checked only when ALL visible items are selected
234-
// Indeterminate state is handled via DOM manipulation in ConsoleDataView
235-
isSelected: allVisibleSelected,
236226
onSelect: (_event: any, isSelecting: boolean) => {
237-
// When unchecked or indeterminate, clicking selects all visible items
238-
// When checked, clicking deselects all visible items
239227
selection.onSelectAll(isSelecting, visibleItems);
240228
},
229+
isSelected: Boolean(allVisibleSelected),
230+
// NOTE: isIndeterminate is set via DOM manipulation in ConsoleDataView
241231
},
242232
},
243233
};
@@ -273,14 +263,11 @@ export const useConsoleDataViewData = <
273263
...column.props,
274264
select: {
275265
...column.props.select,
276-
// Checkbox is checked only when ALL visible items are selected
277-
// Indeterminate state is handled via DOM manipulation in ConsoleDataView
278-
isSelected: allVisibleSelected,
279266
onSelect: (_event: any, isSelecting: boolean) => {
280-
// When unchecked or indeterminate, clicking selects all visible items
281-
// When checked, clicking deselects all visible items
282267
selection.onSelectAll(isSelecting, visibleItems);
283268
},
269+
isSelected: Boolean(allVisibleSelected),
270+
// NOTE: isIndeterminate is set via DOM manipulation in ConsoleDataView
284271
},
285272
},
286273
};

frontend/packages/console-app/src/components/data-view/useIndeterminateCheckbox.ts

Lines changed: 0 additions & 35 deletions
This file was deleted.

frontend/packages/console-app/src/components/nodes/NodesPage.tsx

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -665,9 +665,7 @@ const NodeList: FC<NodeListProps> = ({
665665
}) => {
666666
const { t } = useTranslation();
667667
const { columns, resetAllColumnWidths } = useNodesColumns(vmsEnabled, nodeMgmtV1Enabled);
668-
const nodeMetrics = useConsoleSelector<NodeMetrics>(({ UI }) => {
669-
return UI.getIn(['metrics', 'node']);
670-
});
668+
const nodeMetrics = useConsoleSelector<NodeMetrics>(({ UI }) => UI.getIn(['metrics', 'node']));
671669
const columnManagementID = referenceForModel(NodeModel);
672670
const statusExtensions = useNodeStatusExtensions();
673671

@@ -692,6 +690,21 @@ const NodeList: FC<NodeListProps> = ({
692690
onComplete: clearSelection,
693691
});
694692

693+
const getDataViewRows = useCallback(
694+
(rowData: any, tableColumns: any) =>
695+
getNodeDataViewRows(
696+
(rowData as unknown) as RowProps<NodeRowItem, GetNodeStatusExtensions>[],
697+
tableColumns,
698+
nodeMetrics,
699+
statusExtensions,
700+
{
701+
selectedItems: selectedIds,
702+
onSelect: onSelectItem,
703+
},
704+
),
705+
[nodeMetrics, statusExtensions, selectedIds, onSelectItem],
706+
);
707+
695708
const columnLayout = useMemo(
696709
() => ({
697710
id: columnManagementID,
@@ -904,18 +917,7 @@ const NodeList: FC<NodeListProps> = ({
904917
initialFilters={initialFilters}
905918
additionalFilterNodes={additionalFilterNodes}
906919
matchesAdditionalFilters={matchesAdditionalFilters}
907-
getDataViewRows={(rowData, tableColumns) =>
908-
getNodeDataViewRows(
909-
(rowData as unknown) as RowProps<NodeRowItem, GetNodeStatusExtensions>[],
910-
tableColumns,
911-
nodeMetrics,
912-
statusExtensions,
913-
{
914-
selectedItems: selectedIds,
915-
onSelect: onSelectItem,
916-
},
917-
)
918-
}
920+
getDataViewRows={getDataViewRows}
919921
hideNameLabelFilters={hideNameLabelFilters}
920922
hideLabelFilter={hideLabelFilter}
921923
hideColumnManagement={hideColumnManagement}
@@ -1035,7 +1037,7 @@ export const NodesPage: FC<NodesPageProps> = ({ selector }) => {
10351037
filterVirtualMachineInstancesByNode(vmis, node.metadata.name),
10361038
]),
10371039
);
1038-
}, [isKubevirtPluginActive, nodes, nodesLoadError, nodesLoaded, vmis, vmisLoadError, vmisLoaded]);
1040+
}, [isKubevirtPluginActive, nodes, nodesLoaded, nodesLoadError, vmis, vmisLoaded, vmisLoadError]);
10391041

10401042
useEffect(() => {
10411043
const updateMetrics = async () => {

frontend/packages/console-shared/src/utils/icon-utils.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { IconDefinition } from '@patternfly/react-icons/dist/esm/createIcon';
1+
import type { IconDefinition, IconData } from '@patternfly/react-icons/dist/esm/createIcon';
22

33
const ICON_OPERATOR = 'icon-operator';
44
export type CSVIcon = { base64data: string; mediatype: string };
@@ -10,11 +10,22 @@ export const getDefaultOperatorIcon = () => ICON_OPERATOR;
1010

1111
/**
1212
* Modified from PF createIcon, returns a string with the SVG element instead of a React component.
13+
* Supports both old IconDefinition format and new IconConfig format with nested icon property.
1314
*/
1415
export const getSvgFromPfIconConfig = (
15-
{ xOffset = 0, yOffset = 0, width, height, svgPath }: IconDefinition,
16+
iconConfig: IconDefinition | { icon: IconData; [key: string]: any },
1617
className?: string,
1718
): string => {
19+
// Handle new format where icon data is nested under 'icon' property (IconData type)
20+
const iconDef: IconDefinition | IconData = 'icon' in iconConfig ? iconConfig.icon : iconConfig;
21+
const { xOffset = 0, yOffset = 0, width, height } = iconDef;
22+
// IconDefinition uses 'svgPath', IconData uses 'svgPathData'
23+
const pathData =
24+
'svgPath' in iconDef && iconDef.svgPath
25+
? iconDef.svgPath
26+
: 'svgPathData' in iconDef
27+
? iconDef.svgPathData
28+
: '';
1829
const viewBox = [xOffset, yOffset, width, height].join(' ');
1930

2031
return `
@@ -25,6 +36,6 @@ export const getSvgFromPfIconConfig = (
2536
width="1em"
2637
height="1em"
2738
>
28-
<path d='${svgPath}' />
39+
<path d='${pathData}' />
2940
</svg>`;
3041
};

frontend/public/components/masthead/masthead-toolbar.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,6 @@ const MastheadToolbarContents: FC<MastheadToolbarContentsProps> = ({
775775
<NotificationBadge
776776
aria-label={t('public~Notification drawer')}
777777
onClick={drawerToggle}
778-
// @ts-expect-error this prop is accepted as a button variant (but not documented).
779-
// this usage of the undocumented variant was approved by UX
780778
variant="plain"
781779
count={alertCount || 0}
782780
data-quickstart-id="qs-masthead-notifications"
@@ -827,8 +825,6 @@ const MastheadToolbarContents: FC<MastheadToolbarContentsProps> = ({
827825
<NotificationBadge
828826
aria-label={t('public~Notification drawer')}
829827
onClick={drawerToggle}
830-
// @ts-expect-error this prop is accepted as a button variant (but not documented).
831-
// this usage of the undocumented variant was approved by UX
832828
variant="plain"
833829
count={alertCount}
834830
data-quickstart-id="qs-masthead-notifications"

0 commit comments

Comments
 (0)