[Discover] [Unified Data Table] Improve absolute column width handling (

#190288) ## Summary This PR improves the handling of columns with absolute widths in Discover, including the following enhancements: - If there are no auto width columns in the profile default app state, set the last column to auto width so the default columns always fill the grid. - If there are no auto width columns remaining when removing a column from the grid, set the last column to auto width so the remaining columns fill the grid. - Add a "Reset width" button to the column header popovers to allow resetting absolute width columns back to auto width. https://github.com/user-attachments/assets/0c588969-5720-40e3-91e2-07a83a93b797 Resolves #189817. Related #188550. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Julia Rechkunova <[email protected]>
elastic · Aug 20, 2024 · 349fdac · 349fdac
1 parent 5f19307
commit 349fdac
Show file tree

Hide file tree

Showing 29 changed files with 707 additions and 152 deletions.
diff --git a/packages/kbn-unified-data-table/README.md b/packages/kbn-unified-data-table/README.md
@@ -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. |
@@ -81,7 +81,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) => {

diff --git a/packages/kbn-unified-data-table/index.ts b/packages/kbn-unified-data-table/index.ts
@@ -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';
 

diff --git a/packages/kbn-unified-data-table/src/components/actions/columns.test.ts b/packages/kbn-unified-data-table/src/components/actions/columns.test.ts
@@ -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({
@@ -28,6 +29,7 @@ function getStateColumnAction(
     columns: state.columns,
     sort: state.sort,
     defaultOrder: 'desc',
+    settings: state.settings,
   });
 }
 
@@ -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(
@@ -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 } } },
+    });
+  });
 });
diff --git a/packages/kbn-unified-data-table/src/components/actions/columns.ts b/packages/kbn-unified-data-table/src/components/actions/columns.ts
@@ -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,
@@ -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) {
@@ -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,
@@ -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,
+    },
+  };
+}