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

[DataView] UX improvements for index pattern input in data view flyout #152138

Merged
merged 34 commits into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
6ab50c3
[DataView] Show all indices when index pattern ends with ","
jughosta Feb 24, 2023
00d4afd
[DataView] Allow to switch between view modes
jughosta Feb 24, 2023
ae5e3d3
[DataView] Improve interactions
jughosta Feb 24, 2023
e865d75
[DataView] Improve the input help
jughosta Feb 24, 2023
467f0f9
[DataView] Clean up i18n
jughosta Feb 24, 2023
907b6cd
[DataView] Add tests
jughosta Feb 27, 2023
c77b427
Merge branch 'main' into improvements-for-data-view-flyout
jughosta Feb 27, 2023
e359053
[DataView] Add a fix for the warning
jughosta Feb 27, 2023
7898c9a
[DataView] Persist "indices per page" setting in localStorage
jughosta Feb 27, 2023
6033f31
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Feb 27, 2023
d8e6e2a
[DataView] Update tests
jughosta Feb 27, 2023
b8cd0e7
Merge remote-tracking branch 'origin/improvements-for-data-view-flyou…
jughosta Feb 27, 2023
189596c
Merge branch 'main' into improvements-for-data-view-flyout
jughosta Feb 27, 2023
9d02c5c
[DataView] Add more tests. Handle partial match better.
jughosta Feb 27, 2023
e116b8d
[DataView] Add in-product docs popover for the index pattern input
jughosta Feb 27, 2023
6ef4a30
Merge branch 'main' into improvements-for-data-view-flyout
jughosta Feb 27, 2023
872f1ad
Merge remote-tracking branch 'origin/improvements-for-data-view-flyou…
jughosta Feb 27, 2023
f961abb
[DataView] Fix for checks
jughosta Feb 27, 2023
d65bfc5
[DataView] Improve matching logic when rendering
jughosta Feb 27, 2023
6ceab6a
Merge branch 'main' into improvements-for-data-view-flyout
jughosta Feb 27, 2023
a5f7f3f
[DataView] Add a placeholder
jughosta Feb 27, 2023
6b8505a
Merge branch 'main' into improvements-for-data-view-flyout
jughosta Feb 27, 2023
72eca96
Merge branch 'main' into improvements-for-data-view-flyout
jughosta Feb 28, 2023
5bfcf54
Merge remote-tracking branch 'upstream/main' into improvements-for-da…
jughosta Mar 1, 2023
d36e595
[DataView] Update messaging
jughosta Mar 1, 2023
7ec88e0
[DataView] Revert checkboxes and icons
jughosta Mar 1, 2023
f681b82
[DataView] Readd placeholder
jughosta Mar 1, 2023
c02bf10
Merge branch 'main' into improvements-for-data-view-flyout
jughosta Mar 2, 2023
8877942
[DataView] Update wording
jughosta Mar 2, 2023
4380abe
Merge branch 'main' into improvements-for-data-view-flyout
jughosta Mar 2, 2023
5614856
[DataView] Fix character list
jughosta Mar 2, 2023
f6c4003
Merge remote-tracking branch 'origin/improvements-for-data-view-flyou…
jughosta Mar 2, 2023
2d0cdc1
[DataView] Update "minus" item
jughosta Mar 2, 2023
bcdb2bd
[DataView] Address build checks
jughosta Mar 2, 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
1 change: 1 addition & 0 deletions src/plugins/data_view_editor/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
],
"requiredBundles": [
"kibanaReact",
"kibanaUtils",
"esUiShared"
]
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* 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 React from 'react';
import { findTestSubject } from '@elastic/eui/lib/test';
import { mountWithIntl } from '@kbn/test-jest-helpers';
import { TitleDocsPopover } from './title_docs_popover';

describe('DataViewEditor TitleDocsPopover', () => {
it('should render normally', async () => {
const component = mountWithIntl(<TitleDocsPopover />);

expect(findTestSubject(component, 'indexPatternDocsButton').exists()).toBeTruthy();
expect(findTestSubject(component, 'indexPatternDocsPopoverContent').exists()).toBeFalsy();

findTestSubject(component, 'indexPatternDocsButton').simulate('click');

await component.update();

expect(findTestSubject(component, 'indexPatternDocsPopoverContent').exists()).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* 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 React, { useState } from 'react';
import { css } from '@emotion/react';
import { FormattedMessage } from '@kbn/i18n-react';
import {
EuiButtonIcon,
EuiPanel,
EuiPopover,
EuiPopoverTitle,
EuiText,
EuiCode,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

export const TitleDocsPopover: React.FC = () => {
const [isOpen, setIsOpen] = useState<boolean>(false);

const helpButton = (
<EuiButtonIcon
onClick={() => setIsOpen((prev) => !prev)}
iconType="documentation"
data-test-subj="indexPatternDocsButton"
aria-label={i18n.translate('indexPatternEditor.titleDocsPopover.ariaLabel', {
defaultMessage: 'Index pattern examples',
})}
/>
);

return (
<EuiPopover
button={helpButton}
isOpen={isOpen}
display="inlineBlock"
panelPaddingSize="none"
closePopover={() => setIsOpen(false)}
>
<EuiPopoverTitle paddingSize="s">
{i18n.translate('indexPatternEditor.titleDocsPopover.title', {
defaultMessage: 'Index pattern',
})}
</EuiPopoverTitle>
<EuiPanel
className="eui-yScroll"
css={css`
max-height: 40vh;
max-width: 500px;
`}
color="transparent"
paddingSize="m"
>
<EuiText size="s" data-test-subj="indexPatternDocsPopoverContent">
<p>
<FormattedMessage
id="indexPatternEditor.titleDocsPopover.indexPatternDescription"
defaultMessage="An index pattern is a string that you use to match one or more data streams, indices, or aliases."
/>
</p>
<ul>
<li>
<p>
<FormattedMessage
id="indexPatternEditor.titleDocsPopover.useWildcardDescription"
defaultMessage="Match multiple sources with a wildcard (*)."
/>
</p>
<p>
<EuiCode>filebeat-*</EuiCode>
</p>
</li>
<li>
<p>
<FormattedMessage
id="indexPatternEditor.titleDocsPopover.useCommasDescription"
defaultMessage="Separate multiple single sources with a comma (,)."
/>
</p>
<p>
<EuiCode>filebeat-a,filebeat-b</EuiCode>
</p>
</li>
<li>
<p>
<FormattedMessage
id="indexPatternEditor.titleDocsPopover.useMinusDescription"
defaultMessage="Exclude a source with the minus sign (-)."
/>
</p>
<p>
<EuiCode>filebeat-*,-filebeat-c</EuiCode>
</p>
</li>
<li>
<p>
<FormattedMessage
id="indexPatternEditor.titleDocsPopover.dontUseSpecialCharactersDescription"
defaultMessage="Spaces and the characters /?”<> are not allowed."
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like the | char is missing, also the quote character changed. I'm comparing to the form schema change.

/>
</p>
</li>
</ul>
</EuiText>
</EuiPanel>
</EuiPopover>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @gchaps,

Could you please review this new popover?
I pulled the text from docs.

Screenshot 2023-03-01 at 12 02 46

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm tempted to shorted the index pattern field description text now that there's a popover to just the first sentence.

Copy link
Contributor

@gchaps gchaps Mar 1, 2023

Choose a reason for hiding this comment

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

I agree with shortening the hint text under the field description. I'll work up some text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found that back in v7.10 we had the following messaging: An index pattern can match a single source, for example, filebeat-4-3-22, or multiple data sources, filebeat-*.

Could it be a better hint for the input?

Screenshot 2023-03-02 at 13 07 35

);
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { canAppendWildcard } from '../../lib';
import { schema } from '../form_schema';
import { RollupIndicesCapsResponse, IndexPatternConfig, MatchedIndicesSet } from '../../types';
import { matchedIndiciesDefault } from '../../data_view_editor_service';
import { TitleDocsPopover } from './title_docs_popover';

interface TitleFieldProps {
isRollup: boolean;
Expand Down Expand Up @@ -194,6 +195,8 @@ export const TitleField = ({
isLoading={field.isValidating}
fullWidth
data-test-subj="createIndexPatternTitleInput"
append={<TitleDocsPopover />}
placeholder="example-*"
/>
</EuiFormRow>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ export const schema = {
defaultMessage: 'Index pattern',
}),
defaultValue: '',
helpText: i18n.translate('indexPatternEditor.validations.titleHelpText', {
defaultMessage:
'Enter an index pattern that matches one or more data sources. Use an asterisk (*) to match multiple characters. Spaces and the characters , /, ?, ", <, >, | are not allowed.',
}),
validations: [
{
validator: fieldValidators.emptyField(
Expand Down Expand Up @@ -84,7 +80,7 @@ export const schema = {
},
isAdHoc: {
label: i18n.translate('indexPatternEditor.editor.form.IsAdHocLabel', {
defaultMessage: 'Creeate AdHoc DataView',
defaultMessage: 'Create AdHoc DataView',
}),
defaultValue: false,
type: 'hidden',
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,39 @@
*/

import React from 'react';
import { IndicesList } from '.';
import { IndicesList, IndicesListProps, PER_PAGE_STORAGE_KEY } from './indices_list';
import { shallow } from 'enzyme';
import { MatchedItem } from '@kbn/data-views-plugin/public';
import { Storage } from '@kbn/kibana-utils-plugin/public';

const indices = [
{ name: 'kibana', tags: [] },
{ name: 'es', tags: [] },
] as unknown as MatchedItem[];

const similarIndices = [
{ name: 'logstash', tags: [] },
{ name: 'some_logs', tags: [] },
] as unknown as MatchedItem[];

describe('IndicesList', () => {
const commonProps: Omit<IndicesListProps, 'query'> = {
indices,
isExactMatch: jest.fn(() => false),
};

afterEach(() => {
new Storage(localStorage).remove(PER_PAGE_STORAGE_KEY);
});

it('should render normally', () => {
const component = shallow(<IndicesList indices={indices} query="" />);
const component = shallow(<IndicesList {...commonProps} query="" />);

expect(component).toMatchSnapshot();
});

it('should change pages', () => {
const component = shallow(<IndicesList indices={indices} query="" />);
const component = shallow(<IndicesList {...commonProps} query="" />);

const instance = component.instance() as IndicesList;

Expand All @@ -36,7 +51,7 @@ describe('IndicesList', () => {
});

it('should change per page', () => {
const component = shallow(<IndicesList indices={indices} query="" />);
const component = shallow(<IndicesList {...commonProps} query="" />);

const instance = component.instance() as IndicesList;
instance.onChangePerPage(1);
Expand All @@ -46,14 +61,33 @@ describe('IndicesList', () => {
});

it('should highlight the query in the matches', () => {
const component = shallow(<IndicesList indices={indices} query="ki" />);
const component = shallow(
<IndicesList
{...commonProps}
query="es,ki"
isExactMatch={(indexName) => indexName === 'es'}
/>
);

expect(component).toMatchSnapshot();
});

it('should highlight fully when an exact match', () => {
const component = shallow(
<IndicesList
{...commonProps}
indices={similarIndices}
query="logs*"
isExactMatch={(indexName) => indexName === 'some_logs'}
/>
);

expect(component).toMatchSnapshot();
});

describe('updating props', () => {
it('should render all new indices', () => {
const component = shallow(<IndicesList indices={indices} query="" />);
const component = shallow(<IndicesList {...commonProps} query="" />);

const moreIndices = [
...indices,
Expand Down
Loading