diff --git a/packages/eui/.loki/reference/chrome_desktop_Tabular_Content_EuiDataGrid_Visible_Columns.png b/packages/eui/.loki/reference/chrome_desktop_Tabular_Content_EuiDataGrid_Visible_Columns.png new file mode 100644 index 00000000000..449826e2cc9 Binary files /dev/null and b/packages/eui/.loki/reference/chrome_desktop_Tabular_Content_EuiDataGrid_Visible_Columns.png differ diff --git a/packages/eui/.loki/reference/chrome_mobile_Tabular_Content_EuiDataGrid_Visible_Columns.png b/packages/eui/.loki/reference/chrome_mobile_Tabular_Content_EuiDataGrid_Visible_Columns.png new file mode 100644 index 00000000000..30a9077872d Binary files /dev/null and b/packages/eui/.loki/reference/chrome_mobile_Tabular_Content_EuiDataGrid_Visible_Columns.png differ diff --git a/packages/eui/changelogs/upcoming/9209.md b/packages/eui/changelogs/upcoming/9209.md new file mode 100644 index 00000000000..82afe2af595 --- /dev/null +++ b/packages/eui/changelogs/upcoming/9209.md @@ -0,0 +1,4 @@ +**Bug fixes** + +- Fixed unexpected duplicate columns in `EuiDataGrid` crashing the column sorting by removing duplicate columns entirely + diff --git a/packages/eui/src/components/datagrid/controls/column_selector.tsx b/packages/eui/src/components/datagrid/controls/column_selector.tsx index 5ff1bdfc568..51a59de176f 100644 --- a/packages/eui/src/components/datagrid/controls/column_selector.tsx +++ b/packages/eui/src/components/datagrid/controls/column_selector.tsx @@ -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); } } @@ -170,8 +193,8 @@ export const useDataGridColumnSelector = ( 0 - ? `${orderedVisibleColumns.length}/${availableColumns.length}` - : availableColumns.length + ? `${orderedVisibleColumns.length}/${sortedColumns.length}` + : sortedColumns.length } iconType="tableDensityNormal" data-test-subj="dataGridColumnSelectorButton" diff --git a/packages/eui/src/components/datagrid/data_grid.spec.tsx b/packages/eui/src/components/datagrid/data_grid.spec.tsx index 71399fbbabe..74c320d3b1a 100644 --- a/packages/eui/src/components/datagrid/data_grid.spec.tsx +++ b/packages/eui/src/components/datagrid/data_grid.spec.tsx @@ -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( { + cy.mount( + + ); + + 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(); diff --git a/packages/eui/src/components/datagrid/data_grid.stories.tsx b/packages/eui/src/components/datagrid/data_grid.stories.tsx index baf139384cb..cae8d23a61f 100644 --- a/packages/eui/src/components/datagrid/data_grid.stories.tsx +++ b/packages/eui/src/components/datagrid/data_grid.stories.tsx @@ -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'; @@ -176,7 +176,7 @@ export const DraggableColumns: Story = { ), }, render: (args: EuiDataGridProps) => , - play: async ({ canvasElement }: PlayFunctionContext) => { + play: async ({ canvasElement }: StoryContext) => { const canvas = within(canvasElement); await waitFor(async () => { @@ -231,9 +231,37 @@ export const ColumnActions: Story = { gridStyle={{ fontSize: 's', cellPadding: 's' }} /> ), - play: async ({ canvasElement }: PlayFunctionContext) => { + play: async ({ canvasElement }: StoryContext) => { 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) => , + play: async ({ canvasElement }: StoryContext) => { + const canvas = within(canvasElement); + await canvas.waitForAndClick('dataGridColumnSelectorButton'); + await canvas.waitForEuiPopoverVisible(); + }, +}; diff --git a/packages/eui/src/components/datagrid/data_grid.tsx b/packages/eui/src/components/datagrid/data_grid.tsx index 2976c630f68..abfecb53c2f 100644 --- a/packages/eui/src/components/datagrid/data_grid.tsx +++ b/packages/eui/src/components/datagrid/data_grid.tsx @@ -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]); diff --git a/packages/eui/src/components/datagrid/utils/in_memory.tsx b/packages/eui/src/components/datagrid/utils/in_memory.tsx index 33d840e0901..b5a4a5c082f 100644 --- a/packages/eui/src/components/datagrid/utils/in_memory.tsx +++ b/packages/eui/src/components/datagrid/utils/in_memory.tsx @@ -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; @@ -112,11 +113,7 @@ export const EuiDataGridInMemoryRenderer: FunctionComponent< column.isExpandable !== undefined ? column.isExpandable : true; return ( -
+