Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions packages/eui/changelogs/upcoming/9209.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
**Bug fixes**

- Fixed unexpected duplicate columns in `EuiDataGrid` crashing the column sorting by removing duplicate columns entirely

Original file line number Diff line number Diff line change
Expand Up @@ -64,22 +64,45 @@ export const useDataGridColumnSelector = (

const [sortedColumns, setSortedColumns] = useDependentState(() => {
const availableColumnIds = availableColumns.map(({ id }) => id);
const availableSet = new Set(availableColumnIds);
// Filter visibleColumns to only include existing columns
// remove duplicate columns to ensure unique columns
const availableColumnIdsSet = new Set(availableColumnIds);

if (process.env.NODE_ENV === 'development') {
if (availableColumnIds.length > availableColumnIdsSet.size) {
const duplicateIds: string[] = [];

for (const id of availableColumnIds) {
if (
!duplicateIds.includes(id) &&
availableColumnIds.filter((_id) => _id === id).length > 1
) {
duplicateIds.push(id);
}
}

console.warn(
`⚠️ EuiDataGrid: Duplicate column IDs detected and removed: ${duplicateIds.join(
', '
)}.`,
'\n Column IDs must be unique. Only the first occurrence of each duplicate will be used.'
);
}
}

const validVisibleColumns = visibleColumns.filter((id) =>
availableSet.has(id)
availableColumnIdsSet.has(id)
);
const visibleSet = new Set(validVisibleColumns);

const result: string[] = [];
let visibleIndex = 0;

for (const columnId of availableColumnIds) {
for (const columnId of [...availableColumnIdsSet]) {
if (visibleSet.has(columnId)) {
// Replace with next visible column in order
result.push(validVisibleColumns[visibleIndex++]);
if (visibleIndex < validVisibleColumns.length) {
result.push(validVisibleColumns[visibleIndex++]);
}
} else {
// Keep hidden column in original position
result.push(columnId);
}
}
Expand Down Expand Up @@ -170,8 +193,8 @@ export const useDataGridColumnSelector = (
<EuiDataGridToolbarControl
badgeContent={
numberOfHiddenFields > 0
? `${orderedVisibleColumns.length}/${availableColumns.length}`
: availableColumns.length
? `${orderedVisibleColumns.length}/${sortedColumns.length}`
: sortedColumns.length
}
iconType="tableDensityNormal"
data-test-subj="dataGridColumnSelectorButton"
Expand Down
53 changes: 52 additions & 1 deletion packages/eui/src/components/datagrid/data_grid.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ a, footer\tb, footer
).should('have.attr', 'aria-checked', 'false');
});

it('renders columns in the corect order when `columns` and `visibleColumns` have different orders', () => {
it('renders columns in the correct order when `columns` and `visibleColumns` have different orders', () => {
cy.mount(
<StatefulDataGrid
{...props}
Expand Down Expand Up @@ -839,6 +839,57 @@ a, footer\tb, footer
).should('have.attr', 'aria-checked', 'false');
});

it('removes duplicates from `columns` with equal `id`', () => {
cy.mount(
<StatefulDataGrid
{...props}
columns={[
{ id: 'a', display: 'First' },
{ id: 'b', display: 'Second.A' },
{ id: 'b', display: 'Second.B' },
{ id: 'c', display: 'Third' },
{ id: 'd', display: 'Fourth' },
]}
columnVisibility={{
...props.columnVisibility,
visibleColumns: ['b', 'd', 'a'],
}}
/>
);

cy.get(
'[data-gridcell-column-index="0"][data-gridcell-visible-row-index="-1"] .euiDataGridHeaderCell__content'
).should('have.text', 'Second.A');
cy.get(
'[data-gridcell-column-index="1"][data-gridcell-visible-row-index="-1"] .euiDataGridHeaderCell__content'
).should('have.text', 'Fourth');
cy.get(
'[data-gridcell-column-index="2"][data-gridcell-visible-row-index="-1"] .euiDataGridHeaderCell__content'
).should('have.text', 'First');

cy.get('.euiNotificationBadge').then(($items) => {
const textContent = $items.text();
expect(textContent).to.equal('3/4');
});

/* checks column selector popover order */
cy.get('[data-test-subj="dataGridColumnSelectorButton"]').click();

cy.get('.euiDataGridColumnSelector__item').then(($items) => {
const columnsCount = $items.length;
expect(columnsCount).to.equal(4);
});

cy.get('.euiDataGridColumnSelector__item').eq(0).should('have.text', 'b');
cy.get('.euiDataGridColumnSelector__item').eq(1).should('have.text', 'd');
cy.get('.euiDataGridColumnSelector__item').eq(2).should('have.text', 'c');
cy.get('.euiDataGridColumnSelector__item').eq(3).should('have.text', 'a');

cy.get(
'.euiDataGridColumnSelector__item [data-test-subj="dataGridColumnSelectorToggleColumnVisibility-c"]'
).should('have.attr', 'aria-checked', 'false');
});

it('reorders columns from the column selector', () => {
cy.mount(<StatefulDataGrid {...props} />);

Expand Down
34 changes: 31 additions & 3 deletions packages/eui/src/components/datagrid/data_grid.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import React, { useRef, useEffect } from 'react';
import { css } from '@emotion/react';
import type { Meta, StoryObj, ReactRenderer } from '@storybook/react';
import type { PlayFunctionContext } from '@storybook/csf';
import { StoryContext } from '@storybook/csf';
import { expect, fireEvent, waitFor } from '@storybook/test';
import { action } from '@storybook/addon-actions';
import { within } from '../../../.storybook/test';
Expand Down Expand Up @@ -176,7 +176,7 @@ export const DraggableColumns: Story = {
),
},
render: (args: EuiDataGridProps) => <StatefulDataGrid {...args} />,
play: async ({ canvasElement }: PlayFunctionContext<ReactRenderer>) => {
play: async ({ canvasElement }: StoryContext<ReactRenderer>) => {
const canvas = within(canvasElement);

await waitFor(async () => {
Expand Down Expand Up @@ -231,9 +231,37 @@ export const ColumnActions: Story = {
gridStyle={{ fontSize: 's', cellPadding: 's' }}
/>
),
play: async ({ canvasElement }: PlayFunctionContext<ReactRenderer>) => {
play: async ({ canvasElement }: StoryContext<ReactRenderer>) => {
const canvas = within(canvasElement);
await canvas.waitForAndClick('dataGridHeaderCellActionButton-account');
await canvas.waitForEuiPopoverVisible();
},
};

export const VisibleColumns: Story = {
tags: ['vrt-only'],
args: {
...defaultStorybookArgs,
columnVisibility: {
// `visibleColumns` has a different order than `columns` - `visibleColumns` determines the order
visibleColumns: ['date', 'name', 'email', 'location'],
setVisibleColumns: action('setVisibleColumns'),
},
// `columns` contains duplicates - only the first occurrence is used
columns: [
{ id: 'name', displayAsText: 'Name' },
{ id: 'email', displayAsText: 'Email' },
{ id: 'location', displayAsText: 'Location' },
{ id: 'name', displayAsText: 'Name (duplicate)' },
{ id: 'email', displayAsText: 'Email (duplicate)' },
{ id: 'account', displayAsText: 'Account' },
{ id: 'date', displayAsText: 'Date' },
],
},
render: (args: EuiDataGridProps) => <StatefulDataGrid {...args} />,
play: async ({ canvasElement }: StoryContext<ReactRenderer>) => {
const canvas = within(canvasElement);
await canvas.waitForAndClick('dataGridColumnSelectorButton');
await canvas.waitForEuiPopoverVisible();
},
};
15 changes: 11 additions & 4 deletions packages/eui/src/components/datagrid/data_grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,17 @@ export const EuiDataGrid = memo(
*/
const displayValues: { [key: string]: string } = useMemo(() => {
return columns.reduce(
(acc: { [key: string]: string }, column: EuiDataGridColumn) => ({
...acc,
[column.id]: column.displayAsText || column.id,
}),
(acc: { [key: string]: string }, column: EuiDataGridColumn) => {
// prevent duplicate values
if (!acc[column.id]) {
return {
...acc,
[column.id]: column.displayAsText || column.id,
};
}

return acc;
},
{}
);
}, [columns]);
Expand Down
7 changes: 2 additions & 5 deletions packages/eui/src/components/datagrid/utils/in_memory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export const EuiDataGridInMemoryRenderer: FunctionComponent<
cells.push(
columns
.map((column, j) => {
const key = `${i}-${j}-${column.id}`;
const skipThisColumn =
inMemory.skipColumns &&
inMemory.skipColumns.indexOf(column.id) !== -1;
Expand All @@ -112,11 +113,7 @@ export const EuiDataGridInMemoryRenderer: FunctionComponent<
column.isExpandable !== undefined ? column.isExpandable : true;

return (
<div
key={`${i}-${column.id}`}
data-dg-row={i}
data-dg-column={column.id}
>
<div key={key} data-dg-row={i} data-dg-column={column.id}>
<CellElement
rowIndex={i}
colIndex={j}
Expand Down