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] Move truncate-by-height into Discover #114320

Merged
merged 38 commits into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
4a49e61
[Discover] move source field formatter to discover
dimaanj Oct 7, 2021
7024757
[Discover] cleanup source field from field formatters
dimaanj Oct 7, 2021
7714a4e
Merge branch 'master' into move-truncate-by-height
kibanamachine Oct 8, 2021
66c0f9d
[Discover] return source field format
dimaanj Oct 8, 2021
254d5ca
[Discover] move truncate by height to discover settings category, app…
dimaanj Oct 8, 2021
9cfeb8e
Merge branch 'master' into move-truncate-by-height
kibanamachine Oct 11, 2021
5a201c4
[Discover] improve code readability, fix i18n
dimaanj Oct 11, 2021
a610b67
[Discover] fix remaining i18n
dimaanj Oct 11, 2021
8b163ca
[Discover] fix unit tests
dimaanj Oct 11, 2021
57ff582
[Discover] return truncate-by-height setting to general
dimaanj Oct 12, 2021
2dabce6
[Discover] return i18n naming
dimaanj Oct 12, 2021
7a82ebc
Merge branch 'master' into move-truncate-by-height
kibanamachine Oct 12, 2021
a92d021
Merge branch 'master' into move-truncate-by-height
kibanamachine Oct 13, 2021
5f7d2f1
[Discover] apply suggestions
dimaanj Oct 13, 2021
7a9f271
[Discover] fix i18n
dimaanj Oct 13, 2021
262260b
Merge branch 'master' into move-truncate-by-height
kibanamachine Oct 14, 2021
a0e86e3
Update src/plugins/discover/server/ui_settings.ts
dimaanj Oct 14, 2021
8c49f95
[Discover] fix embeddable and discover grid truncation styles
dimaanj Oct 14, 2021
5337751
[Discover] fix tests
dimaanj Oct 15, 2021
03ef077
Merge branch 'master' into move-truncate-by-height
dimaanj Oct 15, 2021
b5069b0
[Discover] get rid of emotion
dimaanj Oct 15, 2021
855ab1b
Merge branch 'master' into move-truncate-by-height
kibanamachine Oct 18, 2021
f296163
Merge branch 'master' into move-truncate-by-height
kibanamachine Oct 18, 2021
010161f
Merge branch 'master' of https://github.com/elastic/kibana into move-…
dimaanj Oct 20, 2021
220ffc7
[Discover] apply suggestions
dimaanj Oct 20, 2021
71af85d
[Discover] inject emotion styles properly
dimaanj Oct 21, 2021
0d9493d
[Discover] remove console.log
dimaanj Oct 21, 2021
d3d4372
[Discover] revert react router changes
dimaanj Oct 21, 2021
7f6b878
[Discover] fix truncate max height reset
dimaanj Oct 21, 2021
ca1a41f
[Discover] simplify
dimaanj Oct 21, 2021
e2b6983
[Discover] return injection to the right place
dimaanj Oct 21, 2021
7b87342
[Discover] remove unused import
dimaanj Oct 21, 2021
09adf01
Merge branch 'master' into move-truncate-by-height
kibanamachine Oct 21, 2021
00a2345
Merge branch 'master' into move-truncate-by-height
dimaanj Oct 22, 2021
4bb8b12
Merge branch 'master' into move-truncate-by-height
kibanamachine Oct 24, 2021
1c5547d
[Discover] apply suggestions
dimaanj Oct 31, 2021
8e4fd8f
Merge branch 'main' into move-truncate-by-height
kibanamachine Oct 31, 2021
5a9c4e4
Merge branch 'main' into move-truncate-by-height
kibanamachine Nov 2, 2021
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
1 change: 1 addition & 0 deletions src/plugins/discover/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ export const MODIFY_COLUMNS_ON_SWITCH = 'discover:modifyColumnsOnSwitch';
export const SEARCH_FIELDS_FROM_SOURCE = 'discover:searchFieldsFromSource';
export const MAX_DOC_FIELDS_DISPLAYED = 'discover:maxDocFieldsDisplayed';
export const SHOW_MULTIFIELDS = 'discover:showMultiFields';
export const TRUNCATE_MAX_HEIGHT = 'truncate:maxHeight';
export const SEARCH_EMBEDDABLE_TYPE = 'search';
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@
text-align: center;
}

.truncate-by-height {
overflow: hidden;
}

.table {
// Nesting
.table {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import { indexPatternWithTimefieldMock } from '../../../../../../__mocks__/index
import { uiSettingsMock } from '../../../../../../__mocks__/ui_settings';
import { DocViewsRegistry } from '../../../../../doc_views/doc_views_registry';

jest.mock('../lib/row_formatter', () => {
const originalModule = jest.requireActual('../lib/row_formatter');
jest.mock('../lib/formatters/row_formatter', () => {
const originalModule = jest.requireActual('../lib/formatters/row_formatter');
return {
...originalModule,
formatRow: () => <span>mocked_document_cell</span>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import { ElasticSearchHit, DocViewFilterFn } from '../../../../../doc_views/doc_
import { getContextUrl } from '../../../../../helpers/get_context_url';
import { getSingleDocUrl } from '../../../../../helpers/get_single_doc_url';
import { TableRowDetails } from './table_row_details';
import { formatRow, formatTopLevelObject } from '../lib/row_formatter';
import { formatRow, formatTopLevelObject } from '../lib/formatters/row_formatter';
import { formatSource } from '../lib/formatters/source_formatter';

export type DocTableRow = ElasticSearchHit & {
isAnchor?: boolean;
Expand All @@ -32,6 +33,7 @@ export interface TableRowProps {
onRemoveColumn?: (column: string) => void;
useNewFieldsApi: boolean;
hideTimeColumn: boolean;
isShortDots: boolean;
filterManager: FilterManager;
addBasePath: (path: string) => string;
fieldsToShow: string[];
Expand All @@ -45,6 +47,7 @@ export const TableRow = ({
useNewFieldsApi,
fieldsToShow,
hideTimeColumn,
isShortDots,
onAddColumn,
onRemoveColumn,
filterManager,
Expand All @@ -63,18 +66,23 @@ export const TableRow = ({
// toggle display of the rows details, a full list of the fields from each row
const toggleRow = () => setOpen((prevOpen) => !prevOpen);

/**
* Fill an element with the value of a field
*/
const displayField = (fieldName: string) => {
const formatField = (fieldName: string) => {
if (fieldName === '_source') {
return formatSource({
hit: row,
indexPattern,
isShortDots,
});
}

const formattedField = indexPattern.formatField(row, fieldName);

// field formatters take care of escaping
// eslint-disable-next-line react/no-danger
const fieldElement = <span dangerouslySetInnerHTML={{ __html: formattedField }} />;

return <div className="truncate-by-height">{fieldElement}</div>;
const element = <span dangerouslySetInnerHTML={{ __html: formattedField }} />;
return <div className="truncate-by-height">{element}</div>;
};

const inlineFilter = useCallback(
(column: string, type: '+' | '-') => {
const field = indexPattern.fields.getByName(column);
Expand Down Expand Up @@ -116,7 +124,7 @@ export const TableRow = ({
<TableCell
key={indexPattern.timeFieldName}
timefield={true}
formatted={displayField(indexPattern.timeFieldName)}
formatted={formatField(indexPattern.timeFieldName)}
filterable={Boolean(mapping(indexPattern.timeFieldName)?.filterable && filter)}
column={indexPattern.timeFieldName}
inlineFilter={inlineFilter}
Expand Down Expand Up @@ -166,7 +174,7 @@ export const TableRow = ({
key={column}
timefield={false}
sourcefield={column === '_source'}
formatted={displayField(column)}
formatted={formatField(column)}
filterable={isFilterable}
column={column}
inlineFilter={inlineFilter}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import React, { forwardRef, useCallback, useMemo } from 'react';
import { EuiIcon, EuiSpacer, EuiText } from '@elastic/eui';
import type { IndexPattern, IndexPatternField } from 'src/plugins/data/common';
import { FormattedMessage } from '@kbn/i18n/react';
import { css, Global } from '@emotion/react';
import { TableHeader } from './components/table_header/table_header';
import { FORMATS_UI_SETTINGS } from '../../../../../../../field_formats/common';
import {
DOC_HIDE_TIME_COLUMN_SETTING,
SAMPLE_SIZE_SETTING,
SHOW_MULTIFIELDS,
SORT_DEFAULT_ORDER_SETTING,
TRUNCATE_MAX_HEIGHT,
} from '../../../../../../common';
import { getServices } from '../../../../../kibana_services';
import { SortOrder } from './components/table_header/helpers';
Expand Down Expand Up @@ -99,6 +101,8 @@ export interface DocTableWrapperProps extends DocTableProps {
render: (params: DocTableRenderProps) => JSX.Element;
}

const TRUNCATE_GRADIENT_HEIGHT = 15;

const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));

export const DocTableWrapper = forwardRef(
Expand Down Expand Up @@ -128,6 +132,7 @@ export const DocTableWrapper = forwardRef(
isShortDots,
sampleSize,
showMultiFields,
maxHeight,
filterManager,
addBasePath,
] = useMemo(() => {
Expand All @@ -138,6 +143,7 @@ export const DocTableWrapper = forwardRef(
services.uiSettings.get(FORMATS_UI_SETTINGS.SHORT_DOTS_ENABLE),
services.uiSettings.get(SAMPLE_SIZE_SETTING, 500),
services.uiSettings.get(SHOW_MULTIFIELDS, false),
services.uiSettings.get(TRUNCATE_MAX_HEIGHT),
services.filterManager,
services.addBasePath,
];
Expand Down Expand Up @@ -205,6 +211,7 @@ export const DocTableWrapper = forwardRef(
row={current}
useNewFieldsApi={useNewFieldsApi}
hideTimeColumn={hideTimeColumn}
isShortDots={isShortDots}
onAddColumn={onAddColumn}
onRemoveColumn={onRemoveColumn}
filterManager={filterManager}
Expand All @@ -223,10 +230,41 @@ export const DocTableWrapper = forwardRef(
onRemoveColumn,
filterManager,
addBasePath,
isShortDots,
fieldsToShow,
]
);

const truncateStyles = (
<Global
Copy link
Contributor

Choose a reason for hiding this comment

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

I think adding it in this place, might cause some issues. This is the legacy table implementation, which might not be enabled, if users opt-in for datagrid. But we're using the truncate-by-height class also outside the legacy implementation in one place - the tabular tab (https://github.com/elastic/kibana/blob/master/src/plugins/discover/public/application/components/table/table_cell_value.tsx#L28) which would still be available even when new data grid is enabled, but then wouldn't get those styles anymore.

I think we need to pull this out a layer, and e.g. put it into the discover layout component. cc @kertal Or am I missunderstanding the hierachy of components here?

Copy link
Member

Choose a reason for hiding this comment

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

no this is correct! excellent catch. However, do we really need to have this configureable in the same way it is handled in table cell? since it's a different use case here. For now I'd say, yes this needs to be pull moved up in the hierarchy ... but then there's the problem, that we would also need this in the embeddable. So maybe it's better to add a similar implementation to the doc viewer table? A doc-viewer-truncate-by-height for this use case?

Copy link
Contributor Author

@dimaanj dimaanj Oct 14, 2021

Choose a reason for hiding this comment

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

There is also a problem with embeddable. When we have multiple discover tables in it, <Global> emotion styles will be overwritten per each table globally. So to get rid of that, I would suggest to apply inline emoution styles to each element which needs truncation, like I did in the last commit.

styles={css`
.truncate-by-height {
overflow: hidden;
max-height: ${maxHeight > 0 ? `${maxHeight}px !important` : 'none'};
display: inline-block;
}
.truncate-by-height:before {
top: ${maxHeight > 0
? maxHeight - TRUNCATE_GRADIENT_HEIGHT
: TRUNCATE_GRADIENT_HEIGHT * -1}px;
}
`}
/>
);

const noResultsError = (
<div className="kbnDocTable__error">
<EuiText size="xs" color="subdued">
<EuiIcon type="visualizeApp" size="m" color="subdued" />
<EuiSpacer size="m" />
<FormattedMessage
id="discover.docTable.noResultsTitle"
defaultMessage="No results found"
/>
</EuiText>
</div>
);

return (
<div
className="kbnDocTableWrapper eui-yScroll eui-xScroll"
Expand All @@ -237,6 +275,7 @@ export const DocTableWrapper = forwardRef(
data-render-complete={!isLoading}
ref={ref as React.MutableRefObject<HTMLDivElement>}
>
{maxHeight !== 0 && truncateStyles}
{rows.length !== 0 &&
render({
columnLength: columns.length,
Expand All @@ -246,18 +285,7 @@ export const DocTableWrapper = forwardRef(
renderHeader,
renderRows,
})}
{!rows.length && (
<div className="kbnDocTable__error">
<EuiText size="xs" color="subdued">
<EuiIcon type="visualizeApp" size="m" color="subdued" />
<EuiSpacer size="m" />
<FormattedMessage
id="discover.docTable.noResultsTitle"
defaultMessage="No results found"
/>
</EuiText>
</div>
)}
{!rows.length && noResultsError}
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* 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 { IndexPattern } from '../../../../../../../../../data/common';
import { stubbedSavedObjectIndexPattern } from '../../../../../../../../../data/common/stubs';
import { fieldFormatsMock } from '../../../../../../../../../field_formats/common/mocks';
import { DocTableRow } from '../../components/table_row';

export const hit = {
_id: 'a',
_type: 'doc',
_score: 1,
_source: {
foo: 'bar',
number: 42,
hello: '<h1>World</h1>',
also: 'with "quotes" or \'single quotes\'',
},
} as DocTableRow;

const createIndexPattern = () => {
const id = 'my-index';
const {
type,
version,
attributes: { timeFieldName, fields, title },
} = stubbedSavedObjectIndexPattern(id);

return new IndexPattern({
spec: { id, type, version, timeFieldName, fields: JSON.parse(fields), title },
fieldFormats: fieldFormatsMock,
shortDotsEnable: false,
metaFields: [],
});
};

export const indexPattern = createIndexPattern();
Original file line number Diff line number Diff line change
Expand Up @@ -8,43 +8,11 @@

import ReactDOM from 'react-dom/server';
import { formatRow, formatTopLevelObject } from './row_formatter';
import { IndexPattern } from '../../../../../../../../data/common';
import { fieldFormatsMock } from '../../../../../../../../field_formats/common/mocks';
import { setServices } from '../../../../../../kibana_services';
import { DiscoverServices } from '../../../../../../build_services';
import { stubbedSavedObjectIndexPattern } from '../../../../../../../../data/common/stubs';
import { setServices } from '../../../../../../../kibana_services';
import { DiscoverServices } from '../../../../../../../build_services';
import { indexPattern, hit } from './mocks';

describe('Row formatter', () => {
const hit = {
_id: 'a',
_type: 'doc',
_score: 1,
_source: {
foo: 'bar',
number: 42,
hello: '<h1>World</h1>',
also: 'with "quotes" or \'single quotes\'',
},
};

const createIndexPattern = () => {
const id = 'my-index';
const {
type,
version,
attributes: { timeFieldName, fields, title },
} = stubbedSavedObjectIndexPattern(id);

return new IndexPattern({
spec: { id, type, version, timeFieldName, fields: JSON.parse(fields), title },
fieldFormats: fieldFormatsMock,
shortDotsEnable: false,
metaFields: [],
});
};

const indexPattern = createIndexPattern();

const fieldsToShow = indexPattern.fields.getAll().map((fld) => fld.name);

// Realistic response with alphabetical insertion order
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

import React, { Fragment } from 'react';
import type { IndexPattern } from 'src/plugins/data/common';
import { MAX_DOC_FIELDS_DISPLAYED } from '../../../../../../../common';
import { getServices } from '../../../../../../kibana_services';
import { MAX_DOC_FIELDS_DISPLAYED } from '../../../../../../../../common';
import { getServices } from '../../../../../../../kibana_services';

import './row_formatter.scss';

Expand All @@ -18,7 +18,7 @@ interface Props {
}
const TemplateComponent = ({ defPairs }: Props) => {
return (
<dl className={'source truncate-by-height'}>
<dl className="source truncate-by-height">
{defPairs.map((pair, idx) => (
<Fragment key={idx}>
<dt>{pair[0]}:</dt>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* 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 { formatSource } from './source_formatter';
import { hit, indexPattern } from './mocks';

describe('_source formatter', () => {
it('should format properly', () => {
const element = formatSource({ hit, indexPattern, isShortDots: true });
expect(element).toMatchInlineSnapshot(`
<TemplateComponent
defPairs={
Array [
Array [
"also",
"with \\"quotes\\" or 'single quotes'",
],
Array [
"foo",
"bar",
],
Array [
"hello",
"<h1>World</h1>",
],
Array [
"number",
42,
],
]
}
/>
`);
});
});
Loading