Skip to content

Commit b2a1406

Browse files
mturleyclaude
mturley
and
claude
authored
Add manage columns functionality to HardwareConfigurationTable using reusable ManageColumnsModal and useManageColumns hook (#2101)
* Initial commit copying code from opendatahub-io/odh-dashboard#5825 (e46d06125bb15897179b6e88e43ec11593ee9c76) Signed-off-by: Mike Turley <[email protected]> * Fix imports Signed-off-by: Mike Turley <[email protected]> * Add manage columns functionality to HardwareConfigurationTable Integrate the ManageColumnsModal and useManageColumns pattern to allow users to customize which columns are visible in the hardware configuration table and their order. Key changes: - Create useHardwareConfigColumns hook that combines manage columns with latency filter effects (when filter changes, relevant columns update) - Add constants for sticky columns, default visible fields, and storage key - Add toolbarActions prop to HardwareConfigurationFilterToolbar - Integrate ManageColumnsModal in HardwareConfigurationTable - Fix import paths in copied odh-dashboard components Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Mike Turley <[email protected]> * Fix TPS column names Signed-off-by: Mike Turley <[email protected]> * TODO add this to ODH version - reset default columns button Signed-off-by: Mike Turley <[email protected]> * TODO add this to ODH version - Move modal state into useManageColumns hook Refactored ManageColumnsModal to get isModalOpen and closeModal from manageColumnsResult instead of having separate isOpen and onClose props. This simplifies usage by letting the hook manage all modal state internally. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Mike Turley <[email protected]> * Remove unnecessary type conversion HardwareConfigColumn is directly assignable to SortableData since HardwareConfigColumnField (a string literal union) is a subtype of string. Removed the useMemo conversion and unused SortableData import. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Mike Turley <[email protected]> * Rearrange columns Signed-off-by: Mike Turley <[email protected]> * Fix description Signed-off-by: Mike Turley <[email protected]> * Add comment Signed-off-by: Mike Turley <[email protected]> * TODO add to ODH version - Refactor for more generic types Signed-off-by: Mike Turley <[email protected]> * Add comment Signed-off-by: Mike Turley <[email protected]> * Fix key used for sticky hardware config column Signed-off-by: Mike Turley <[email protected]> * Fix test Signed-off-by: Mike Turley <[email protected]> * TODO add to ODH version - Fix search to handle NBSP and show empty state - Normalize whitespace (including NBSP) to regular spaces when comparing column labels with search input - Add EmptyState component when search returns no matching columns Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Mike Turley <[email protected]> --------- Signed-off-by: Mike Turley <[email protected]> Co-authored-by: Claude Opus 4.5 <[email protected]>
1 parent 353a878 commit b2a1406

File tree

19 files changed

+1912
-111
lines changed

19 files changed

+1912
-111
lines changed

‎clients/ui/frontend/package-lock.json‎

Lines changed: 93 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎clients/ui/frontend/package.json‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
"@emotion/styled": "^11.14.0",
9393
"@patternfly/patternfly": "^6.4.0",
9494
"@patternfly/react-core": "^6.4.0",
95+
"@patternfly/react-drag-drop": "^6.4.0",
9596
"@patternfly/react-icons": "^6.4.0",
9697
"@patternfly/react-styles": "^6.4.0",
9798
"@patternfly/react-table": "^6.4.0",
@@ -104,15 +105,15 @@
104105
"mod-arch-shared": "~1.2.2",
105106
"react": "^18",
106107
"react-dom": "^18",
108+
"react-markdown": "^10.1.0",
107109
"react-router": "^7.12.0",
108110
"react-router-dom": "^7.12.0",
109-
"sass": "^1.83.0",
110-
"showdown": "^2.1.0",
111-
"react-markdown": "^10.1.0",
112111
"rehype-raw": "^7.0.0",
113112
"rehype-sanitize": "^6.0.0",
114113
"rehype-unwrap-images": "^1.0.0",
115-
"remark-gfm": "^4.0.1"
114+
"remark-gfm": "^4.0.1",
115+
"sass": "^1.83.0",
116+
"showdown": "^2.1.0"
116117
},
117118
"optionalDependencies": {
118119
"@babel/preset-env": "^7.26.9",

‎clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelCatalog/modelCatalogTabs.cy.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ describe('Model Catalog Details Tabs', () => {
314314
// TPS P90 column should be visible (matching percentile)
315315
modelCatalog
316316
.findHardwareConfigurationTableHeaders()
317-
.should('contain.text', `TPS${NBSP}Latency P90`);
317+
.should('contain.text', `TPS${NBSP}P90`);
318318
});
319319
});
320320

@@ -337,7 +337,7 @@ describe('Model Catalog Details Tabs', () => {
337337
// TPS P90 column should be visible (matching percentile)
338338
modelCatalog
339339
.findHardwareConfigurationTableHeaders()
340-
.should('contain.text', `TPS${NBSP}Latency P90`);
340+
.should('contain.text', `TPS${NBSP}P90`);
341341

342342
// E2E and ITL columns should be hidden
343343
modelCatalog.findHardwareConfigurationTableHeaders().should('not.contain.text', 'E2E');
@@ -368,7 +368,7 @@ describe('Model Catalog Details Tabs', () => {
368368
// TPS Mean column should be visible (matching percentile)
369369
modelCatalog
370370
.findHardwareConfigurationTableHeaders()
371-
.should('contain.text', `TPS${NBSP}Latency Mean`);
371+
.should('contain.text', `TPS${NBSP}Mean`);
372372

373373
// TTFT and ITL columns should be hidden
374374
modelCatalog.findHardwareConfigurationTableHeaders().should('not.contain.text', 'TTFT');
@@ -405,7 +405,7 @@ describe('Model Catalog Details Tabs', () => {
405405
.should('contain.text', `TTFT${NBSP}Latency P90`);
406406
modelCatalog
407407
.findHardwareConfigurationTableHeaders()
408-
.should('contain.text', `TPS${NBSP}Latency P90`);
408+
.should('contain.text', `TPS${NBSP}P90`);
409409
// E2E and ITL should NOT be visible (filter is applied, not cleared)
410410
modelCatalog.findHardwareConfigurationTableHeaders().should('not.contain.text', 'E2E');
411411
modelCatalog.findHardwareConfigurationTableHeaders().should('not.contain.text', 'ITL');
@@ -422,7 +422,7 @@ describe('Model Catalog Details Tabs', () => {
422422
// Non-latency columns should still be visible
423423
modelCatalog
424424
.findHardwareConfigurationTableHeaders()
425-
.should('contain.text', 'Hardware Configuration');
425+
.should('contain.text', 'Hardware configuration');
426426
modelCatalog
427427
.findHardwareConfigurationTableHeaders()
428428
.should('contain.text', 'Workload type');

‎clients/ui/frontend/src/app/pages/modelCatalog/components/HardwareConfigurationFilterToolbar.tsx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,15 @@ type HardwareConfigurationFilterToolbarProps = {
2727
includeBasicFilters?: boolean;
2828
/** If true, shows performance filter chips. Landing page passes performanceViewEnabled, details page passes true. */
2929
includePerformanceFilters?: boolean;
30+
/** Optional content to render at the end of the toolbar (e.g., manage columns button). */
31+
toolbarActions?: React.ReactNode;
3032
};
3133

3234
const HardwareConfigurationFilterToolbar: React.FC<HardwareConfigurationFilterToolbarProps> = ({
3335
onResetAllFilters,
3436
includeBasicFilters = false,
3537
includePerformanceFilters = true,
38+
toolbarActions,
3639
}) => {
3740
const {
3841
filterOptions,
@@ -155,6 +158,12 @@ const HardwareConfigurationFilterToolbar: React.FC<HardwareConfigurationFilterTo
155158
<ToolbarItem>
156159
<HardwareConfigurationFilter />
157160
</ToolbarItem>
161+
{toolbarActions && (
162+
<>
163+
<ToolbarItem variant="separator" />
164+
<ToolbarItem>{toolbarActions}</ToolbarItem>
165+
</>
166+
)}
158167
</ToolbarGroup>
159168
<ModelCatalogActiveFilters filtersToShow={filtersToShow} />
160169
</ToolbarContent>

‎clients/ui/frontend/src/app/pages/modelCatalog/components/HardwareConfigurationTable.tsx‎

Lines changed: 45 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,15 @@
11
import * as React from 'react';
22
import { DashboardEmptyTableView, Table } from 'mod-arch-shared';
3-
import { Spinner } from '@patternfly/react-core';
3+
import { Button, Spinner } from '@patternfly/react-core';
4+
import { ColumnsIcon } from '@patternfly/react-icons';
45
import { OuterScrollContainer } from '@patternfly/react-table';
56
import { CatalogPerformanceMetricsArtifact } from '~/app/modelCatalogTypes';
67
import { ModelCatalogContext } from '~/app/context/modelCatalog/ModelCatalogContext';
7-
import {
8-
ALL_LATENCY_PROPERTY_KEYS,
9-
getLatencyPropertyKey,
10-
LatencyMetric,
11-
parseLatencyFilterKey,
12-
} from '~/concepts/modelCatalog/const';
138
import { getActiveLatencyFieldName } from '~/app/pages/modelCatalog/utils/modelCatalogUtils';
14-
import { hardwareConfigColumns, HardwareConfigColumn } from './HardwareConfigurationTableColumns';
9+
import { ManageColumnsModal } from '~/app/shared/components/manageColumns/ManageColumnsModal';
1510
import HardwareConfigurationTableRow from './HardwareConfigurationTableRow';
1611
import HardwareConfigurationFilterToolbar from './HardwareConfigurationFilterToolbar';
12+
import { useHardwareConfigColumns } from './useHardwareConfigColumns';
1713

1814
type HardwareConfigurationTableProps = {
1915
performanceArtifacts: CatalogPerformanceMetricsArtifact[];
@@ -32,44 +28,8 @@ const HardwareConfigurationTable: React.FC<HardwareConfigurationTableProps> = ({
3228
// Get the active latency filter field name (if any)
3329
const activeLatencyField = getActiveLatencyFieldName(filterData);
3430

35-
// When a latency filter is selected, show only that column and hide other latency columns
36-
// Also show the TPS column with the matching percentile (e.g., TTFT P90 filter shows TPS P90)
37-
const filteredColumns = React.useMemo((): HardwareConfigColumn[] => {
38-
if (!activeLatencyField) {
39-
// No latency filter selected, show all columns
40-
return hardwareConfigColumns;
41-
}
42-
43-
// Parse the active filter field name to extract metric, percentile, and propertyKey
44-
const parsed = parseLatencyFilterKey(activeLatencyField);
45-
46-
// Get the property key (short format) that matches the column field
47-
const activePropertyKey = parsed.propertyKey;
48-
49-
// Build the matching TPS property key using the same percentile (e.g., TTFT P90 filter shows TPS P90)
50-
const matchingTpsPropertyKey = getLatencyPropertyKey(LatencyMetric.TPS, parsed.percentile);
51-
52-
// Filter out latency columns that don't match the active filter
53-
return hardwareConfigColumns.filter((column) => {
54-
// Check if this column is a latency column (using short property keys)
55-
const isLatencyColumn = ALL_LATENCY_PROPERTY_KEYS.some(
56-
(propertyKey) => propertyKey === column.field,
57-
);
58-
59-
// If it's not a latency column, keep it
60-
if (!isLatencyColumn) {
61-
return true;
62-
}
63-
64-
// Show TPS column with matching percentile (they measure throughput, not latency delay)
65-
if (column.field === matchingTpsPropertyKey) {
66-
return true;
67-
}
68-
69-
// If it's a latency column (not TPS), only keep it if it matches the active filter
70-
return column.field === activePropertyKey;
71-
});
72-
}, [activeLatencyField]);
31+
// Use the custom hook that combines manage columns with latency filter effects
32+
const { columns, manageColumnsResult } = useHardwareConfigColumns(activeLatencyField);
7333

7434
if (isLoading) {
7535
return <Spinner size="lg" />;
@@ -80,35 +40,54 @@ const HardwareConfigurationTable: React.FC<HardwareConfigurationTableProps> = ({
8040
resetPerformanceFiltersToDefaults();
8141
};
8242

43+
const manageColumnsButton = (
44+
<Button
45+
variant="link"
46+
icon={<ColumnsIcon />}
47+
onClick={manageColumnsResult.openModal}
48+
data-testid="manage-columns-button"
49+
>
50+
Manage columns
51+
</Button>
52+
);
53+
8354
const toolbarContent = (
8455
<HardwareConfigurationFilterToolbar
8556
onResetAllFilters={handleClearFilters}
8657
includePerformanceFilters
58+
toolbarActions={manageColumnsButton}
8759
/>
8860
);
8961

9062
return (
91-
<OuterScrollContainer>
92-
<Table
93-
data-testid="hardware-configuration-table"
94-
variant="compact"
95-
isStickyHeader
96-
hasStickyColumns
97-
data={performanceArtifacts}
98-
columns={filteredColumns}
99-
toolbarContent={toolbarContent}
100-
onClearFilters={handleClearFilters}
101-
defaultSortColumn={0}
102-
emptyTableView={<DashboardEmptyTableView onClearFilters={handleClearFilters} />}
103-
rowRenderer={(artifact) => (
104-
<HardwareConfigurationTableRow
105-
key={artifact.customProperties?.config_id?.string_value}
106-
performanceArtifact={artifact}
107-
columns={filteredColumns}
108-
/>
109-
)}
63+
<>
64+
<OuterScrollContainer>
65+
<Table
66+
data-testid="hardware-configuration-table"
67+
variant="compact"
68+
isStickyHeader
69+
hasStickyColumns
70+
data={performanceArtifacts}
71+
columns={columns}
72+
toolbarContent={toolbarContent}
73+
onClearFilters={handleClearFilters}
74+
defaultSortColumn={0}
75+
emptyTableView={<DashboardEmptyTableView onClearFilters={handleClearFilters} />}
76+
rowRenderer={(artifact) => (
77+
<HardwareConfigurationTableRow
78+
key={artifact.customProperties?.config_id?.string_value}
79+
performanceArtifact={artifact}
80+
columns={columns}
81+
/>
82+
)}
83+
/>
84+
</OuterScrollContainer>
85+
<ManageColumnsModal
86+
manageColumnsResult={manageColumnsResult}
87+
description="Manage the columns that will appear in the hardware configuration table."
88+
dataTestId="hardware-config-manage-columns"
11089
/>
111-
</OuterScrollContainer>
90+
</>
11291
);
11392
};
11493

0 commit comments

Comments
 (0)