[Discover] Set data table row height to auto-fit by default (#164218)

- Closes #164285 This PR changes the default value of "rowHeight" setting to be "Auto-fit". <img width="600" alt="Screenshot 2023-08-17 at 19 46 03" src="https://github.com/elastic/kibana/assets/1415710/44cc2cc6-8bbd-41a9-b34c-94a189edbd7b"> When testing, make sure to delete "discover:dataGridRowHeight" from the browser localStorage, refresh the page and press "New" in Discover. Partially addresses #131130 (it can still hide "Reset" after page reload) --------- Co-authored-by: Kibana Machine <[email protected]>
elastic · Sep 12, 2023 · 0f07497 · 0f07497
1 parent b796f13
commit 0f07497
Show file tree

Hide file tree

Showing 11 changed files with 136 additions and 19 deletions.
diff --git a/packages/kbn-unified-data-table/src/constants.ts b/packages/kbn-unified-data-table/src/constants.ts
@@ -20,7 +20,7 @@ export const ROWS_PER_PAGE_OPTIONS = [10, 25, 50, DEFAULT_ROWS_PER_PAGE, 250, 50
 export const ROWS_HEIGHT_OPTIONS = {
   auto: -1,
   single: 0,
-  default: 3,
+  default: -1,
 };
 
 export const defaultMonacoEditorWidth = 370;

diff --git a/packages/kbn-unified-data-table/src/hooks/use_row_heights_options.test.tsx b/packages/kbn-unified-data-table/src/hooks/use_row_heights_options.test.tsx
@@ -11,8 +11,6 @@ import { Storage } from '@kbn/kibana-utils-plugin/public';
 import { LocalStorageMock } from '../../__mocks__/local_storage_mock';
 import { useRowHeightsOptions } from './use_row_heights_options';
 
-const CONFIG_ROW_HEIGHT = 3;
-
 describe('useRowHeightsOptions', () => {
   test('should apply rowHeight from savedSearch', () => {
     const { result } = renderHook(() => {
@@ -32,7 +30,7 @@ describe('useRowHeightsOptions', () => {
         storage: new LocalStorageMock({
           ['discover:dataGridRowHeight']: {
             previousRowHeight: 5,
-            previousConfigRowHeight: 3,
+            previousConfigRowHeight: -1,
           },
         }) as unknown as Storage,
         consumer: 'discover',
@@ -52,7 +50,7 @@ describe('useRowHeightsOptions', () => {
     });
 
     expect(result.current.defaultHeight).toEqual({
-      lineCount: CONFIG_ROW_HEIGHT,
+      lineCount: 3,
     });
   });
 
@@ -61,17 +59,15 @@ describe('useRowHeightsOptions', () => {
       return useRowHeightsOptions({
         storage: new LocalStorageMock({
           ['discover:dataGridRowHeight']: {
-            previousRowHeight: 4,
-            // different from uiSettings (config), now user changed it to 3, but prev was 4
+            previousRowHeight: 5,
+            // different from uiSettings (config), now user changed it to -1, but prev was 4
             previousConfigRowHeight: 4,
           },
         }) as unknown as Storage,
         consumer: 'discover',
       });
     });
 
-    expect(result.current.defaultHeight).toEqual({
-      lineCount: CONFIG_ROW_HEIGHT,
-    });
+    expect(result.current.defaultHeight).toEqual('auto');
   });
 });
diff --git a/packages/kbn-unified-data-table/src/hooks/use_row_heights_options.ts b/packages/kbn-unified-data-table/src/hooks/use_row_heights_options.ts
@@ -29,7 +29,7 @@ interface UseRowHeightProps {
  * Converts rowHeight of EuiDataGrid to rowHeight number (-1 to 20)
  */
 const serializeRowHeight = (rowHeight?: EuiDataGridRowHeightOption): number => {
-  if (rowHeight === 'auto') {
+  if (rowHeight === 'auto' || rowHeight === ROWS_HEIGHT_OPTIONS.auto) {
     return ROWS_HEIGHT_OPTIONS.auto;
   } else if (typeof rowHeight === 'object' && rowHeight.lineCount) {
     return rowHeight.lineCount; // custom
@@ -75,11 +75,16 @@ export const useRowHeightsOptions = ({
       rowHeight = configRowHeight;
     }
 
+    const defaultHeight = deserializeRowHeight(rowHeight);
+
     return {
-      defaultHeight: deserializeRowHeight(rowHeight),
+      defaultHeight,
       lineHeight: '1.6em',
       onChange: ({ defaultHeight: newRowHeight }: EuiDataGridRowHeightsOptions) => {
-        const newSerializedRowHeight = serializeRowHeight(newRowHeight);
+        const newSerializedRowHeight = serializeRowHeight(
+          // pressing "Reset to default" triggers onChange with the same value
+          newRowHeight === defaultHeight ? configRowHeight : newRowHeight
+        );
         updateStoredRowHeight(newSerializedRowHeight, configRowHeight, storage, consumer);
         onUpdateRowHeight?.(newSerializedRowHeight);
       },

diff --git a/src/plugins/discover/server/ui_settings.ts b/src/plugins/discover/server/ui_settings.ts
@@ -287,7 +287,7 @@ export const getUiSettings: (docLinks: DocLinksServiceSetup) => Record<string, U
     name: i18n.translate('discover.advancedSettings.params.rowHeightTitle', {
       defaultMessage: 'Row height in the Document Explorer',
     }),
-    value: 3,
+    value: -1,
     category: ['discover'],
     description: i18n.translate('discover.advancedSettings.params.rowHeightText', {
       defaultMessage:

diff --git a/test/functional/apps/discover/classic/_doc_table.ts b/test/functional/apps/discover/classic/_doc_table.ts
@@ -28,6 +28,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
   const defaultSettings = {
     defaultIndex: 'logstash-*',
     hideAnnouncements: true,
+    'doc_table:legacy': true,
   };
   const testSubjects = getService('testSubjects');
 
@@ -75,7 +76,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
 
     describe('classic table in window 900x700', async function () {
       before(async () => {
-        await kibanaServer.uiSettings.update({ 'doc_table:legacy': true });
         await browser.setWindowSize(900, 700);
         await PageObjects.common.navigateToApp('discover');
         await PageObjects.discover.waitUntilSearchingHasFinished();
@@ -95,7 +95,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
 
     describe('classic table in window 600x700', async function () {
       before(async () => {
-        await kibanaServer.uiSettings.update({ 'doc_table:legacy': true });
         await browser.setWindowSize(600, 700);
         await PageObjects.common.navigateToApp('discover');
         await PageObjects.discover.waitUntilSearchingHasFinished();
@@ -115,7 +114,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
 
     describe('legacy', async function () {
       before(async () => {
-        await kibanaServer.uiSettings.update({ 'doc_table:legacy': true });
         await PageObjects.common.navigateToApp('discover');
         await PageObjects.discover.waitUntilSearchingHasFinished();
       });

diff --git a/test/functional/apps/discover/group2/_data_grid_context.ts b/test/functional/apps/discover/group2/_data_grid_context.ts
@@ -30,7 +30,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
     'header',
     'unifiedFieldList',
   ]);
-  const defaultSettings = { defaultIndex: 'logstash-*' };
+  const defaultSettings = {
+    defaultIndex: 'logstash-*',
+    'discover:rowHeightOption': 0, // single line
+  };
   const kibanaServer = getService('kibanaServer');
   const esArchiver = getService('esArchiver');
   const dashboardAddPanel = getService('dashboardAddPanel');

diff --git a/test/functional/apps/discover/group2/_data_grid_doc_table.ts b/test/functional/apps/discover/group2/_data_grid_doc_table.ts
@@ -28,6 +28,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
   ]);
   const defaultSettings = {
     defaultIndex: 'logstash-*',
+    'discover:rowHeightOption': 0, // single line
   };
   const testSubjects = getService('testSubjects');
   const security = getService('security');

diff --git a/test/functional/apps/discover/group2/_data_grid_pagination.ts b/test/functional/apps/discover/group2/_data_grid_pagination.ts
@@ -15,7 +15,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
   const kibanaServer = getService('kibanaServer');
   const dataGrid = getService('dataGrid');
   const PageObjects = getPageObjects(['settings', 'common', 'discover', 'header', 'timePicker']);
-  const defaultSettings = { defaultIndex: 'logstash-*' };
+  const defaultSettings = {
+    defaultIndex: 'logstash-*',
+    'discover:rowHeightOption': 0, // single line
+  };
   const testSubjects = getService('testSubjects');
   const retry = getService('retry');
   const security = getService('security');

diff --git a/test/functional/apps/discover/group2/_data_grid_row_height.ts b/test/functional/apps/discover/group2/_data_grid_row_height.ts
@@ -0,0 +1,89 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import expect from '@kbn/expect';
+import { FtrProviderContext } from '../ftr_provider_context';
+
+export default function ({ getService, getPageObjects }: FtrProviderContext) {
+  const browser = getService('browser');
+  const esArchiver = getService('esArchiver');
+  const kibanaServer = getService('kibanaServer');
+  const dataGrid = getService('dataGrid');
+  const PageObjects = getPageObjects(['settings', 'common', 'discover', 'header', 'timePicker']);
+  const defaultSettings = { defaultIndex: 'logstash-*' };
+  const security = getService('security');
+
+  describe('discover data grid row height', function describeIndexTests() {
+    before(async () => {
+      await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']);
+      await browser.setWindowSize(1200, 2000);
+      await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
+      await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover');
+    });
+
+    after(async () => {
+      await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover');
+      await kibanaServer.uiSettings.replace({});
+      await kibanaServer.savedObjects.cleanStandardList();
+    });
+
+    beforeEach(async function () {
+      await PageObjects.timePicker.setDefaultAbsoluteRangeViaUiSettings();
+      await kibanaServer.uiSettings.update(defaultSettings);
+      await PageObjects.common.navigateToApp('discover');
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+    });
+
+    it('should use the default row height', async () => {
+      const rows = await dataGrid.getDocTableRows();
+      expect(rows.length).to.be.above(0);
+
+      await dataGrid.clickGridSettings();
+      expect(await dataGrid.getCurrentRowHeightValue()).to.be('Auto fit');
+    });
+
+    it('should allow to change row height and reset it', async () => {
+      await dataGrid.clickGridSettings();
+      expect(await dataGrid.getCurrentRowHeightValue()).to.be('Auto fit');
+
+      await dataGrid.changeRowHeightValue('Single');
+
+      // toggle the popover
+      await dataGrid.clickGridSettings();
+      await dataGrid.clickGridSettings();
+
+      expect(await dataGrid.getCurrentRowHeightValue()).to.be('Single');
+
+      await dataGrid.resetRowHeightValue();
+
+      expect(await dataGrid.getCurrentRowHeightValue()).to.be('Auto fit');
+
+      await dataGrid.changeRowHeightValue('Custom');
+
+      await dataGrid.resetRowHeightValue();
+
+      expect(await dataGrid.getCurrentRowHeightValue()).to.be('Auto fit');
+    });
+
+    it('should persist the selection after reloading the page', async () => {
+      await dataGrid.clickGridSettings();
+      expect(await dataGrid.getCurrentRowHeightValue()).to.be('Auto fit');
+
+      await dataGrid.changeRowHeightValue('Single');
+
+      expect(await dataGrid.getCurrentRowHeightValue()).to.be('Single');
+
+      await browser.refresh();
+
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      await dataGrid.clickGridSettings();
+
+      expect(await dataGrid.getCurrentRowHeightValue()).to.be('Single');
+    });
+  });
+}
diff --git a/test/functional/apps/discover/group2/index.ts b/test/functional/apps/discover/group2/index.ts
@@ -29,6 +29,7 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) {
     loadTestFile(require.resolve('./_data_grid_row_navigation'));
     loadTestFile(require.resolve('./_data_grid_doc_table'));
     loadTestFile(require.resolve('./_data_grid_copy_to_clipboard'));
+    loadTestFile(require.resolve('./_data_grid_row_height'));
     loadTestFile(require.resolve('./_data_grid_pagination'));
     loadTestFile(require.resolve('./_data_grid_footer'));
     loadTestFile(require.resolve('./_adhoc_data_views'));

diff --git a/test/functional/services/data_grid.ts b/test/functional/services/data_grid.ts
@@ -304,6 +304,27 @@ export class DataGridService extends FtrService {
     await this.testSubjects.click('gridEditFieldButton');
   }
 
+  public async clickGridSettings() {
+    await this.testSubjects.click('dataGridDisplaySelectorButton');
+  }
+
+  public async getCurrentRowHeightValue() {
+    const buttonGroup = await this.testSubjects.find('rowHeightButtonGroup');
+    return (
+      await buttonGroup.findByCssSelector('.euiButtonGroupButton-isSelected')
+    ).getVisibleText();
+  }
+
+  public async changeRowHeightValue(newValue: string) {
+    const buttonGroup = await this.testSubjects.find('rowHeightButtonGroup');
+    const option = await buttonGroup.findByCssSelector(`[data-text="${newValue}"]`);
+    await option.click();
+  }
+
+  public async resetRowHeightValue() {
+    await this.testSubjects.click('resetDisplaySelector');
+  }
+
   public async getDetailsRow(): Promise<WebElementWrapper> {
     const detailRows = await this.getDetailsRows();
     return detailRows[0];