From adc7bdead64439926ca9298b07be506f19c9893b Mon Sep 17 00:00:00 2001 From: Rastislav Hepner Date: Thu, 7 May 2026 13:14:42 +0200 Subject: [PATCH 1/4] chore: format files prettier & eslint auto-fix --- .../ColumnManagementModal.test.tsx | 95 +++++++++++-------- .../ColumnManagementModal.tsx | 59 ++++++------ 2 files changed, 81 insertions(+), 73 deletions(-) diff --git a/packages/module/src/ColumnManagementModal/ColumnManagementModal.test.tsx b/packages/module/src/ColumnManagementModal/ColumnManagementModal.test.tsx index 7ec26827..7a76f156 100644 --- a/packages/module/src/ColumnManagementModal/ColumnManagementModal.test.tsx +++ b/packages/module/src/ColumnManagementModal/ColumnManagementModal.test.tsx @@ -1,9 +1,9 @@ -import '@testing-library/jest-dom' +import '@testing-library/jest-dom'; import { render, screen, fireEvent } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import ColumnManagementModal, { ColumnManagementModalColumn } from './ColumnManagementModal'; -const DEFAULT_COLUMNS : ColumnManagementModalColumn[] = [ +const DEFAULT_COLUMNS: ColumnManagementModalColumn[] = [ { title: 'ID', key: 'id', @@ -37,17 +37,20 @@ const setColumns = jest.fn(); // Simple mock to track when DragDropSort is used jest.mock('@patternfly/react-drag-drop', () => ({ DragDropSort: ({ children }) =>
{children}
, - Droppable: ({ wrapper }) => wrapper, + Droppable: ({ wrapper }) => wrapper })); -const renderColumnManagementModal = (props = {}) => render( setColumns(newColumns)} - isOpen - onClose={onClose} - data-testid="column-mgmt-modal" - {...props} -/>); +const renderColumnManagementModal = (props = {}) => + render( + setColumns(newColumns)} + isOpen + onClose={onClose} + data-testid="column-mgmt-modal" + {...props} + /> + ); beforeEach(() => { jest.clearAllMocks(); @@ -56,17 +59,21 @@ beforeEach(() => { const getCheckboxesState = () => { // Get only the column checkboxes (exclude the BulkSelect checkbox) - const checkboxes = screen.getByTestId('column-mgmt-modal').querySelectorAll('input[type="checkbox"][data-testid^="column-check-"]'); - return (Array.from(checkboxes) as HTMLInputElement[]).map(c => c.checked); -} + const checkboxes = screen + .getByTestId('column-mgmt-modal') + .querySelectorAll('input[type="checkbox"][data-testid^="column-check-"]'); + return (Array.from(checkboxes) as HTMLInputElement[]).map((c) => c.checked); +}; describe('ColumnManagementModal component', () => { it('should have disabled and checked checkboxes for columns that should always be shown', () => { - expect(getCheckboxesState()).toEqual(DEFAULT_COLUMNS.map(c => c.isShownByDefault)); + expect(getCheckboxesState()).toEqual(DEFAULT_COLUMNS.map((c) => c.isShownByDefault)); }); it('should have checkbox checked if column is shown by default', () => { - const idCheckbox = screen.getByTestId('column-mgmt-modal').querySelector('input[type="checkbox"][data-testid="column-check-id"]'); + const idCheckbox = screen + .getByTestId('column-mgmt-modal') + .querySelector('input[type="checkbox"][data-testid="column-check-id"]'); expect(idCheckbox).toHaveAttribute('disabled'); expect(idCheckbox).toHaveAttribute('checked'); @@ -81,18 +88,20 @@ describe('ColumnManagementModal component', () => { fireEvent.click(screen.getByText('Reset to default')); - expect(getCheckboxesState()).toEqual(DEFAULT_COLUMNS.map(c => c.isShownByDefault)); + expect(getCheckboxesState()).toEqual(DEFAULT_COLUMNS.map((c) => c.isShownByDefault)); }); it('should call onReset callback when reset to default is clicked', () => { const onResetMock = jest.fn(); - render(); + render( + + ); const resetButtons = screen.getAllByText('Reset to default'); // Click the last one rendered (the new modal) @@ -103,13 +112,15 @@ describe('ColumnManagementModal component', () => { it('should display custom reset button label', () => { const customLabel = 'Restaurer par défaut'; - render(); + render( + + ); expect(screen.getByText(customLabel)).toBeInTheDocument(); }); @@ -124,7 +135,7 @@ describe('ColumnManagementModal component', () => { const selectAllButton = screen.getByText('Select all (4)'); await userEvent.click(selectAllButton); - expect(getCheckboxesState()).toEqual(DEFAULT_COLUMNS.map(_ => true)); + expect(getCheckboxesState()).toEqual(DEFAULT_COLUMNS.map((_) => true)); }); it('should change columns on save', () => { @@ -132,7 +143,7 @@ describe('ColumnManagementModal component', () => { fireEvent.click(screen.getByText('Save')); const expectedColumns = DEFAULT_COLUMNS; - const impactColumn = expectedColumns.find(c => c.title === 'Impact'); + const impactColumn = expectedColumns.find((c) => c.title === 'Impact'); if (impactColumn === undefined) { throw new Error('Expected to find "Impact" column in "DEFAULT_COLUMNS"'); @@ -175,16 +186,18 @@ describe('ColumnManagementModal component', () => { DEFAULT_COLUMNS[3], // Score (last -> first) DEFAULT_COLUMNS[0], // ID DEFAULT_COLUMNS[2], // Impact - DEFAULT_COLUMNS[1], // Publish date + DEFAULT_COLUMNS[1] // Publish date ]; const applyColumnsMock = jest.fn(); - render(); + render( + + ); // Click reset to default - get all buttons and click the last one const resetButtons = screen.getAllByText('Reset to default'); @@ -197,7 +210,7 @@ describe('ColumnManagementModal component', () => { // Verify that the saved columns match the original reordered columns order // (after reset, it should restore the order from appliedColumns which is reorderedColumns) expect(applyColumnsMock).toHaveBeenCalledWith( - reorderedColumns.map(col => ({ + reorderedColumns.map((col) => ({ ...col, isShown: col.isShownByDefault })) diff --git a/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx b/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx index 006cb6c3..0fa9481c 100644 --- a/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx +++ b/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx @@ -1,11 +1,6 @@ import type { FunctionComponent } from 'react'; import { useState, useEffect } from 'react'; -import { - Button, - Content, - ContentVariants, - ButtonVariant, -} from '@patternfly/react-core'; +import { Button, Content, ContentVariants, ButtonVariant } from '@patternfly/react-core'; import { ModalProps, Modal, ModalVariant } from '@patternfly/react-core/deprecated'; import ListManager, { ListManagerItem } from '../ListManager/ListManager'; @@ -46,30 +41,32 @@ export interface ColumnManagementModalProps extends Omit = ( - { title = 'Manage columns', - description = 'Selected categories will be displayed in the table.', - isOpen = false, - onClose = () => undefined, - appliedColumns, - applyColumns, - ouiaId = 'ColumnManagementModal', - enableDragDrop = false, - onReset, - resetToDefaultLabel = 'Reset to default', - ...props }: ColumnManagementModalProps) => { - +const ColumnManagementModal: FunctionComponent = ({ + title = 'Manage columns', + description = 'Selected categories will be displayed in the table.', + isOpen = false, + onClose = () => undefined, + appliedColumns, + applyColumns, + ouiaId = 'ColumnManagementModal', + enableDragDrop = false, + onReset, + resetToDefaultLabel = 'Reset to default', + ...props +}: ColumnManagementModalProps) => { const [ currentColumns, setCurrentColumns ] = useState(() => - appliedColumns.map(column => ({ ...column, isShown: column.isShown ?? column.isShownByDefault })) + appliedColumns.map((column) => ({ ...column, isShown: column.isShown ?? column.isShownByDefault })) ); // Sync with appliedColumns when they change useEffect(() => { - setCurrentColumns(appliedColumns.map(column => ({ ...column, isShown: column.isShown ?? column.isShownByDefault }))); + setCurrentColumns( + appliedColumns.map((column) => ({ ...column, isShown: column.isShown ?? column.isShownByDefault })) + ); }, [ appliedColumns ]); // Convert ColumnManagementModalColumn to ListManagerItem - const listManagerItems: ListManagerItem[] = currentColumns.map(column => ({ + const listManagerItems: ListManagerItem[] = currentColumns.map((column) => ({ key: column.key, title: column.title, isSelected: column.isShown, @@ -79,16 +76,14 @@ const ColumnManagementModal: FunctionComponent = ( const resetToDefault = () => { // Reset both visibility and order to match the original appliedColumns - setCurrentColumns(appliedColumns.map(column => ({ ...column, isShown: column.isShownByDefault ?? false }))); + setCurrentColumns(appliedColumns.map((column) => ({ ...column, isShown: column.isShownByDefault ?? false }))); onReset?.(); }; const updateColumns = (items: ListManagerItem[]) => { - const newColumns = currentColumns.map(column => { - const matchingItem = items.find(item => item.key === column.key); - return matchingItem - ? { ...column, isShown: matchingItem.isSelected ?? column.isShownByDefault } - : column; + const newColumns = currentColumns.map((column) => { + const matchingItem = items.find((item) => item.key === column.key); + return matchingItem ? { ...column, isShown: matchingItem.isSelected ?? column.isShownByDefault } : column; }); setCurrentColumns(newColumns); }; @@ -103,8 +98,8 @@ const ColumnManagementModal: FunctionComponent = ( const handleOrderChange = (items: ListManagerItem[]) => { // Update the order of currentColumns based on the new order from ListManager - const newColumns = items.map(item => { - const originalColumn = currentColumns.find(col => col.key === item.key); + const newColumns = items.map((item) => { + const originalColumn = currentColumns.find((col) => col.key === item.key); if (!originalColumn) { throw new Error(`Column with key ${item.key} not found`); } @@ -114,7 +109,7 @@ const ColumnManagementModal: FunctionComponent = ( }; const handleSave = (items: ListManagerItem[]) => { - const updatedColumns = items.map(item => ({ + const updatedColumns = items.map((item) => ({ key: item.key, title: item.title, isShown: item.isSelected, @@ -158,6 +153,6 @@ const ColumnManagementModal: FunctionComponent = ( /> ); -} +}; export default ColumnManagementModal; From 313782250e9cb88c4b5be32a1e9d5d1d3d93b5a5 Mon Sep 17 00:00:00 2001 From: Rastislav Hepner Date: Thu, 7 May 2026 13:20:27 +0200 Subject: [PATCH 2/4] feat: add generic parameter for appliedColumns and applyColumns --- .../ColumnManagementModal/ColumnManagementModal.tsx | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx b/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx index 0fa9481c..fb8d3987 100644 --- a/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx +++ b/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx @@ -1,4 +1,3 @@ -import type { FunctionComponent } from 'react'; import { useState, useEffect } from 'react'; import { Button, Content, ContentVariants, ButtonVariant } from '@patternfly/react-core'; import { ModalProps, Modal, ModalVariant } from '@patternfly/react-core/deprecated'; @@ -18,15 +17,15 @@ export interface ColumnManagementModalColumn { } /** extends ModalProps */ -export interface ColumnManagementModalProps extends Omit { +export interface ColumnManagementModalProps extends Omit { /** Flag to show the modal */ isOpen?: boolean; /** Invoked when modal visibility is changed */ onClose?: (event: KeyboardEvent | React.MouseEvent) => void; /** Current column state */ - appliedColumns: ColumnManagementModalColumn[]; + appliedColumns: T[]; /** Invoked with new column state after save button is clicked */ - applyColumns: (newColumns: ColumnManagementModalColumn[]) => void; + applyColumns: (newColumns: T[]) => void; /* Modal description text */ description?: string; /* Modal title text */ @@ -41,7 +40,7 @@ export interface ColumnManagementModalProps extends Omit = ({ +const ColumnManagementModal = ({ title = 'Manage columns', description = 'Selected categories will be displayed in the table.', isOpen = false, @@ -53,7 +52,7 @@ const ColumnManagementModal: FunctionComponent = ({ onReset, resetToDefaultLabel = 'Reset to default', ...props -}: ColumnManagementModalProps) => { +}: ColumnManagementModalProps) => { const [ currentColumns, setCurrentColumns ] = useState(() => appliedColumns.map((column) => ({ ...column, isShown: column.isShown ?? column.isShownByDefault })) ); From 618b60c41cacf7ece6a940a45d9ebc5e910aa7ae Mon Sep 17 00:00:00 2001 From: Rastislav Hepner Date: Thu, 7 May 2026 14:02:32 +0200 Subject: [PATCH 3/4] feat: save same column shape as it was received --- .../ColumnManagementModal.tsx | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx b/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx index fb8d3987..fa6a1606 100644 --- a/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx +++ b/packages/module/src/ColumnManagementModal/ColumnManagementModal.tsx @@ -17,7 +17,8 @@ export interface ColumnManagementModalColumn { } /** extends ModalProps */ -export interface ColumnManagementModalProps extends Omit { +export interface ColumnManagementModalProps + extends Omit { /** Flag to show the modal */ isOpen?: boolean; /** Invoked when modal visibility is changed */ @@ -53,7 +54,7 @@ const ColumnManagementModal = ) => { - const [ currentColumns, setCurrentColumns ] = useState(() => + const [ currentColumns, setCurrentColumns ] = useState(() => appliedColumns.map((column) => ({ ...column, isShown: column.isShown ?? column.isShownByDefault })) ); @@ -108,13 +109,13 @@ const ColumnManagementModal = { - const updatedColumns = items.map((item) => ({ - key: item.key, - title: item.title, - isShown: item.isSelected, - isShownByDefault: item.isShownByDefault, - isUntoggleable: item.isUntoggleable - })); + const updatedColumns = items.map((item) => { + const originalColumn = currentColumns.find((col) => col.key === item.key); + if (!originalColumn) { + throw new Error(`Column with key ${item.key} not found`); + } + return { ...originalColumn, isShown: item.isSelected }; + }); applyColumns(updatedColumns); onClose({} as KeyboardEvent); }; From c8ac82e33f03d8358d885a2cd63e14d21c655f95 Mon Sep 17 00:00:00 2001 From: Rastislav Hepner Date: Thu, 7 May 2026 14:19:36 +0200 Subject: [PATCH 4/4] test: preserve extension keys on columns Assisted-by: Cursor --- .../ColumnManagementModal.test.tsx | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/module/src/ColumnManagementModal/ColumnManagementModal.test.tsx b/packages/module/src/ColumnManagementModal/ColumnManagementModal.test.tsx index 7a76f156..a23ec2dc 100644 --- a/packages/module/src/ColumnManagementModal/ColumnManagementModal.test.tsx +++ b/packages/module/src/ColumnManagementModal/ColumnManagementModal.test.tsx @@ -1,5 +1,5 @@ import '@testing-library/jest-dom'; -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import ColumnManagementModal, { ColumnManagementModalColumn } from './ColumnManagementModal'; @@ -31,6 +31,15 @@ const DEFAULT_COLUMNS: ColumnManagementModalColumn[] = [ } ]; +interface ExtendedColumn extends ColumnManagementModalColumn { + dataKey: string; +} + +const EXTENDED_COLUMNS: ExtendedColumn[] = DEFAULT_COLUMNS.map((col) => ({ + ...col, + dataKey: `row.${col.key}` +})); + const onClose = jest.fn(); const setColumns = jest.fn(); @@ -164,6 +173,35 @@ describe('ColumnManagementModal component', () => { expect(setColumns).not.toHaveBeenCalled(); }); + it('should preserve extended column fields when saving', () => { + const applyColumnsMock = jest.fn(); + render( + + appliedColumns={EXTENDED_COLUMNS} + applyColumns={applyColumnsMock} + isOpen + onClose={onClose} + data-testid="extended-column-modal" + /> + ); + + const modal = screen.getByTestId('extended-column-modal'); + fireEvent.click(within(modal).getByTestId('column-check-impact')); + fireEvent.click(within(modal).getByRole('button', { name: 'Save' })); + + expect(applyColumnsMock).toHaveBeenCalledTimes(1); + const saved = applyColumnsMock.mock.calls[0][0] as ExtendedColumn[]; + + // Test that we have same number of columns + expect(saved).toHaveLength(EXTENDED_COLUMNS.length); + + // Test that extension keys were preserved + saved.forEach((col) => { + const source = EXTENDED_COLUMNS.find((aec) => aec.key === col.key); + expect(source?.dataKey).toBe(col.dataKey); + }); + }); + describe('enableDragDrop prop', () => { it('should default enableDragDrop to false', () => { // Default behavior should not enable drag and drop