Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discover] [Unified Data Table] Improve absolute column width handling #190288

Merged
merged 15 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from 13 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
4 changes: 2 additions & 2 deletions packages/kbn-unified-data-table/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Props description:
| **dataView** | DataView | The used data view. |
| **loadingState** | DataLoadingState | Determines if data is currently loaded. |
| **onFilter** | DocViewFilterFn | Function to add a filter in the grid cell or document flyout. |
| **onResize** | (optional)(colSettings: { columnId: string; width: number }) => void; | Function triggered when a column is resized by the user. |
| **onResize** | (optional)(colSettings: { columnId: string; width: number | undefind }) => void; | Function triggered when a column is resized by the user, passes `undefined` for auto-width. |
| **onSetColumns** | (columns: string[], hideTimeColumn: boolean) => void; | Function to set all columns. |
| **onSort** | (optional)(sort: string[][]) => void; | Function to change sorting of the documents, skipped when isSortEnabled is set to false. |
| **rows** | (optional)DataTableRecord[] | Array of documents provided by Elasticsearch. |
Expand Down Expand Up @@ -80,7 +80,7 @@ Usage example:
onFilter={() => {
// Add logic to refetch the data when the filter by field was added/removed. Refetch data.
}}
onResize={(colSettings: { columnId: string; width: number }) => {
onResize={(colSettings: { columnId: string; width: number | undefined }) => {
// Update the table state with the new width for the column
}}
onSetColumns={(columns: string[], hideTimeColumn: boolean) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-unified-data-table/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export * as columnActions from './src/components/actions/columns';
export { getRowsPerPageOptions } from './src/utils/rows_per_page';
export { popularizeField } from './src/utils/popularize_field';

export { useColumns } from './src/hooks/use_data_grid_columns';
export { useColumns, type UseColumnsProps } from './src/hooks/use_data_grid_columns';
export { OPEN_DETAILS, SELECT_ROW } from './src/components/data_table_columns'; // TODO: deprecate?
export { DataTableRowControl } from './src/components/data_table_row_control';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import { getStateColumnActions } from './columns';
import { dataViewMock } from '@kbn/discover-utils/src/__mocks__';
import { Capabilities } from '@kbn/core/types';
import { dataViewsMock } from '../../../__mocks__/data_views';
import { UnifiedDataTableSettings } from '../../types';

function getStateColumnAction(
state: { columns?: string[]; sort?: string[][] },
state: { columns?: string[]; sort?: string[][]; settings?: UnifiedDataTableSettings },
setAppState: (state: { columns: string[]; sort?: string[][] }) => void
) {
return getStateColumnActions({
Expand All @@ -28,6 +29,7 @@ function getStateColumnAction(
columns: state.columns,
sort: state.sort,
defaultOrder: 'desc',
settings: state.settings,
});
}

Expand All @@ -41,6 +43,7 @@ describe('Test column actions', () => {
actions.onAddColumn('test');
expect(setAppState).toHaveBeenCalledWith({ columns: ['test'] });
});

test('getStateColumnActions with columns and sort in state', () => {
const setAppState = jest.fn();
const actions = getStateColumnAction(
Expand Down Expand Up @@ -77,4 +80,95 @@ describe('Test column actions', () => {
columns: ['second', 'first'],
});
});

it('should pass settings to setAppState', () => {
const setAppState = jest.fn();
const settings: UnifiedDataTableSettings = { columns: { first: { width: 100 } } };
const actions = getStateColumnAction({ columns: ['first'], settings }, setAppState);
actions.onAddColumn('second');
expect(setAppState).toHaveBeenCalledWith({ columns: ['first', 'second'], settings });
setAppState.mockClear();
actions.onRemoveColumn('second');
expect(setAppState).toHaveBeenCalledWith({ columns: ['first'], settings, sort: [] });
setAppState.mockClear();
actions.onMoveColumn('first', 0);
expect(setAppState).toHaveBeenCalledWith({ columns: ['first'], settings });
setAppState.mockClear();
actions.onSetColumns(['first', 'second'], true);
expect(setAppState).toHaveBeenCalledWith({ columns: ['first', 'second'], settings });
setAppState.mockClear();
});

it('should clean up settings to remove non-existing columns', () => {
const setAppState = jest.fn();
const actions = getStateColumnAction(
{
columns: ['first', 'second', 'third'],
settings: { columns: { first: { width: 100 }, second: { width: 200 } } },
},
setAppState
);
actions.onRemoveColumn('second');
expect(setAppState).toHaveBeenCalledWith({
columns: ['first', 'third'],
settings: { columns: { first: { width: 100 } } },
sort: [],
});
setAppState.mockClear();
actions.onSetColumns(['first', 'third'], true);
expect(setAppState).toHaveBeenCalledWith({
columns: ['first', 'third'],
settings: { columns: { first: { width: 100 } } },
});
});

it('should reset the last column to auto width if only absolute width columns remain', () => {
const setAppState = jest.fn();
let actions = getStateColumnAction(
{
columns: ['first', 'second', 'third'],
settings: { columns: { second: { width: 100 }, third: { width: 100, display: 'test' } } },
},
setAppState
);
actions.onRemoveColumn('first');
expect(setAppState).toHaveBeenCalledWith({
columns: ['second', 'third'],
settings: { columns: { second: { width: 100 }, third: { display: 'test' } } },
sort: [],
});
setAppState.mockClear();
actions = getStateColumnAction(
{
columns: ['first', 'second', 'third'],
settings: { columns: { second: { width: 100 }, third: { width: 100 } } },
},
setAppState
);
actions.onSetColumns(['second', 'third'], true);
expect(setAppState).toHaveBeenCalledWith({
columns: ['second', 'third'],
settings: { columns: { second: { width: 100 } } },
});
});

it('should not reset the last column to auto width if there are remaining auto width columns', () => {
const setAppState = jest.fn();
const actions = getStateColumnAction(
{ columns: ['first', 'second', 'third'], settings: { columns: { third: { width: 100 } } } },
setAppState
);
actions.onRemoveColumn('first');
expect(setAppState).toHaveBeenCalledWith({
columns: ['second', 'third'],
settings: { columns: { third: { width: 100 } } },
sort: [],
});
setAppState.mockClear();
actions.onSetColumns(['second', 'third'], true);
expect(setAppState).toHaveBeenCalledWith({
columns: ['second', 'third'],
settings: { columns: { third: { width: 100 } } },
});
});
});
172 changes: 124 additions & 48 deletions packages/kbn-unified-data-table/src/components/actions/columns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,52 +5,13 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { Capabilities } from '@kbn/core/public';

import type { Capabilities } from '@kbn/core/public';
import type { DataViewsContract } from '@kbn/data-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
import { omit } from 'lodash';
import { popularizeField } from '../../utils/popularize_field';

/**
* Helper function to provide a fallback to a single _source column if the given array of columns
* is empty, and removes _source if there are more than 1 columns given
* @param columns
* @param useNewFieldsApi should a new fields API be used
*/
function buildColumns(columns: string[], useNewFieldsApi = false) {
if (columns.length > 1 && columns.indexOf('_source') !== -1) {
return columns.filter((col) => col !== '_source');
} else if (columns.length !== 0) {
return columns;
}
return useNewFieldsApi ? [] : ['_source'];
}

export function addColumn(columns: string[], columnName: string, useNewFieldsApi?: boolean) {
if (columns.includes(columnName)) {
return columns;
}
return buildColumns([...columns, columnName], useNewFieldsApi);
}

export function removeColumn(columns: string[], columnName: string, useNewFieldsApi?: boolean) {
if (!columns.includes(columnName)) {
return columns;
}
return buildColumns(
columns.filter((col) => col !== columnName),
useNewFieldsApi
);
}

export function moveColumn(columns: string[], columnName: string, newIndex: number) {
if (newIndex < 0 || newIndex >= columns.length || !columns.includes(columnName)) {
return columns;
}
const modifiedColumns = [...columns];
modifiedColumns.splice(modifiedColumns.indexOf(columnName), 1); // remove at old index
modifiedColumns.splice(newIndex, 0, columnName); // insert before new index
return modifiedColumns;
}
import type { UnifiedDataTableSettings } from '../../types';

export function getStateColumnActions({
capabilities,
Expand All @@ -61,34 +22,50 @@ export function getStateColumnActions({
columns,
sort,
defaultOrder,
settings,
}: {
capabilities: Capabilities;
dataView: DataView;
dataViews: DataViewsContract;
useNewFieldsApi: boolean;
setAppState: (state: { columns: string[]; sort?: string[][] }) => void;
setAppState: (state: {
columns: string[];
sort?: string[][];
settings?: UnifiedDataTableSettings;
}) => void;
columns?: string[];
sort: string[][] | undefined;
defaultOrder: string;
settings?: UnifiedDataTableSettings;
}) {
function onAddColumn(columnName: string) {
popularizeField(dataView, columnName, dataViews, capabilities);
const nextColumns = addColumn(columns || [], columnName, useNewFieldsApi);
const nextSort = columnName === '_score' && !sort?.length ? [['_score', defaultOrder]] : sort;
setAppState({ columns: nextColumns, sort: nextSort });
setAppState({ columns: nextColumns, sort: nextSort, settings });
}

function onRemoveColumn(columnName: string) {
popularizeField(dataView, columnName, dataViews, capabilities);

const nextColumns = removeColumn(columns || [], columnName, useNewFieldsApi);
// The state's sort property is an array of [sortByColumn,sortDirection]
const nextSort = sort && sort.length ? sort.filter((subArr) => subArr[0] !== columnName) : [];
setAppState({ columns: nextColumns, sort: nextSort });

let nextSettings = cleanColumnSettings(nextColumns, settings);

// When columns are removed, reset the last column to auto width if only absolute
// width columns remain, to ensure the columns fill the available grid space
if (nextColumns.length < (columns?.length ?? 0)) {
nextSettings = adjustLastColumnWidth(nextColumns, nextSettings);
}

setAppState({ columns: nextColumns, sort: nextSort, settings: nextSettings });
}

function onMoveColumn(columnName: string, newIndex: number) {
const nextColumns = moveColumn(columns || [], columnName, newIndex);
setAppState({ columns: nextColumns });
setAppState({ columns: nextColumns, settings });
}

function onSetColumns(nextColumns: string[], hideTimeColumn: boolean) {
Expand All @@ -98,7 +75,15 @@ export function getStateColumnActions({
? (nextColumns || []).slice(1)
: nextColumns;

setAppState({ columns: actualColumns });
let nextSettings = cleanColumnSettings(nextColumns, settings);

// When columns are removed, reset the last column to auto width if only absolute
// width columns remain, to ensure the columns fill the available grid space
if (actualColumns.length < (columns?.length ?? 0)) {
nextSettings = adjustLastColumnWidth(actualColumns, nextSettings);
}

setAppState({ columns: actualColumns, settings: nextSettings });
}
return {
onAddColumn,
Expand All @@ -107,3 +92,94 @@ export function getStateColumnActions({
onSetColumns,
};
}

/**
* Helper function to provide a fallback to a single _source column if the given array of columns
* is empty, and removes _source if there are more than 1 columns given
* @param columns
* @param useNewFieldsApi should a new fields API be used
*/
function buildColumns(columns: string[], useNewFieldsApi = false) {
if (columns.length > 1 && columns.indexOf('_source') !== -1) {
return columns.filter((col) => col !== '_source');
} else if (columns.length !== 0) {
return columns;
}
return useNewFieldsApi ? [] : ['_source'];
}

function addColumn(columns: string[], columnName: string, useNewFieldsApi?: boolean) {
if (columns.includes(columnName)) {
return columns;
}
return buildColumns([...columns, columnName], useNewFieldsApi);
}

function removeColumn(columns: string[], columnName: string, useNewFieldsApi?: boolean) {
if (!columns.includes(columnName)) {
return columns;
}
return buildColumns(
columns.filter((col) => col !== columnName),
useNewFieldsApi
);
}

function moveColumn(columns: string[], columnName: string, newIndex: number) {
if (newIndex < 0 || newIndex >= columns.length || !columns.includes(columnName)) {
return columns;
}
const modifiedColumns = [...columns];
modifiedColumns.splice(modifiedColumns.indexOf(columnName), 1); // remove at old index
modifiedColumns.splice(newIndex, 0, columnName); // insert before new index
return modifiedColumns;
}

function cleanColumnSettings(
columns: string[],
settings?: UnifiedDataTableSettings
): UnifiedDataTableSettings | undefined {
const columnSettings = settings?.columns;

if (!columnSettings) {
return settings;
}

const nextColumnSettings = columns.reduce<NonNullable<UnifiedDataTableSettings['columns']>>(
(acc, column) => (columnSettings[column] ? { ...acc, [column]: columnSettings[column] } : acc),
{}
);

return { ...settings, columns: nextColumnSettings };
}

function adjustLastColumnWidth(
columns: string[],
settings?: UnifiedDataTableSettings
): UnifiedDataTableSettings | undefined {
const columnSettings = settings?.columns;

if (!columns.length || !columnSettings) {
return settings;
}

const hasAutoWidthColumn = columns.some((colId) => columnSettings[colId]?.width == null);

if (hasAutoWidthColumn) {
return settings;
}

const lastColumn = columns[columns.length - 1];
const lastColumnSettings = omit(columnSettings[lastColumn] ?? {}, 'width');
const lastColumnSettingsOptional = Object.keys(lastColumnSettings).length
? { [lastColumn]: lastColumnSettings }
: undefined;

return {
...settings,
columns: {
...omit(columnSettings, lastColumn),
...lastColumnSettingsOptional,
},
};
}
Loading