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

[Lens] Add "no data" popover #69147

Merged
merged 17 commits into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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 @@ -7,7 +7,7 @@
<b>Signature:</b>

```typescript
SearchBar: React.ComponentClass<Pick<Pick<SearchBarProps, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "intl" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "timeHistory" | "onFiltersUpdated">, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "timeHistory" | "onFiltersUpdated">, any> & {
WrappedComponent: React.ComponentType<Pick<SearchBarProps, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "intl" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "timeHistory" | "onFiltersUpdated"> & ReactIntl.InjectedIntlProps>;
SearchBar: React.ComponentClass<Pick<Pick<SearchBarProps, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "intl" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "indicateNoData" | "timeHistory" | "onFiltersUpdated">, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "indicateNoData" | "timeHistory" | "onFiltersUpdated">, any> & {
WrappedComponent: React.ComponentType<Pick<SearchBarProps, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "intl" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "indicateNoData" | "timeHistory" | "onFiltersUpdated"> & ReactIntl.InjectedIntlProps>;
}
```
4 changes: 2 additions & 2 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1743,8 +1743,8 @@ export const search: {
// Warning: (ae-missing-release-tag) "SearchBar" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
export const SearchBar: React.ComponentClass<Pick<Pick<SearchBarProps, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "intl" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "timeHistory" | "onFiltersUpdated">, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "timeHistory" | "onFiltersUpdated">, any> & {
WrappedComponent: React.ComponentType<Pick<SearchBarProps, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "intl" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "timeHistory" | "onFiltersUpdated"> & ReactIntl.InjectedIntlProps>;
export const SearchBar: React.ComponentClass<Pick<Pick<SearchBarProps, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "intl" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "indicateNoData" | "timeHistory" | "onFiltersUpdated">, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "indicateNoData" | "timeHistory" | "onFiltersUpdated">, any> & {
WrappedComponent: React.ComponentType<Pick<SearchBarProps, "query" | "isLoading" | "filters" | "onRefresh" | "onRefreshChange" | "refreshInterval" | "intl" | "indexPatterns" | "dataTestSubj" | "customSubmitButton" | "screenTitle" | "showQueryBar" | "showQueryInput" | "showFilterBar" | "showDatePicker" | "showAutoRefreshOnly" | "isRefreshPaused" | "dateRangeFrom" | "dateRangeTo" | "showSaveQuery" | "savedQuery" | "onQueryChange" | "onQuerySubmit" | "onSaved" | "onSavedQueryUpdated" | "onClearSavedQuery" | "indicateNoData" | "timeHistory" | "onFiltersUpdated"> & ReactIntl.InjectedIntlProps>;
};

// Warning: (ae-forgotten-export) The symbol "SearchBarOwnProps" needs to be exported by the entry point index.d.ts
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';
import { mountWithIntl as mount } from 'test_utils/enzyme_helpers';
import { NoDataPopover } from './no_data_popover';
import { EuiTourStep } from '@elastic/eui';
import { act } from 'react-dom/test-utils';

describe('NoDataPopover', () => {
const createMockStorage = () => ({
get: jest.fn(),
set: jest.fn(),
remove: jest.fn(),
clear: jest.fn(),
});

it('should hide popover if showNoDataPopover is set to false', () => {
const Child = () => <span />;
const instance = mount(
<NoDataPopover storage={createMockStorage()} showNoDataPopover={false}>
<Child />
</NoDataPopover>
);
expect(instance.find(EuiTourStep).prop('isStepOpen')).toBe(false);
expect(instance.find(EuiTourStep).find(Child)).toHaveLength(1);
});

it('should hide popover if showNoDataPopover is set to true, but local storage flag is set', () => {
const child = <span />;
const storage = createMockStorage();
storage.get.mockReturnValue(true);
const instance = mount(
<NoDataPopover storage={storage} showNoDataPopover={false}>
{child}
</NoDataPopover>
);
expect(instance.find(EuiTourStep).prop('isStepOpen')).toBe(false);
});

it('should render popover if showNoDataPopover is set to true and local storage flag is not set', () => {
const child = <span />;
const instance = mount(
<NoDataPopover storage={createMockStorage()} showNoDataPopover={true}>
{child}
</NoDataPopover>
);
expect(instance.find(EuiTourStep).prop('isStepOpen')).toBe(true);
});

it('should hide popover if it is closed', async () => {
const props = {
children: <span />,
showNoDataPopover: true,
storage: createMockStorage(),
};
const instance = mount(<NoDataPopover {...props} />);
act(() => {
instance.find(EuiTourStep).prop('closePopover')!();
});
instance.setProps({ ...props });
expect(instance.find(EuiTourStep).prop('isStepOpen')).toBe(false);
});

it('should set local storage flag and hide on closing with button', () => {
const props = {
children: <span />,
showNoDataPopover: true,
storage: createMockStorage(),
};
const instance = mount(<NoDataPopover {...props} />);
act(() => {
instance.find(EuiTourStep).prop('footerAction')!.props.onClick();
});
instance.setProps({ ...props });
expect(props.storage.set).toHaveBeenCalledWith(expect.any(String), true);
expect(instance.find(EuiTourStep).prop('isStepOpen')).toBe(false);
});
});
96 changes: 96 additions & 0 deletions src/plugins/data/public/ui/query_string_input/no_data_popover.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { ReactElement, useEffect, useState } from 'react';
import React from 'react';
import { EuiButtonEmpty, EuiText, EuiTourStep } from '@elastic/eui';
import { IStorageWrapper } from 'src/plugins/kibana_utils/public';
import { i18n } from '@kbn/i18n';

const NO_DATA_POPOVER_STORAGE_KEY = 'data.noDataPopover';

export function NoDataPopover({
showNoDataPopover,
storage,
children,
}: {
showNoDataPopover?: boolean;
storage: IStorageWrapper;
children: ReactElement;
}) {
const [noDataPopoverDismissed, setNoDataPopoverDismissed] = useState(() =>
Boolean(storage.get(NO_DATA_POPOVER_STORAGE_KEY))
);
const [noDataPopoverVisible, setNoDataPopoverVisible] = useState(false);

useEffect(() => {
if (showNoDataPopover && !noDataPopoverDismissed) {
setNoDataPopoverVisible(true);
}
}, [noDataPopoverDismissed, showNoDataPopover]);

return (
<EuiTourStep
onFinish={() => {}}
closePopover={() => {
setNoDataPopoverVisible(false);
}}
content={
<EuiText size="s">
<p style={{ maxWidth: 300 }}>
{i18n.translate('data.noDataPopover.content', {
defaultMessage:
"This time range doesn't contain any data. Increase or adjust the time range to see more fields and create charts",
})}
</p>
</EuiText>
}
minWidth={300}
anchorPosition="downCenter"
step={1}
stepsTotal={1}
isStepOpen={noDataPopoverVisible}
subtitle={i18n.translate('data.noDataPopover.title', { defaultMessage: 'Tip' })}
title=""
footerAction={
<EuiButtonEmpty
size="s"
Copy link
Contributor

Choose a reason for hiding this comment

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

@flash1293 Minor nit, can you change/add these props:

  size="xs"
  flush="right"
  color="text"

data-test-subj="noDataPopoverDismissButton"
onClick={() => {
storage.set(NO_DATA_POPOVER_STORAGE_KEY, true);
setNoDataPopoverDismissed(true);
setNoDataPopoverVisible(false);
}}
>
{i18n.translate('data.noDataPopover.dismissAction', {
defaultMessage: "Don't show again",
})}
</EuiButtonEmpty>
}
>
<div
onFocus={() => {
setNoDataPopoverVisible(false);
}}
>
{children}
</div>
</EuiTourStep>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { useKibana, toMountPoint } from '../../../../kibana_react/public';
import { QueryStringInput } from './query_string_input';
import { doesKueryExpressionHaveLuceneSyntaxError, UI_SETTINGS } from '../../../common';
import { PersistedLog, getQueryLog } from '../../query';
import { NoDataPopover } from './no_data_popover';

interface Props {
query?: Query;
Expand All @@ -63,6 +64,7 @@ interface Props {
customSubmitButton?: any;
isDirty: boolean;
timeHistory?: TimeHistoryContract;
indicateNoData?: boolean;
}

export function QueryBarTopRow(props: Props) {
Expand Down Expand Up @@ -230,10 +232,12 @@ export function QueryBarTopRow(props: Props) {
}

return (
<EuiFlexGroup responsive={false} gutterSize="s">
{renderDatePicker()}
<EuiFlexItem grow={false}>{button}</EuiFlexItem>
</EuiFlexGroup>
<NoDataPopover storage={storage} showNoDataPopover={props.indicateNoData}>
lizozom marked this conversation as resolved.
Show resolved Hide resolved
<EuiFlexGroup responsive={false} gutterSize="s">
{renderDatePicker()}
<EuiFlexItem grow={false}>{button}</EuiFlexItem>
</EuiFlexGroup>
</NoDataPopover>
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export function createSearchBar({ core, storage, data }: StatefulSearchBarDeps)
showSaveQuery={props.showSaveQuery}
screenTitle={props.screenTitle}
indexPatterns={props.indexPatterns}
indicateNoData={props.indicateNoData}
timeHistory={data.query.timefilter.history}
dateRangeFrom={timeRange.from}
dateRangeTo={timeRange.to}
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/data/public/ui/search_bar/search_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export interface SearchBarOwnProps {
onClearSavedQuery?: () => void;

onRefresh?: (payload: { dateRange: TimeRange }) => void;
indicateNoData?: boolean;
}

export type SearchBarProps = SearchBarOwnProps & SearchBarInjectedDeps;
Expand Down Expand Up @@ -402,6 +403,7 @@ class SearchBarUI extends Component<SearchBarProps, State> {
this.props.customSubmitButton ? this.props.customSubmitButton : undefined
}
dataTestSubj={this.props.dataTestSubj}
indicateNoData={this.props.indicateNoData}
/>
);
}
Expand Down
7 changes: 7 additions & 0 deletions test/functional/page_objects/time_picker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ export function TimePickerProvider({ getService, getPageObjects }: FtrProviderCo
await this.setAbsoluteRange(this.defaultStartTime, this.defaultEndTime);
}

async ensureHiddenNoDataPopover() {
const isVisible = await testSubjects.exists('noDataPopoverDismissButton');
if (isVisible) {
await testSubjects.click('noDataPopoverDismissButton');
}
}

/**
* the provides a quicker way to set the timepicker to the default range, saves a few seconds
*/
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lens/public/app_plugin/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ describe('Lens App', () => {
"query": "",
},
"savedQuery": undefined,
"showNoDataPopover": [Function],
},
],
]
Expand Down
21 changes: 21 additions & 0 deletions x-pack/plugins/lens/public/app_plugin/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
} from '../../../../../src/plugins/data/public';

interface State {
indicateNoData: boolean;
isLoading: boolean;
isSaveModalVisible: boolean;
indexPatternsForTopNav: IndexPatternInstance[];
Expand Down Expand Up @@ -97,9 +98,27 @@ export function App({
toDate: currentRange.to,
},
filters: [],
indicateNoData: false,
};
});

const showNoDataPopover = useCallback(() => {
setState((prevState) => ({ ...prevState, indicateNoData: true }));
}, [setState]);

useEffect(() => {
if (state.indicateNoData) {
setState((prevState) => ({ ...prevState, indicateNoData: false }));
}
}, [
setState,
state.indicateNoData,
state.query,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you have to list attributes that are not used explicitly in the effect? Is it because of prevState? 🤔

Copy link
Contributor Author

@flash1293 flash1293 Jun 29, 2020

Choose a reason for hiding this comment

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

We want to show this popover only on first load - if the user starts to change the configuration, they most likely now how to get data (e.g. by changing the index pattern because the wrong one was pre-selected). Then we don't need the popover anymore.

This is just to be extra safe (as the popover is also hidden when the user interacts with the UI outside of it)

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice adding a comment on this as this is not trivial behavior

state.filters,
state.dateRange,
state.indexPatternsForTopNav,
]);

const { lastKnownDoc } = state;

const isSaveable =
Expand Down Expand Up @@ -458,6 +477,7 @@ export function App({
query={state.query}
dateRangeFrom={state.dateRange.fromDate}
dateRangeTo={state.dateRange.toDate}
indicateNoData={state.indicateNoData}
/>
</div>

Expand All @@ -472,6 +492,7 @@ export function App({
savedQuery: state.savedQuery,
doc: state.persistedDoc,
onError,
showNoDataPopover,
onChange: ({ filterableIndexPatterns, doc }) => {
if (!_.isEqual(state.persistedDoc, doc)) {
setState((s) => ({ ...s, lastKnownDoc: doc }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface DataPanelWrapperProps {
activeDatasource: string | null;
datasourceIsLoading: boolean;
dispatch: (action: Action) => void;
showNoDataPopover: () => void;
core: DatasourceDataPanelProps['core'];
query: Query;
dateRange: FramePublicAPI['dateRange'];
Expand Down Expand Up @@ -46,6 +47,7 @@ export const DataPanelWrapper = memo((props: DataPanelWrapperProps) => {
query: props.query,
dateRange: props.dateRange,
filters: props.filters,
showNoDataPopover: props.showNoDataPopover,
};

const [showDatasourceSwitcher, setDatasourceSwitcher] = useState(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ function getDefaultProps() {
data: dataPluginMock.createStartContract(),
expressions: expressionsPluginMock.createStartContract(),
},
showNoDataPopover: jest.fn(),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export interface EditorFrameProps {
filterableIndexPatterns: DatasourceMetaData['filterableIndexPatterns'];
doc: Document;
}) => void;
showNoDataPopover: () => void;
}

export function EditorFrame(props: EditorFrameProps) {
Expand Down Expand Up @@ -255,6 +256,7 @@ export function EditorFrame(props: EditorFrameProps) {
query={props.query}
dateRange={props.dateRange}
filters={props.filters}
showNoDataPopover={props.showNoDataPopover}
/>
}
configPanel={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('editor_frame state management', () => {
dateRange: { fromDate: 'now-7d', toDate: 'now' },
query: { query: '', language: 'lucene' },
filters: [],
showNoDataPopover: jest.fn(),
};
});

Expand Down
Loading