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] Set data table row height to auto-fit by default #164218

Merged
merged 15 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from 10 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
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -32,7 +30,7 @@ describe('useRowHeightsOptions', () => {
storage: new LocalStorageMock({
['discover:dataGridRowHeight']: {
previousRowHeight: 5,
previousConfigRowHeight: 3,
previousConfigRowHeight: -1,
},
}) as unknown as Storage,
consumer: 'discover',
Expand All @@ -52,7 +50,7 @@ describe('useRowHeightsOptions', () => {
});

expect(result.current.defaultHeight).toEqual({
lineCount: CONFIG_ROW_HEIGHT,
lineCount: 3,
});
});

Expand All @@ -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');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ interface UseRowHeightProps {
*/
const SINGLE_ROW_HEIGHT_OPTION = 0;
const AUTO_ROW_HEIGHT_OPTION = -1;
const DEFAULT_ROW_HEIGHT_OPTION = 3;
const DEFAULT_ROW_HEIGHT_OPTION = AUTO_ROW_HEIGHT_OPTION;

/**
* Converts rowHeight of EuiDataGrid to rowHeight number (-1 to 20)
*/
const serializeRowHeight = (rowHeight?: EuiDataGridRowHeightOption): number => {
if (rowHeight === 'auto') {
if (rowHeight === 'auto' || rowHeight === AUTO_ROW_HEIGHT_OPTION) {
return AUTO_ROW_HEIGHT_OPTION;
} else if (typeof rowHeight === 'object' && rowHeight.lineCount) {
return rowHeight.lineCount; // custom
Expand Down Expand Up @@ -84,11 +84,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
);
kertal marked this conversation as resolved.
Show resolved Hide resolved
updateStoredRowHeight(newSerializedRowHeight, configRowHeight, storage, consumer);
onUpdateRowHeight?.(newSerializedRowHeight);
},
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/discover/server/ui_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions test/functional/apps/discover/classic/_doc_table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const defaultSettings = {
defaultIndex: 'logstash-*',
hideAnnouncements: true,
'discover:rowHeightOption': 0, // single line
kertal marked this conversation as resolved.
Show resolved Hide resolved
};
const testSubjects = getService('testSubjects');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
89 changes: 89 additions & 0 deletions test/functional/apps/discover/group2/_data_grid_row_height.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding these!

Original file line number Diff line number Diff line change
@@ -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');
});
});
}
1 change: 1 addition & 0 deletions test/functional/apps/discover/group2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down
21 changes: 21 additions & 0 deletions test/functional/services/data_grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down