Skip to content

Commit

Permalink
[Discover] Show loading indicator when DataView is switched (elastic#…
Browse files Browse the repository at this point in the history
…177240)

Interating a loading indicator after 300ms when a data view is being switched and its field caps request takes longer e.g. because it hasn't been cached before. This is providing a better visual feedback when  there's a loading process in progress. For faster cached requests the loading indicate isn't displayed, preventing a flickered rendering in this case.


Co-authored-by: Julia Rechkunova <julia.rechkunova@gmail.com>
  • Loading branch information
kertal and jughosta authored Feb 23, 2024
1 parent 02d935f commit 9b9b7eb
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@
*/
import './discover_layout.scss';
import React, { ReactElement, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { EuiPage, EuiPageBody, EuiPanel, useEuiBackgroundColor } from '@elastic/eui';
import {
EuiPage,
EuiPageBody,
EuiPanel,
EuiProgress,
useEuiBackgroundColor,
EuiDelayRender,
} from '@elastic/eui';
import { css } from '@emotion/react';
import { i18n } from '@kbn/i18n';
import { METRIC_TYPE } from '@kbn/analytics';
Expand Down Expand Up @@ -81,7 +88,10 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {
if (uiSettings.get(SHOW_FIELD_STATISTICS) !== true) return VIEW_MODE.DOCUMENT_LEVEL;
return state.viewMode ?? VIEW_MODE.DOCUMENT_LEVEL;
});
const dataView = useInternalStateSelector((state) => state.dataView!);
const [dataView, dataViewLoading] = useInternalStateSelector((state) => [
state.dataView!,
state.isDataViewLoading,
]);
const dataState: DataMainMsg = useDataState(main$);
const savedSearch = useSavedSearchInitial();

Expand Down Expand Up @@ -293,6 +303,11 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {
height: 100%;
`}
>
{dataViewLoading && (
<EuiDelayRender delay={300}>
<EuiProgress size="xs" color="accent" position="absolute" />
</EuiDelayRender>
)}
<SavedSearchURLConflictCallout
savedSearch={savedSearch}
spaces={spaces}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { savedSearchMock } from '../../../../__mocks__/saved_search';
import { discoverServiceMock } from '../../../../__mocks__/services';
import type { DataView } from '@kbn/data-views-plugin/common';
import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock';
import { PureTransitionsToTransitions } from '@kbn/kibana-utils-plugin/common/state_containers';
import { InternalStateTransitions } from '../../services/discover_internal_state_container';

const setupTestParams = (dataView: DataView | undefined) => {
const savedSearch = savedSearchMock;
Expand All @@ -26,7 +28,14 @@ const setupTestParams = (dataView: DataView | undefined) => {
discoverState.internalState.transitions.setDataView(savedSearch.searchSource.getField('index')!);
services.dataViews.get = jest.fn(() => Promise.resolve(dataView as DataView));
discoverState.appState.update = jest.fn();
return { services, appState: discoverState.appState, internalState: discoverState.internalState };
discoverState.internalState.transitions = {
setIsDataViewLoading: jest.fn(),
} as unknown as Readonly<PureTransitionsToTransitions<InternalStateTransitions>>;
return {
services,
appState: discoverState.appState,
internalState: discoverState.internalState,
};
};

describe('changeDataView', () => {
Expand All @@ -38,6 +47,8 @@ describe('changeDataView', () => {
index: 'data-view-with-user-default-column-id',
sort: [['@timestamp', 'desc']],
});
expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(1, true);
expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(2, false);
});

it('should set the right app state when a valid data view to switch to is given', async () => {
Expand All @@ -48,11 +59,15 @@ describe('changeDataView', () => {
index: 'data-view-with-various-field-types-id',
sort: [['data', 'desc']],
});
expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(1, true);
expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(2, false);
});

it('should not set the app state when an invalid data view to switch to is given', async () => {
const params = setupTestParams(undefined);
await changeDataView('data-view-with-various-field-types', params);
expect(params.appState.update).not.toHaveBeenCalled();
expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(1, true);
expect(params.internalState.transitions.setIsDataViewLoading).toHaveBeenNthCalledWith(2, false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export async function changeDataView(
const dataView = internalState.getState().dataView;
const state = appState.getState();
let nextDataView: DataView | null = null;
internalState.transitions.setIsDataViewLoading(true);

try {
nextDataView = typeof id === 'string' ? await dataViews.get(id, false) : id;
Expand All @@ -63,4 +64,5 @@ export async function changeDataView(
internalState.transitions.setExpandedDoc(undefined);
}
}
internalState.transitions.setIsDataViewLoading(false);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,16 @@ import type { DataTableRecord } from '@kbn/discover-utils/types';

export interface InternalState {
dataView: DataView | undefined;
isDataViewLoading: boolean;
savedDataViews: DataViewListItem[];
adHocDataViews: DataView[];
expandedDoc: DataTableRecord | undefined;
customFilters: Filter[];
}

interface InternalStateTransitions {
export interface InternalStateTransitions {
setDataView: (state: InternalState) => (dataView: DataView) => InternalState;
setIsDataViewLoading: (state: InternalState) => (isLoading: boolean) => InternalState;
setSavedDataViews: (state: InternalState) => (dataView: DataViewListItem[]) => InternalState;
setAdHocDataViews: (state: InternalState) => (dataViews: DataView[]) => InternalState;
appendAdHocDataViews: (
Expand Down Expand Up @@ -52,6 +54,7 @@ export function getInternalStateContainer() {
return createStateContainer<InternalState, InternalStateTransitions, {}>(
{
dataView: undefined,
isDataViewLoading: false,
adHocDataViews: [],
savedDataViews: [],
expandedDoc: undefined,
Expand All @@ -62,6 +65,10 @@ export function getInternalStateContainer() {
...prevState,
dataView: nextDataView,
}),
setIsDataViewLoading: (prevState: InternalState) => (loading: boolean) => ({
...prevState,
isDataViewLoading: loading,
}),
setSavedDataViews: (prevState: InternalState) => (nextDataViewList: DataViewListItem[]) => ({
...prevState,
savedDataViews: nextDataViewList,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ describe('test fetchAll', () => {
getAppState: () => ({}),
getInternalState: () => ({
dataView: undefined,
isDataViewLoading: false,
savedDataViews: [],
adHocDataViews: [],
expandedDoc: undefined,
Expand Down Expand Up @@ -269,6 +270,7 @@ describe('test fetchAll', () => {
getAppState: () => ({ query }),
getInternalState: () => ({
dataView: undefined,
isDataViewLoading: false,
savedDataViews: [],
adHocDataViews: [],
expandedDoc: undefined,
Expand Down Expand Up @@ -394,6 +396,7 @@ describe('test fetchAll', () => {
getAppState: () => ({ query }),
getInternalState: () => ({
dataView: undefined,
isDataViewLoading: false,
savedDataViews: [],
adHocDataViews: [],
expandedDoc: undefined,
Expand Down

0 comments on commit 9b9b7eb

Please sign in to comment.