From 6d1914f43db8af28f97d75ad40a0af6e15c2b530 Mon Sep 17 00:00:00 2001 From: Vicki League Date: Tue, 25 Feb 2025 09:56:01 -0500 Subject: [PATCH] [CL-485] Add small delay for async action loading state (#12835) --- .../src/async-actions/bit-action.directive.ts | 29 +++-- .../async-actions/form-button.directive.ts | 6 +- .../src/async-actions/standalone.mdx | 29 ++++- .../src/async-actions/standalone.stories.ts | 35 +++++- .../src/button/button.component.html | 4 +- .../components/src/button/button.component.ts | 72 ++++++++--- .../icon-button/icon-button.component.html | 4 +- .../src/icon-button/icon-button.component.ts | 116 +++++++++++++----- .../src/shared/button-like.abstraction.ts | 6 +- .../components/send-form.component.ts | 4 +- .../cipher-attachments.component.spec.ts | 24 ++-- .../cipher-attachments.component.ts | 8 +- .../components/cipher-form.component.ts | 4 +- .../item-details-section.component.spec.ts | 3 + .../add-edit-folder-dialog.component.ts | 2 +- .../assign-collections.component.ts | 4 +- 16 files changed, 253 insertions(+), 97 deletions(-) diff --git a/libs/components/src/async-actions/bit-action.directive.ts b/libs/components/src/async-actions/bit-action.directive.ts index 3e793ae2ec..ac50082852 100644 --- a/libs/components/src/async-actions/bit-action.directive.ts +++ b/libs/components/src/async-actions/bit-action.directive.ts @@ -21,30 +21,35 @@ export class BitActionDirective implements OnDestroy { private destroy$ = new Subject(); private _loading$ = new BehaviorSubject(false); - disabled = false; - - @Input("bitAction") handler: FunctionReturningAwaitable; - + /** + * Observable of loading behavior subject + * + * Used in `form-button.directive.ts` + */ readonly loading$ = this._loading$.asObservable(); - constructor( - private buttonComponent: ButtonLikeAbstraction, - @Optional() private validationService?: ValidationService, - @Optional() private logService?: LogService, - ) {} - get loading() { return this._loading$.value; } set loading(value: boolean) { this._loading$.next(value); - this.buttonComponent.loading = value; + this.buttonComponent.loading.set(value); } + disabled = false; + + @Input("bitAction") handler: FunctionReturningAwaitable; + + constructor( + private buttonComponent: ButtonLikeAbstraction, + @Optional() private validationService?: ValidationService, + @Optional() private logService?: LogService, + ) {} + @HostListener("click") protected async onClick() { - if (!this.handler || this.loading || this.disabled || this.buttonComponent.disabled) { + if (!this.handler || this.loading || this.disabled || this.buttonComponent.disabled()) { return; } diff --git a/libs/components/src/async-actions/form-button.directive.ts b/libs/components/src/async-actions/form-button.directive.ts index 7c92865b98..1c2855f32e 100644 --- a/libs/components/src/async-actions/form-button.directive.ts +++ b/libs/components/src/async-actions/form-button.directive.ts @@ -41,15 +41,15 @@ export class BitFormButtonDirective implements OnDestroy { if (submitDirective && buttonComponent) { submitDirective.loading$.pipe(takeUntil(this.destroy$)).subscribe((loading) => { if (this.type === "submit") { - buttonComponent.loading = loading; + buttonComponent.loading.set(loading); } else { - buttonComponent.disabled = this.disabled || loading; + buttonComponent.disabled.set(this.disabled || loading); } }); submitDirective.disabled$.pipe(takeUntil(this.destroy$)).subscribe((disabled) => { if (this.disabled !== false) { - buttonComponent.disabled = this.disabled || disabled; + buttonComponent.disabled.set(this.disabled || disabled); } }); } diff --git a/libs/components/src/async-actions/standalone.mdx b/libs/components/src/async-actions/standalone.mdx index efde494f2d..f484ea01c5 100644 --- a/libs/components/src/async-actions/standalone.mdx +++ b/libs/components/src/async-actions/standalone.mdx @@ -1,6 +1,7 @@ -import { Meta } from "@storybook/addon-docs"; +import { Meta, Story } from "@storybook/addon-docs"; +import * as stories from "./standalone.stories.ts"; - + # Standalone Async Actions @@ -8,9 +9,13 @@ These directives should be used when building a standalone button that triggers in the background, eg. Refresh buttons. For non-submit buttons that are associated with forms see [Async Actions In Forms](?path=/story/component-library-async-actions-in-forms-documentation--page). +If the long running background task resolves quickly (e.g. less than 75 ms), the loading spinner +will not display on the button. This prevents an undesirable "flicker" of the loading spinner when +it is not necessary for the user to see it. + ## Usage -Adding async actions to standalone buttons requires the following 2 steps +Adding async actions to standalone buttons requires the following 2 steps: ### 1. Add a handler to your `Component` @@ -60,3 +65,21 @@ from how click handlers are usually defined with the output syntax `(click)="han `; ``` + +## Stories + +### Promise resolves -- loading spinner is displayed + + + +### Promise resolves -- quickly without loading spinner + + + +### Promise rejects + + + +### Observable + + diff --git a/libs/components/src/async-actions/standalone.stories.ts b/libs/components/src/async-actions/standalone.stories.ts index f658dfb0f0..52b85b8856 100644 --- a/libs/components/src/async-actions/standalone.stories.ts +++ b/libs/components/src/async-actions/standalone.stories.ts @@ -11,9 +11,9 @@ import { IconButtonModule } from "../icon-button"; import { BitActionDirective } from "./bit-action.directive"; -const template = ` +const template = /*html*/ ` `; @@ -22,9 +22,30 @@ const template = ` selector: "app-promise-example", }) class PromiseExampleComponent { + statusEmoji = "🟡"; action = async () => { await new Promise((resolve, reject) => { - setTimeout(resolve, 2000); + setTimeout(() => { + resolve(); + this.statusEmoji = "🟢"; + }, 5000); + }); + }; +} + +@Component({ + template, + selector: "app-action-resolves-quickly", +}) +class ActionResolvesQuicklyComponent { + statusEmoji = "🟡"; + + action = async () => { + await new Promise((resolve, reject) => { + setTimeout(() => { + resolve(); + this.statusEmoji = "🟢"; + }, 50); }); }; } @@ -59,6 +80,7 @@ export default { PromiseExampleComponent, ObservableExampleComponent, RejectedPromiseExampleComponent, + ActionResolvesQuicklyComponent, ], imports: [ButtonModule, IconButtonModule, BitActionDirective], providers: [ @@ -100,3 +122,10 @@ export const RejectedPromise: ObservableStory = { template: ``, }), }; + +export const ActionResolvesQuickly: PromiseStory = { + render: (args) => ({ + props: args, + template: ``, + }), +}; diff --git a/libs/components/src/button/button.component.html b/libs/components/src/button/button.component.html index ee4d150dfc..a07ab9fb99 100644 --- a/libs/components/src/button/button.component.html +++ b/libs/components/src/button/button.component.html @@ -1,10 +1,10 @@ - + diff --git a/libs/components/src/button/button.component.ts b/libs/components/src/button/button.component.ts index 96311f9152..0b4ce3073c 100644 --- a/libs/components/src/button/button.component.ts +++ b/libs/components/src/button/button.component.ts @@ -2,7 +2,9 @@ // @ts-strict-ignore import { coerceBooleanProperty } from "@angular/cdk/coercion"; import { NgClass } from "@angular/common"; -import { Input, HostBinding, Component } from "@angular/core"; +import { Input, HostBinding, Component, model, computed } from "@angular/core"; +import { toObservable, toSignal } from "@angular/core/rxjs-interop"; +import { debounce, interval } from "rxjs"; import { ButtonLikeAbstraction, ButtonType } from "../shared/button-like.abstraction"; @@ -49,6 +51,9 @@ const buttonStyles: Record = { providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }], standalone: true, imports: [NgClass], + host: { + "[attr.disabled]": "disabledAttr()", + }, }) export class ButtonComponent implements ButtonLikeAbstraction { @HostBinding("class") get classList() { @@ -64,24 +69,41 @@ export class ButtonComponent implements ButtonLikeAbstraction { "tw-no-underline", "hover:tw-no-underline", "focus:tw-outline-none", - "disabled:tw-bg-secondary-300", - "disabled:hover:tw-bg-secondary-300", - "disabled:tw-border-secondary-300", - "disabled:hover:tw-border-secondary-300", - "disabled:!tw-text-muted", - "disabled:hover:!tw-text-muted", - "disabled:tw-cursor-not-allowed", - "disabled:hover:tw-no-underline", ] .concat(this.block ? ["tw-w-full", "tw-block"] : ["tw-inline-block"]) - .concat(buttonStyles[this.buttonType ?? "secondary"]); + .concat(buttonStyles[this.buttonType ?? "secondary"]) + .concat( + this.showDisabledStyles() || this.disabled() + ? [ + "disabled:tw-bg-secondary-300", + "disabled:hover:tw-bg-secondary-300", + "disabled:tw-border-secondary-300", + "disabled:hover:tw-border-secondary-300", + "disabled:!tw-text-muted", + "disabled:hover:!tw-text-muted", + "disabled:tw-cursor-not-allowed", + "disabled:hover:tw-no-underline", + ] + : [], + ); } - @HostBinding("attr.disabled") - get disabledAttr() { - const disabled = this.disabled != null && this.disabled !== false; - return disabled || this.loading ? true : null; - } + protected disabledAttr = computed(() => { + const disabled = this.disabled() != null && this.disabled() !== false; + return disabled || this.loading() ? true : null; + }); + + /** + * Determine whether it is appropriate to display the disabled styles. We only want to show + * the disabled styles if the button is truly disabled, or if the loading styles are also + * visible. + * + * We can't use `disabledAttr` for this, because it returns `true` when `loading` is `true`. + * We only want to show disabled styles during loading if `showLoadingStyles` is `true`. + */ + protected showDisabledStyles = computed(() => { + return this.showLoadingStyle() || (this.disabledAttr() && this.loading() === false); + }); @Input() buttonType: ButtonType; @@ -96,7 +118,23 @@ export class ButtonComponent implements ButtonLikeAbstraction { this._block = coerceBooleanProperty(value); } - @Input() loading = false; + loading = model(false); - @Input() disabled = false; + /** + * Determine whether it is appropriate to display a loading spinner. We only want to show + * a spinner if it's been more than 75 ms since the `loading` state began. This prevents + * a spinner "flash" for actions that are synchronous/nearly synchronous. + * + * We can't use `loading` for this, because we still need to disable the button during + * the full `loading` state. I.e. we only want the spinner to be debounced, not the + * loading state. + * + * This pattern of converting a signal to an observable and back to a signal is not + * recommended. TODO -- find better way to use debounce with signals (CL-596) + */ + protected showLoadingStyle = toSignal( + toObservable(this.loading).pipe(debounce((isLoading) => interval(isLoading ? 75 : 0))), + ); + + disabled = model(false); } diff --git a/libs/components/src/icon-button/icon-button.component.html b/libs/components/src/icon-button/icon-button.component.html index 6eeaaaffaf..0145f0b0ba 100644 --- a/libs/components/src/icon-button/icon-button.component.html +++ b/libs/components/src/icon-button/icon-button.component.html @@ -1,10 +1,10 @@ - +