Skip to content

Commit 288388e

Browse files
authored
fix(columnManagementModal): refactor to use ListManager in modal (#815)
* fix(columnManagementModal): refactor to use ListManager in modal * address review comments * put back required aria-labelledby * fix lint error
1 parent 0ff966e commit 288388e

File tree

10 files changed

+370
-149
lines changed

10 files changed

+370
-149
lines changed

package-lock.json

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

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@
8383
"whatwg-fetch": "^3.6.20"
8484
},
8585
"dependencies": {
86-
"@patternfly/react-drag-drop": "^6.3.0",
86+
"@patternfly/react-drag-drop": "^6.0.0",
8787
"@patternfly/react-tokens": "^6.0.0",
8888
"sharp": "^0.34.0"
8989
}

packages/module/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
"react-jss": "^10.10.0"
3939
},
4040
"peerDependencies": {
41+
"@patternfly/react-drag-drop": "^6.0.0",
4142
"react": "^17 || ^18 || ^19",
4243
"react-dom": "^17 || ^18 || ^19"
4344
},

packages/module/patternfly-docs/content/extensions/component-groups/examples/ColumnManagementModal/ColumnManagementModal.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,11 @@ Clicking the "Manage columns" button will open the column management modal. The
2929
```js file="./ColumnManagementModalExample.tsx"
3030

3131
```
32+
33+
### With drag and drop reordering
34+
35+
When `enableDragDrop` is set to `true`, users can drag and drop columns to reorder them. The order changes are reflected both in the modal and in the table when applied. This is useful when column order matters for the user experience.
36+
37+
```js file="./ColumnManagementModalDragDropExample.tsx"
38+
39+
```
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
import { FunctionComponent, useState } from 'react';
2+
import { Button, ButtonVariant } from '@patternfly/react-core';
3+
import { Table, Tbody, Td, Th, Tr, Thead } from '@patternfly/react-table';
4+
import { ColumnsIcon } from '@patternfly/react-icons';
5+
import ColumnManagementModal, {
6+
ColumnManagementModalColumn
7+
} from '@patternfly/react-component-groups/dist/dynamic/ColumnManagementModal';
8+
9+
const DEFAULT_COLUMNS: ColumnManagementModalColumn[] = [
10+
{
11+
title: 'ID',
12+
key: 'id',
13+
isShownByDefault: true,
14+
isShown: true,
15+
isUntoggleable: true
16+
},
17+
{
18+
title: 'Publish date',
19+
key: 'publishDate',
20+
isShownByDefault: true,
21+
isShown: true
22+
},
23+
{
24+
title: 'Impact',
25+
key: 'impact',
26+
isShownByDefault: true,
27+
isShown: true
28+
},
29+
{
30+
title: 'Score',
31+
key: 'score',
32+
isShownByDefault: false,
33+
isShown: false
34+
},
35+
{
36+
title: 'CVSS Vector',
37+
key: 'cvssVector',
38+
isShownByDefault: false,
39+
isShown: false
40+
},
41+
{
42+
title: 'Severity',
43+
key: 'severity',
44+
isShownByDefault: true,
45+
isShown: true
46+
}
47+
];
48+
49+
const ROWS = [
50+
{
51+
id: 'CVE-2024-1546',
52+
publishDate: '20 Feb 2024',
53+
impact: 'Important',
54+
score: '7.5',
55+
cvssVector: 'CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N',
56+
severity: 'High'
57+
},
58+
{
59+
id: 'CVE-2024-1547',
60+
publishDate: '20 Feb 2024',
61+
impact: 'Important',
62+
score: '7.5',
63+
cvssVector: 'CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N',
64+
severity: 'High'
65+
},
66+
{
67+
id: 'CVE-2024-1548',
68+
publishDate: '20 Feb 2024',
69+
impact: 'Moderate',
70+
score: '6.1',
71+
cvssVector: 'CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:C/C:L/I:L/A:N',
72+
severity: 'Medium'
73+
},
74+
{
75+
id: 'CVE-2024-1549',
76+
publishDate: '20 Feb 2024',
77+
impact: 'Moderate',
78+
score: '6.1',
79+
cvssVector: 'CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:C/C:L/I:L/A:N',
80+
severity: 'Medium'
81+
}
82+
];
83+
84+
export const ColumnManagementModalDragDropExample: FunctionComponent = () => {
85+
const [ columns, setColumns ] = useState(DEFAULT_COLUMNS);
86+
const [ isOpen, setOpen ] = useState(false);
87+
88+
return (
89+
<>
90+
<ColumnManagementModal
91+
appliedColumns={columns}
92+
applyColumns={(newColumns) => setColumns(newColumns)}
93+
isOpen={isOpen}
94+
onClose={() => setOpen(false)}
95+
enableDragDrop={true}
96+
title="Manage and reorder columns"
97+
description="Selected categories will be displayed in the table. Drag and drop to reorder columns."
98+
/>
99+
<Button
100+
className="pf-v6-u-mb-sm"
101+
onClick={() => setOpen(true)}
102+
variant={ButtonVariant.secondary}
103+
icon={<ColumnsIcon />}
104+
>
105+
Manage columns
106+
</Button>
107+
<Table aria-label="Simple table with reorderable columns" variant="compact">
108+
<Thead>
109+
<Tr>
110+
{columns
111+
.filter((column) => column.isShown)
112+
.map((column) => (
113+
<Th key={column.key}>{column.title}</Th>
114+
))}
115+
</Tr>
116+
</Thead>
117+
<Tbody>
118+
{ROWS.map((row, rowIndex) => (
119+
<Tr key={rowIndex}>
120+
{columns
121+
.filter((column) => column.isShown)
122+
.map((column, columnIndex) => (
123+
<Td key={columnIndex}>{row[column.key]}</Td>
124+
))}
125+
</Tr>
126+
))}
127+
</Tbody>
128+
</Table>
129+
</>
130+
);
131+
};

packages/module/src/ColumnManagementModal/ColumnManagementModal.test.tsx

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import '@testing-library/jest-dom'
22
import { render, screen, fireEvent } from '@testing-library/react';
3+
import userEvent from '@testing-library/user-event';
34
import ColumnManagementModal, { ColumnManagementModalColumn } from './ColumnManagementModal';
45

56
const DEFAULT_COLUMNS : ColumnManagementModalColumn[] = [
@@ -33,18 +34,29 @@ const DEFAULT_COLUMNS : ColumnManagementModalColumn[] = [
3334
const onClose = jest.fn();
3435
const setColumns = jest.fn();
3536

37+
// Simple mock to track when DragDropSort is used
38+
jest.mock('@patternfly/react-drag-drop', () => ({
39+
DragDropSort: ({ children }) => <div data-testid="drag-drop-sort">{children}</div>,
40+
Droppable: ({ wrapper }) => wrapper,
41+
}));
42+
43+
const renderColumnManagementModal = (props = {}) => render(<ColumnManagementModal
44+
appliedColumns={DEFAULT_COLUMNS}
45+
applyColumns={newColumns => setColumns(newColumns)}
46+
isOpen
47+
onClose={onClose}
48+
data-testid="column-mgmt-modal"
49+
{...props}
50+
/>);
51+
3652
beforeEach(() => {
37-
render(<ColumnManagementModal
38-
appliedColumns={DEFAULT_COLUMNS}
39-
applyColumns={newColumns => setColumns(newColumns)}
40-
isOpen
41-
onClose={onClose}
42-
data-testid="column-mgmt-modal"
43-
/>);
53+
jest.clearAllMocks();
54+
renderColumnManagementModal();
4455
});
4556

4657
const getCheckboxesState = () => {
47-
const checkboxes = screen.getByTestId('column-mgmt-modal').querySelectorAll('input[type="checkbox"]');
58+
// Get only the column checkboxes (exclude the BulkSelect checkbox)
59+
const checkboxes = screen.getByTestId('column-mgmt-modal').querySelectorAll('input[type="checkbox"][data-testid^="column-check-"]');
4860
return (Array.from(checkboxes) as HTMLInputElement[]).map(c => c.checked);
4961
}
5062

@@ -54,7 +66,7 @@ describe('ColumnManagementModal component', () => {
5466
});
5567

5668
it('should have checkbox checked if column is shown by default', () => {
57-
const idCheckbox = screen.getByTestId('column-mgmt-modal').querySelector('input[type="checkbox"][data-ouia-component-id="ColumnManagementModal-column0-checkbox"]');
69+
const idCheckbox = screen.getByTestId('column-mgmt-modal').querySelector('input[type="checkbox"][data-testid="column-check-id"]');
5870

5971
expect(idCheckbox).toHaveAttribute('disabled');
6072
expect(idCheckbox).toHaveAttribute('checked');
@@ -72,11 +84,15 @@ describe('ColumnManagementModal component', () => {
7284
expect(getCheckboxesState()).toEqual(DEFAULT_COLUMNS.map(c => c.isShownByDefault));
7385
});
7486

75-
it('should set all columns to show upon clicking on "Select all"', () => {
87+
it('should set all columns to show upon clicking on "Select all"', async () => {
7688
// disable Impact column which is enabled by default
7789
fireEvent.click(screen.getByText('Impact'));
7890

79-
fireEvent.click(screen.getByText('Select all'));
91+
// Use the BulkSelect to select all
92+
const menuToggle = screen.getByLabelText('Bulk select toggle');
93+
await userEvent.click(menuToggle);
94+
const selectAllButton = screen.getByText('Select all (4)');
95+
await userEvent.click(selectAllButton);
8096

8197
expect(getCheckboxesState()).toEqual(DEFAULT_COLUMNS.map(_ => true));
8298
});
@@ -103,6 +119,23 @@ describe('ColumnManagementModal component', () => {
103119
fireEvent.click(screen.getByText('Cancel'));
104120

105121
expect(onClose).toHaveBeenCalled();
106-
expect(setColumns).toHaveBeenCalledWith(DEFAULT_COLUMNS);
122+
// applyColumns should NOT be called on cancel
123+
expect(setColumns).not.toHaveBeenCalled();
124+
});
125+
126+
describe('enableDragDrop prop', () => {
127+
it('should default enableDragDrop to false', () => {
128+
// Default behavior should not enable drag and drop
129+
expect(screen.queryByTestId('drag-drop-sort')).not.toBeInTheDocument();
130+
});
131+
132+
it('should pass enableDragDrop prop to ListManager', () => {
133+
// Test that the prop is properly passed through to ListManager
134+
jest.clearAllMocks();
135+
renderColumnManagementModal({ enableDragDrop: true });
136+
137+
// When enableDragDrop is true, DragDropSort should be rendered
138+
expect(screen.getByTestId('drag-drop-sort')).toBeInTheDocument();
139+
});
107140
});
108141
});

0 commit comments

Comments
 (0)