Skip to content

CONSOLE-5040: use ConsoleDataViewColumn instead of TableColumn #16246

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

Open
vikram-raj wants to merge 4 commits into
base: main
Choose a base branch
from
Open

CONSOLE-5040: use ConsoleDataViewColumn instead of TableColumn #16246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a9d6eef
use ConsoleDataViewColumn instead of TableColumn
vikram-raj Apr 2, 2026
9f53167
fix PDB list Selector column sorting
vikram-raj Apr 6, 2026
e158e0b
fix Nodes list Labels column sorting
vikram-raj Apr 6, 2026
a0ed4ae
improve code quality in DataView column sorting
vikram-raj Apr 6, 2026
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
2 changes: 2 additions & 0 deletions frontend/packages/console-app/src/components/data-view/ConsoleDataView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,3 +285,5 @@ export const getLabelsColumnWidthStyleProp = (width: number | undefined, default
width: `${width ?? defaultWidth}px`,
},
});

export const MODIFIER_NOWRAP = 'nowrap' as const;
44 changes: 31 additions & 13 deletions frontend/packages/console-app/src/components/data-view/useConsoleDataViewData.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import { useTranslation } from 'react-i18next';
import { useSearchParams } from 'react-router';
import type { ConsoleDataViewTh } from '@console/dynamic-plugin-sdk/src/api/internal-types';
import type {
TableColumn,
DataViewManagedColumn,
RowProps,
TableColumn,
} from '@console/dynamic-plugin-sdk/src/extensions/console-types';
import { useActiveColumns } from '@console/internal/components/factory/Table/active-columns-hook';
import { sortResourceByValue } from '@console/internal/components/factory/Table/sort';
Expand All @@ -21,7 +22,10 @@ import { useConsoleDataViewSort, getSortByDirection } from './useConsoleDataView
const isDataViewConfigurableColumn = (
column: ConsoleDataViewTh,
): column is Extract<DataViewTh, { cell: ReactNode }> => {
return (column as any)?.cell !== undefined;
if (column == null || typeof column !== 'object') {
return false;
}
return 'cell' in column && (column as { cell?: ReactNode }).cell !== undefined;
};

export const useConsoleDataViewData = <
Expand All @@ -38,7 +42,7 @@ export const useConsoleDataViewData = <
customRowData,
isResizable = true,
}: {
columns: TableColumn<TData>[];
columns: DataViewManagedColumn<TData>[];
filteredData: TData[];
filters: TFilters;
getDataViewRows: GetDataViewRows<TData, TCustomRowData>;
Expand Down Expand Up @@ -89,13 +93,21 @@ export const useConsoleDataViewData = <

const dataViewColumns = useMemo<ConsoleDataViewColumn<TData>[]>(
() =>
activeColumns.map(({ id, title, sort, props, resizableProps }, index) => {
activeColumns.map((column, index) => {
const { id, title } = column;
const { props, resizableProps } = column as TableColumn<TData>;
const sortFunction =
'sortFunction' in column && column.sortFunction !== undefined
? column.sortFunction
: (column as TableColumn<TData>).sort;

Comment on lines +99 to +103
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Normalize legacy sort fallback to exclude boolean values

Line 99 can propagate TableColumn.sort booleans into sortFunction. When this is true, the column gets sortable header state, but the data never sorts because Lines 152–162 only execute for string or function. Restrict fallback to string keys only.

Suggested fix 
-        const sortFunction =
-          'sortFunction' in column && column.sortFunction !== undefined
-            ? column.sortFunction
-            : (column as TableColumn<TData>).sort;
+        const legacySort = (column as TableColumn<TData>).sort;
+        const sortFunction =
+          'sortFunction' in column && column.sortFunction !== undefined
+            ? column.sortFunction
+            : typeof legacySort === 'string'
+              ? legacySort
+              : undefined;

-        const headerProps: ThProps = {
-          ...props,
-          dataLabel: title,
-          sort: sortFunction,
-        };
+        const headerProps: ThProps = {
+          ...props,
+          dataLabel: title,
+        };

Also applies to: 152-162

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@frontend/packages/console-app/src/components/data-view/useConsoleDataViewData.tsx`
around lines 96 - 100, The fallback assignment to sortFunction should not accept
boolean TableColumn.sort values; change the logic in useConsoleDataViewData so
that sortFunction is set to column.sort only when typeof (column as
TableColumn<TData>).sort === 'string' (exclude booleans), otherwise keep
undefined; likewise ensure the subsequent sorting-path that inspects sort (used
where sort keys/handlers are applied) only treats string keys or function values
(i.e., guard against boolean sort values) so sorting headers don't become active
without sortable behavior.

const headerProps: ThProps = {
...props,
dataLabel: title,
sort: sortFunction,
};

if (sort) {
if (sortFunction) {
headerProps.sort = {
columnIndex: index,
sortBy: {
Expand All @@ -106,17 +118,23 @@ export const useConsoleDataViewData = <
};
}

const headerCell =
isDataViewConfigurableColumn(column as ConsoleDataViewTh) &&
(column as { cell?: ReactNode }).cell !== undefined ? (
(column as { cell: ReactNode }).cell
) : title ? (
<span>{title}</span>
) : (
<span className="pf-v6-u-screen-reader">{t('public~Actions')}</span>
);

return {
id,
title,
sortFunction: sort,
sortFunction,
props: headerProps,
resizableProps: isResizable ? resizableProps : undefined,
cell: title ? (
<span>{title}</span>
) : (
<span className="pf-v6-u-screen-reader">{t('public~Actions')}</span>
),
cell: headerCell,
};
}),
[activeColumns, t, isResizable],
Expand All @@ -135,13 +153,13 @@ export const useConsoleDataViewData = <
}

if (typeof sortColumn.sortFunction === 'string') {
return filteredData.sort(
return [...filteredData].sort(
sortResourceByValue(sortDirection, (obj) => _.get(obj, sortColumn.sortFunction as string)),
);
}

if (typeof sortColumn.sortFunction === 'function') {
return sortColumn.sortFunction(filteredData, sortDirection);
return sortColumn.sortFunction([...filteredData], sortDirection);
}

return filteredData;
Expand Down
89 changes: 51 additions & 38 deletions frontend/packages/console-app/src/components/nodes/NodesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ConsoleDataView,
nameCellProps,
getLabelsColumnWidthStyleProp,
MODIFIER_NOWRAP,
} from '@console/app/src/components/data-view/ConsoleDataView';
import type {
ConsoleDataViewColumn,
Expand All @@ -32,12 +33,12 @@ import type {
NodeCertificateSigningRequestKind,
OwnerReference,
RowProps,
TableColumn,
} from '@console/dynamic-plugin-sdk/src/extensions/console-types';
import type { NodeMetrics } from '@console/internal/actions/ui';
import { setNodeMetrics } from '@console/internal/actions/ui';
import { coFetchJSON } from '@console/internal/co-fetch';
import ListPageHeader from '@console/internal/components/factory/ListPage/ListPageHeader';
import { sortResourceByValue } from '@console/internal/components/factory/Table/sort';
import { PROMETHEUS_BASE_PATH } from '@console/internal/components/graphs';
import { getPrometheusURL, PrometheusEndpoint } from '@console/internal/components/graphs/helpers';
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
Expand Down Expand Up @@ -170,10 +171,21 @@ const nodeColumnInfo = Object.freeze({

const kind = 'Node';

const nodeRowLabelsSortKey = (obj: K8sResourceCommon) => {
const labels = getLabels(obj);
if (_.isEmpty(labels)) {
return '';
}
return Object.keys(labels)
.sort()
.map((k) => `${k}=${labels[k]}`)
.join(',');
};

const useNodesColumns = (
vmsEnabled: boolean,
nodeMgmtV1Enabled: boolean,
): { columns: TableColumn<NodeRowItem>[]; resetAllColumnWidths: () => void } => {
): { columns: ConsoleDataViewColumn<NodeRowItem>[]; resetAllColumnWidths: () => void } => {
const { t } = useTranslation();
const { getResizableProps, getWidth, resetAllColumnWidths } = useColumnWidthSettings(NodeModel);

Expand All @@ -182,53 +194,53 @@ const useNodesColumns = (
{
title: t('console-app~Name'),
id: nodeColumnInfo.name.id,
sort: 'metadata.name',
sortFunction: 'metadata.name',
resizableProps: getResizableProps(nodeColumnInfo.name.id),
props: {
...nameCellProps,
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
},
{
title: t('console-app~Status'),
id: nodeColumnInfo.status.id,
sort: sortWithCSRResource(nodeReadiness, 'False'),
sortFunction: sortWithCSRResource(nodeReadiness, 'False'),
resizableProps: getResizableProps(nodeColumnInfo.status.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
},
...(nodeMgmtV1Enabled
? [
{
title: t('console-app~Groups'),
id: nodeColumnInfo.groups.id,
sort: 'groups',
sortFunction: 'groups',
resizableProps: getResizableProps(nodeColumnInfo.groups.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
},
]
: []),
{
title: t('console-app~Machine set'),
id: nodeColumnInfo.machineOwner.id,
sort: 'machineOwner.name',
sortFunction: 'machineOwner.name',
resizableProps: getResizableProps(nodeColumnInfo.machineOwner.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
},
...(vmsEnabled
? [
{
title: t('console-app~Virtual machines'),
id: nodeColumnInfo.vms.id,
sort: 'virtualMachines',
sortFunction: 'virtualMachines',
resizableProps: getResizableProps(nodeColumnInfo.vms.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
info: {
tooltip: t(
'console-app~This count is based on your access permissions and might not include all virtual machines.',
Expand All @@ -244,128 +256,129 @@ const useNodesColumns = (
{
title: t('console-app~Pods'),
id: nodeColumnInfo.pods.id,
sort: sortWithCSRResource(nodePods, 0),
sortFunction: sortWithCSRResource(nodePods, 0),
resizableProps: getResizableProps(nodeColumnInfo.pods.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
},
{
title: t('console-app~Memory'),
id: nodeColumnInfo.memory.id,
sort: sortWithCSRResource(nodeMemory, 0),
sortFunction: sortWithCSRResource(nodeMemory, 0),
resizableProps: getResizableProps(nodeColumnInfo.memory.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
},
{
title: t('console-app~CPU'),
id: nodeColumnInfo.cpu.id,
sort: sortWithCSRResource(nodeCPU, 0),
sortFunction: sortWithCSRResource(nodeCPU, 0),
resizableProps: getResizableProps(nodeColumnInfo.cpu.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
},
{
title: t('console-app~Roles'),
id: nodeColumnInfo.role.id,
sort: sortWithCSRResource(nodeRolesSort, ''),
sortFunction: sortWithCSRResource(nodeRolesSort, ''),
resizableProps: getResizableProps(nodeColumnInfo.role.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
additional: true,
},
{
title: t('console-app~Architecture'),
id: nodeColumnInfo.architecture.id,
sort: sortWithCSRResource(nodeArch, ''),
sortFunction: sortWithCSRResource(nodeArch, ''),
resizableProps: getResizableProps(nodeColumnInfo.architecture.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
additional: true,
},
{
title: t('console-app~Filesystem'),
id: nodeColumnInfo.filesystem.id,
sort: sortWithCSRResource(nodeFS, 0),
sortFunction: sortWithCSRResource(nodeFS, 0),
resizableProps: getResizableProps(nodeColumnInfo.filesystem.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
additional: true,
},
{
title: t('console-app~Created'),
id: nodeColumnInfo.created.id,
sort: 'metadata.creationTimestamp',
sortFunction: 'metadata.creationTimestamp',
resizableProps: getResizableProps(nodeColumnInfo.created.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
additional: true,
},
{
title: t('console-app~Instance type'),
id: nodeColumnInfo.instanceType.id,
sort: sortWithCSRResource(nodeInstanceType, ''),
sortFunction: sortWithCSRResource(nodeInstanceType, ''),
resizableProps: getResizableProps(nodeColumnInfo.instanceType.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
additional: true,
},
{
title: t('console-app~Machine'),
id: nodeColumnInfo.machine.id,
sort: sortWithCSRResource(nodeMachine, ''),
sortFunction: sortWithCSRResource(nodeMachine, ''),
resizableProps: getResizableProps(nodeColumnInfo.machine.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
additional: true,
},
{
title: t('console-app~MachineConfigPool'),
id: nodeColumnInfo.machineConfigPool.id,
sort: 'machineConfigPool.metadata.name',
sortFunction: 'machineConfigPool.metadata.name',
resizableProps: getResizableProps(nodeColumnInfo.machineConfigPool.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
additional: true,
},
{
title: t('console-app~Labels'),
id: nodeColumnInfo.labels.id,
sort: 'metadata.labels',
sortFunction: (data, direction) =>
[...data].sort(sortResourceByValue(direction, nodeRowLabelsSortKey)),
resizableProps: getResizableProps(nodeColumnInfo.labels.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
...getLabelsColumnWidthStyleProp(getWidth(nodeColumnInfo.labels.id)),
},
additional: true,
},
{
title: t('console-app~Zone'),
id: nodeColumnInfo.zone.id,
sort: sortWithCSRResource(nodeZone, ''),
sortFunction: sortWithCSRResource(nodeZone, ''),
resizableProps: getResizableProps(nodeColumnInfo.zone.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
additional: true,
},
{
title: t('console-app~Uptime'),
id: nodeColumnInfo.uptime.id,
sort: sortWithCSRResource(nodeUptime, ''),
sortFunction: sortWithCSRResource(nodeUptime, ''),
resizableProps: getResizableProps(nodeColumnInfo.uptime.id),
props: {
modifier: 'nowrap',
modifier: MODIFIER_NOWRAP,
},
additional: true,
},
Expand Down
Loading