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.

Sign up for GitHub

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] Removing large string truncation from doc viewer #164236

Merged
merged 5 commits into from
Aug 23, 2023
Merged

[Discover] Removing large string truncation from doc viewer #164236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ab8e2c0
removing large string truncation from legacy doc viewer
timductive Aug 17, 2023
2f2671f
commit using @elastic.co
timductive Aug 21, 2023
5823732
fixing tests and removing unused translations
timductive Aug 21, 2023
8f7bb55
fixing functional test
timductive Aug 22, 2023
0f43332
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Aug 22, 2023
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
49 changes: 14 additions & 35 deletions ...gins/discover/public/services/doc_views/components/doc_viewer_table/legacy/table.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,39 +127,34 @@ describe('DocViewTable at Discover', () => {
{
_property: '_index',
addInclusiveFilterButton: true,
collapseBtn: false,
noMappingWarning: false,
toggleColumnButton: true,
underscoreWarning: false,
},
{
_property: 'message',
addInclusiveFilterButton: false,
collapseBtn: true,
noMappingWarning: false,
toggleColumnButton: true,
underscoreWarning: false,
},
{
_property: '_underscore',
addInclusiveFilterButton: false,
collapseBtn: false,
noMappingWarning: false,
toggleColumnButton: true,
underScoreWarning: true,
},
{
_property: 'scripted',
addInclusiveFilterButton: false,
collapseBtn: false,
noMappingWarning: false,
toggleColumnButton: true,
underScoreWarning: false,
},
{
_property: 'not_mapped',
addInclusiveFilterButton: false,
collapseBtn: false,
noMappingWarning: true,
toggleColumnButton: true,
underScoreWarning: false,
Expand All @@ -171,26 +166,21 @@ describe('DocViewTable at Discover', () => {
expect(rowComponent.length).toBe(1);
});

(
[
'addInclusiveFilterButton',
'collapseBtn',
'toggleColumnButton',
'underscoreWarning',
] as const
).forEach((element) => {
const elementExist = check[element];

if (typeof elementExist === 'boolean') {
const btn = findTestSubject(rowComponent, element, '^=');

it(`renders ${element} for '${check._property}' correctly`, () => {
const disabled = btn.length ? btn.props().disabled : true;
const clickAble = btn.length && !disabled ? true : false;
expect(clickAble).toBe(elementExist);
});
(['addInclusiveFilterButton', 'toggleColumnButton', 'underscoreWarning'] as const).forEach(
(element) => {
const elementExist = check[element];

if (typeof elementExist === 'boolean') {
const btn = findTestSubject(rowComponent, element, '^=');

it(`renders ${element} for '${check._property}' correctly`, () => {
const disabled = btn.length ? btn.props().disabled : true;
const clickAble = btn.length && !disabled ? true : false;
expect(clickAble).toBe(elementExist);
});
}
}
});
);
});
});

Expand Down Expand Up @@ -236,17 +226,6 @@ describe('DocViewTable at Discover Context', () => {
btn.simulate('click');
expect(props.filter).toBeCalled();
});

it(`renders functional collapse button`, () => {
const btn = findTestSubject(component, `collapseBtn`);
const html = component.html();

expect(component.html()).toContain('dscTruncateByHeight');

expect(btn.length).toBe(1);
btn.simulate('click');
expect(component.html() !== html).toBeTruthy();
});
});

describe('DocViewTable at Discover Doc', () => {
Expand Down
34 changes: 0 additions & 34 deletions ...r/public/services/doc_views/components/doc_viewer_table/legacy/table_row_btn_collapse.tsx
View file
Open in desktop

This file was deleted.

22 changes: 2 additions & 20 deletions ...ugins/discover/public/services/doc_views/components/doc_viewer_table/table_cell_value.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,10 @@
import { css } from '@emotion/react';
import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiTextColor, EuiToolTip } from '@elastic/eui';
import classNames from 'classnames';
import React, { Fragment, useState } from 'react';
import React, { Fragment } from 'react';
import { i18n } from '@kbn/i18n';
import { IgnoredReason } from '@kbn/discover-utils';
import { FieldRecord } from './table';
import { DocViewTableRowBtnCollapse } from './legacy/table_row_btn_collapse';
Copy link
Member

Choose a reason for hiding this comment

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

this is a nice cleanup @timductive 👏
If was just shortly confused about the title "Legacy doc viewer" ... But this PR is removing the collapse functionality of all doc viewers, right, which makes sense

Copy link
Member

Choose a reason for hiding this comment

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

the coupling between doc_viewer for the 'classic' and the 'new' table is confusing, can't wait until we can clean this up

Copy link
Member Author

Choose a reason for hiding this comment

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

yes the code structure was a little hard for me to follow 😂 but you are correct i changed the title


const COLLAPSE_LINE_LENGTH = 350;

interface IgnoreWarningProps {
reason: IgnoredReason;
Expand Down Expand Up @@ -95,29 +92,14 @@ export const TableFieldValue = ({
rawValue,
ignoreReason,
}: TableFieldValueProps) => {
const [fieldOpen, setFieldOpen] = useState(false);

const value = String(rawValue);
const isCollapsible = value.length > COLLAPSE_LINE_LENGTH;
const isCollapsed = isCollapsible && !fieldOpen;

const valueClassName = classNames({
// eslint-disable-next-line @typescript-eslint/naming-convention
kbnDocViewer__value: true,
dscTruncateByHeight: isCollapsible && isCollapsed,
});

const onToggleCollapse = () => setFieldOpen((fieldOpenPrev) => !fieldOpenPrev);

return (
<Fragment>
{(isCollapsible || ignoreReason) && (
{ignoreReason && (
<EuiFlexGroup gutterSize="s">
{isCollapsible && (
<EuiFlexItem grow={false}>
<DocViewTableRowBtnCollapse onClick={onToggleCollapse} isCollapsed={isCollapsed} />
</EuiFlexItem>
)}
{ignoreReason && (
<EuiFlexItem grow={false}>
<IgnoreWarning reason={ignoreReason} rawValue={rawValue} />
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/fr-FR.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2334,7 +2334,6 @@
"discover.docViews.table.tableTitle": "Tableau",
"discover.docViews.table.toggleColumnInTableButtonAriaLabel": "Afficher/Masquer la colonne dans le tableau",
"discover.docViews.table.toggleColumnInTableButtonTooltip": "Afficher/Masquer la colonne dans le tableau",
"discover.docViews.table.toggleFieldDetails": "Afficher/Masquer les détails du champ",
"discover.docViews.table.unableToFilterForPresenceOfMetaFieldsTooltip": "Impossible de filtrer sur les champs méta",
"discover.docViews.table.unableToFilterForPresenceOfScriptedFieldsTooltip": "Impossible de filtrer sur les champs scriptés",
"discover.docViews.table.unindexedFieldsCanNotBeSearchedTooltip": "Les champs non indexés ou les valeurs ignorées ne peuvent pas être recherchés",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/ja-JP.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2349,7 +2349,6 @@
"discover.docViews.table.tableTitle": "表",
"discover.docViews.table.toggleColumnInTableButtonAriaLabel": "表の列を切り替える",
"discover.docViews.table.toggleColumnInTableButtonTooltip": "表の列を切り替える",
"discover.docViews.table.toggleFieldDetails": "フィールド詳細を切り替える",
"discover.docViews.table.unableToFilterForPresenceOfMetaFieldsTooltip": "メタフィールドの有無でフィルタリングできません",
"discover.docViews.table.unableToFilterForPresenceOfScriptedFieldsTooltip": "スクリプトフィールドの有無でフィルタリングできません",
"discover.docViews.table.unindexedFieldsCanNotBeSearchedTooltip": "インデックスがないフィールドまたは無視された値は検索できません",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/zh-CN.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2349,7 +2349,6 @@
"discover.docViews.table.tableTitle": "表",
"discover.docViews.table.toggleColumnInTableButtonAriaLabel": "在表中切换列",
"discover.docViews.table.toggleColumnInTableButtonTooltip": "在表中切换列",
"discover.docViews.table.toggleFieldDetails": "切换字段详细信息",
"discover.docViews.table.unableToFilterForPresenceOfMetaFieldsTooltip": "无法筛选元数据字段是否存在",
"discover.docViews.table.unableToFilterForPresenceOfScriptedFieldsTooltip": "无法筛选脚本字段是否存在",
"discover.docViews.table.unindexedFieldsCanNotBeSearchedTooltip": "无法搜索未编入索引的字段或被忽略的值",
Expand Down
1 change: 0 additions & 1 deletion x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/discover/search_source_alert.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const getResultsLink = async () => {
// getting the link
await dataGrid.clickRowToggle();
await testSubjects.click('collapseBtn');
const contextMessageElement = await testSubjects.find('tableDocViewRow-context_message-value');
const contextMessage = await contextMessageElement.getVisibleText();
const [, link] = contextMessage.split(`Link\: `);
Expand Down