Skip to content

Commit

Permalink
[APM] Reduce loading indicator flickering (#34701)
Browse files Browse the repository at this point in the history
* [APM] Reduce progress bar flickering

* Make private methods private

* Fix eslint issue with console.log
  • Loading branch information
sorenlouv authored Apr 23, 2019
1 parent e799592 commit e97ea46
Show file tree
Hide file tree
Showing 9 changed files with 325 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { EuiDelayHide, EuiPortal, EuiProgress } from '@elastic/eui';
import { EuiPortal, EuiProgress } from '@elastic/eui';
import React, { Fragment, useMemo, useReducer } from 'react';
import { useDelayedVisibility } from './useDelayedVisibility';

export const GlobalFetchContext = React.createContext({
statuses: {},
Expand All @@ -17,11 +18,18 @@ interface State {

interface Action {
isLoading: boolean;
name: string;
id: number;
}

function reducer(statuses: State, action: Action) {
return { ...statuses, [action.name]: action.isLoading };
// add loading status
if (action.isLoading) {
return { ...statuses, [action.id]: true };
}

// remove loading status
const { [action.id]: statusToRemove, ...restStatuses } = statuses;
return restStatuses;
}

function getIsAnyLoading(statuses: State) {
Expand All @@ -35,18 +43,15 @@ export function GlobalFetchIndicator({
}) {
const [statuses, dispatchStatus] = useReducer(reducer, {});
const isLoading = useMemo(() => getIsAnyLoading(statuses), [statuses]);
const shouldShowLoadingIndicator = useDelayedVisibility(isLoading);

return (
<Fragment>
<EuiDelayHide
hide={!isLoading}
minimumDuration={1000}
render={() => (
<EuiPortal>
<EuiProgress size="xs" position="fixed" />
</EuiPortal>
)}
/>
{shouldShowLoadingIndicator && (
<EuiPortal>
<EuiProgress size="xs" position="fixed" />
</EuiPortal>
)}

<GlobalFetchContext.Provider
value={{ statuses, dispatchStatus }}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { Delayed } from '.';

// Advanced time like setTimeout and mocks Date.now() to stay in sync
class AdvanceTimer {
public nowTime = 0;
public advance(ms: number) {
this.nowTime += ms;
jest.spyOn(Date, 'now').mockReturnValue(this.nowTime);
jest.advanceTimersByTime(ms);
}
}

describe('Delayed', () => {
it('should not flicker between show/hide when the hide interval is very short', async () => {
jest.useFakeTimers();
const visibilityChanges: boolean[] = [];
const advanceTimer = new AdvanceTimer();
const delayed = new Delayed();

delayed.onChange(isVisible => visibilityChanges.push(isVisible));

for (let i = 1; i < 100; i += 2) {
delayed.show();
advanceTimer.advance(1000);
delayed.hide();
advanceTimer.advance(20);
}
advanceTimer.advance(100);

expect(visibilityChanges).toEqual([true, false]);
});

it('should not be shown at all when the duration is very short', async () => {
jest.useFakeTimers();
const advanceTimer = new AdvanceTimer();
const visibilityChanges: boolean[] = [];
const delayed = new Delayed();

delayed.onChange(isVisible => visibilityChanges.push(isVisible));

delayed.show();
advanceTimer.advance(30);
delayed.hide();
advanceTimer.advance(1000);

expect(visibilityChanges).toEqual([]);
});

it('should be displayed for minimum 1000ms', async () => {
jest.useFakeTimers();
const visibilityChanges: boolean[] = [];
const advanceTimer = new AdvanceTimer();
const delayed = new Delayed();

delayed.onChange(isVisible => visibilityChanges.push(isVisible));

delayed.show();
advanceTimer.advance(200);
delayed.hide();
advanceTimer.advance(950);
expect(visibilityChanges).toEqual([true]);
advanceTimer.advance(100);
expect(visibilityChanges).toEqual([true, false]);
delayed.show();
advanceTimer.advance(50);
expect(visibilityChanges).toEqual([true, false, true]);
delayed.hide();
advanceTimer.advance(950);
expect(visibilityChanges).toEqual([true, false, true]);
advanceTimer.advance(100);
expect(visibilityChanges).toEqual([true, false, true, false]);
});

it('should be displayed for minimum 2000ms', async () => {
jest.useFakeTimers();
const visibilityChanges: boolean[] = [];
const advanceTimer = new AdvanceTimer();
const delayed = new Delayed({ minimumVisibleDuration: 2000 });

delayed.onChange(isVisible => visibilityChanges.push(isVisible));

delayed.show();
advanceTimer.advance(200);
delayed.hide();
advanceTimer.advance(1950);
expect(visibilityChanges).toEqual([true]);
advanceTimer.advance(100);
expect(visibilityChanges).toEqual([true, false]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

type Callback = (isVisible: boolean) => void;

export class Delayed {
private displayedAt = 0;
private hideDelayMs: number;
private isVisible = false;
private minimumVisibleDuration: number;
private showDelayMs: number;
private timeoutId?: number;

constructor({
minimumVisibleDuration = 1000,
showDelayMs = 50,
hideDelayMs = 50
} = {}) {
this.minimumVisibleDuration = minimumVisibleDuration;
this.hideDelayMs = hideDelayMs;
this.showDelayMs = showDelayMs;
}

private onChangeCallback: Callback = () => null;

private updateState(isVisible: boolean) {
window.clearTimeout(this.timeoutId);
const ms = !isVisible
? Math.max(
this.displayedAt + this.minimumVisibleDuration - Date.now(),
this.hideDelayMs
)
: this.showDelayMs;

this.timeoutId = window.setTimeout(() => {
if (this.isVisible !== isVisible) {
this.isVisible = isVisible;
this.onChangeCallback(isVisible);
if (isVisible) {
this.displayedAt = Date.now();
}
}
}, ms);
}

public show() {
this.updateState(true);
}

public hide() {
this.updateState(false);
}

public onChange(onChangeCallback: Callback) {
this.onChangeCallback = onChangeCallback;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { cleanup, renderHook } from 'react-hooks-testing-library';
import { useDelayedVisibility } from '.';

afterEach(cleanup);

// Suppress warnings about "act" until async/await syntax is supported: https://github.com/facebook/react/issues/14769
/* eslint-disable no-console */
const originalError = console.error;
beforeAll(() => {
console.error = jest.fn();
});
afterAll(() => {
console.error = originalError;
});

describe('useFetcher', () => {
let hook;
beforeEach(() => {
jest.useFakeTimers();
});

it('is initially false', () => {
hook = renderHook(isLoading => useDelayedVisibility(isLoading), {
initialProps: false
});
expect(hook.result.current).toEqual(false);
});

it('does not change to true immediately', () => {
hook = renderHook(isLoading => useDelayedVisibility(isLoading), {
initialProps: false
});

hook.rerender(true);
jest.advanceTimersByTime(10);
expect(hook.result.current).toEqual(false);
jest.advanceTimersByTime(50);
expect(hook.result.current).toEqual(true);
});

it('does not change to false immediately', () => {
hook = renderHook(isLoading => useDelayedVisibility(isLoading), {
initialProps: false
});

hook.rerender(true);
jest.advanceTimersByTime(100);
hook.rerender(false);
expect(hook.result.current).toEqual(true);
});

it('is true for minimum 1000ms', () => {
hook = renderHook(isLoading => useDelayedVisibility(isLoading), {
initialProps: false
});

hook.rerender(true);
jest.advanceTimersByTime(100);
hook.rerender(false);
jest.advanceTimersByTime(900);
expect(hook.result.current).toEqual(true);
jest.advanceTimersByTime(100);
expect(hook.result.current).toEqual(false);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { useEffect, useRef, useState } from 'react';
import { Delayed } from './Delayed';

export function useDelayedVisibility(
visible: boolean,
hideDelayMs?: number,
showDelayMs?: number,
minimumVisibleDuration?: number
) {
const [isVisible, setIsVisible] = useState(false);
const delayedRef = useRef<Delayed | null>(null);

useEffect(
() => {
const delayed = new Delayed({
hideDelayMs,
showDelayMs,
minimumVisibleDuration
});
delayed.onChange(visibility => {
setIsVisible(visibility);
});
delayedRef.current = delayed;
},
[hideDelayMs, showDelayMs, minimumVisibleDuration]
);

useEffect(
() => {
if (!delayedRef.current) {
return;
}

if (visible) {
delayedRef.current.show();
} else {
delayedRef.current.hide();
}
},
[visible]
);

return isVisible;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

export const STATUS = {
FAILURE: 'FAILURE',
SUCCESS: 'SUCCESS',
LOADING: 'LOADING'
};
import { useRef } from 'react';

let uniqueId = 0;
const getUniqueId = () => uniqueId++;

export function useComponentId() {
const idRef = useRef(getUniqueId());
return idRef.current;
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ describe('when simulating race condition', () => {

beforeEach(async () => {
jest.useFakeTimers();
jest
.spyOn(window, 'requestAnimationFrame')
.mockImplementation(cb => cb(0) as any);

renderSpy = jest.fn();
requestCallOrder = [];
Expand Down
Loading

0 comments on commit e97ea46

Please sign in to comment.