From 40757fa7f46f030b1dc53e402ca2095f2590be2e Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Tue, 30 Jan 2024 16:31:38 -0800 Subject: [PATCH] Prevent the status indicator flickering for quick-returning commands (#265) * add status-indicator-visible * save * save * Prevent the status indicator flickering for quick returns * flx regression * reduce delay, reset spinnerVisible when there's no more running commands * clean up code reuse * move code around * slight optimizations to prevent rendering before spinner is visible * rename var * revert shouldSync change as it broke the sync --- src/app/common/icons/icons.less | 22 ++++++++- src/app/common/icons/icons.tsx | 73 +++++++++++++++++++++++++++--- src/app/sidebar/sidebar.less | 2 +- src/app/workspace/screen/tabs.less | 2 +- 4 files changed, 89 insertions(+), 10 deletions(-) diff --git a/src/app/common/icons/icons.less b/src/app/common/icons/icons.less index 74069f05d..800d18856 100644 --- a/src/app/common/icons/icons.less +++ b/src/app/common/icons/icons.less @@ -48,13 +48,33 @@ } } +/* +The following accounts for a debounce in the status indicator. We will only display the status indicator icon if the parent indicates that it should be visible AND one of the following is true: +1. There is a status to be shown. +2. There is a spinner to be shown and the required delay has passed. +*/ +.status-indicator-visible { + &.spinner-visible, + &.output, + &.error, + &.success { + .positional-icon-visible; + } + + // This is set by the timeout in the status indicator component. + &.spinner-visible { + #spinner { + visibility: visible; + } + } +} + .status-indicator { #spinner, #indicator { visibility: hidden; } .spin #spinner { - visibility: visible; stroke: @term-white; } &.error #indicator { diff --git a/src/app/common/icons/icons.tsx b/src/app/common/icons/icons.tsx index 24d32a33b..e437b0447 100644 --- a/src/app/common/icons/icons.tsx +++ b/src/app/common/icons/icons.tsx @@ -3,6 +3,8 @@ import { StatusIndicatorLevel } from "../../../types/types"; import cn from "classnames"; import { ReactComponent as SpinnerIndicator } from "../../assets/icons/spinner-indicator.svg"; import { boundMethod } from "autobind-decorator"; +import * as mobx from "mobx"; +import * as mobxReact from "mobx-react"; import { ReactComponent as RotateIconSvg } from "../../assets/icons/line/rotate.svg"; @@ -126,33 +128,90 @@ class SyncSpin extends React.Component<{ } interface StatusIndicatorProps { + /** + * The level of the status indicator. This will determine the color of the status indicator. + */ level: StatusIndicatorLevel; className?: string; + /** + * If true, a spinner will be shown around the status indicator. + */ runningCommands?: boolean; } +/** + * This component is used to show the status of a command. It will show a spinner around the status indicator if there are running commands. It will also delay showing the spinner for a short time to prevent flickering. + */ +@mobxReact.observer export class StatusIndicator extends React.Component { iconRef: React.RefObject = React.createRef(); + spinnerVisible: mobx.IObservableValue = mobx.observable.box(false); + timeout: NodeJS.Timeout; + + clearSpinnerTimeout() { + if (this.timeout) { + clearTimeout(this.timeout); + this.timeout = null; + } + mobx.action(() => { + this.spinnerVisible.set(false); + })(); + } + + /** + * This will apply a delay after there is a running command before showing the spinner. This prevents flickering for commands that return quickly. + */ + updateMountCallback() { + const runningCommands = this.props.runningCommands ?? false; + if (runningCommands && !this.timeout) { + this.timeout = setTimeout( + mobx.action(() => { + this.spinnerVisible.set(true); + }), + 100 + ); + } else if (!runningCommands) { + this.clearSpinnerTimeout(); + } + } + + componentDidUpdate(): void { + this.updateMountCallback(); + } + + componentDidMount(): void { + this.updateMountCallback(); + } + + componentWillUnmount(): void { + this.clearSpinnerTimeout(); + } render() { const { level, className, runningCommands } = this.props; + const spinnerVisible = this.spinnerVisible.get(); let statusIndicator = null; - if (level != StatusIndicatorLevel.None || runningCommands) { - let levelClass = null; + if (level != StatusIndicatorLevel.None || spinnerVisible) { + let indicatorLevelClass = null; switch (level) { case StatusIndicatorLevel.Output: - levelClass = "output"; + indicatorLevelClass = "output"; break; case StatusIndicatorLevel.Success: - levelClass = "success"; + indicatorLevelClass = "success"; break; case StatusIndicatorLevel.Error: - levelClass = "error"; + indicatorLevelClass = "error"; break; } + + const spinnerVisibleClass = spinnerVisible ? "spinner-visible" : null; statusIndicator = ( - - + + ); } diff --git a/src/app/sidebar/sidebar.less b/src/app/sidebar/sidebar.less index 80c5353b8..3946ab948 100644 --- a/src/app/sidebar/sidebar.less +++ b/src/app/sidebar/sidebar.less @@ -188,7 +188,7 @@ } &:not(:hover) .status-indicator { - .positional-icon-visible; + .status-indicator-visible; } &.workspaces { diff --git a/src/app/workspace/screen/tabs.less b/src/app/workspace/screen/tabs.less index e88122ad9..3fdc1a572 100644 --- a/src/app/workspace/screen/tabs.less +++ b/src/app/workspace/screen/tabs.less @@ -287,7 +287,7 @@ } &:not(:hover) .status-indicator { - .positional-icon-visible; + .status-indicator-visible; } &:hover {