1
0
mirror of https://github.com/bitwarden/browser.git synced 2025-02-28 03:21:40 +01:00

[CL-485] Add small delay for async action loading state (#12835)

This commit is contained in:
Vicki League 2025-02-25 09:56:01 -05:00 committed by GitHub
parent d11321e28e
commit 6d1914f43d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 253 additions and 97 deletions

View File

@ -21,30 +21,35 @@ export class BitActionDirective implements OnDestroy {
private destroy$ = new Subject<void>();
private _loading$ = new BehaviorSubject<boolean>(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;
}

View File

@ -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);
}
});
}

View File

@ -1,6 +1,7 @@
import { Meta } from "@storybook/addon-docs";
import { Meta, Story } from "@storybook/addon-docs";
import * as stories from "./standalone.stories.ts";
<Meta title="Component Library/Async Actions/Standalone/Documentation" />
<Meta of={stories} />
# 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
<button bitIconButton="bwi-trash" [bitAction]="handler"></button>`;
```
## Stories
### Promise resolves -- loading spinner is displayed
<Story of={stories.UsingPromise} />
### Promise resolves -- quickly without loading spinner
<Story of={stories.ActionResolvesQuickly} />
### Promise rejects
<Story of={stories.RejectedPromise} />
### Observable
<Story of={stories.UsingObservable} />

View File

@ -11,9 +11,9 @@ import { IconButtonModule } from "../icon-button";
import { BitActionDirective } from "./bit-action.directive";
const template = `
const template = /*html*/ `
<button bitButton buttonType="primary" [bitAction]="action" class="tw-mr-2">
Perform action
Perform action {{ statusEmoji }}
</button>
<button bitIconButton="bwi-trash" buttonType="danger" [bitAction]="action"></button>`;
@ -22,9 +22,30 @@ const template = `
selector: "app-promise-example",
})
class PromiseExampleComponent {
statusEmoji = "🟡";
action = async () => {
await new Promise<void>((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<void>((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: `<app-rejected-promise-example></app-rejected-promise-example>`,
}),
};
export const ActionResolvesQuickly: PromiseStory = {
render: (args) => ({
props: args,
template: `<app-action-resolves-quickly></app-action-resolves-quickly>`,
}),
};

View File

@ -1,10 +1,10 @@
<span class="tw-relative">
<span [ngClass]="{ 'tw-invisible': loading }">
<span [ngClass]="{ 'tw-invisible': showLoadingStyle() }">
<ng-content></ng-content>
</span>
<span
class="tw-absolute tw-inset-0 tw-flex tw-items-center tw-justify-center"
[ngClass]="{ 'tw-invisible': !loading }"
[ngClass]="{ 'tw-invisible': !showLoadingStyle() }"
>
<i class="bwi bwi-spinner bwi-lg bwi-spin" aria-hidden="true"></i>
</span>

View File

@ -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<ButtonType, string[]> = {
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<boolean>(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<boolean>(false);
}

View File

@ -1,10 +1,10 @@
<span class="tw-relative">
<span [ngClass]="{ 'tw-invisible': loading }">
<span [ngClass]="{ 'tw-invisible': showLoadingStyle() }">
<i class="bwi" [ngClass]="iconClass" aria-hidden="true"></i>
</span>
<span
class="tw-absolute tw-inset-0 tw-flex tw-items-center tw-justify-center"
[ngClass]="{ 'tw-invisible': !loading }"
[ngClass]="{ 'tw-invisible': !showLoadingStyle() }"
>
<i
class="bwi bwi-spinner bwi-spin"

View File

@ -1,7 +1,9 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { NgClass } from "@angular/common";
import { Component, ElementRef, HostBinding, Input } from "@angular/core";
import { Component, computed, ElementRef, HostBinding, Input, model } from "@angular/core";
import { toObservable, toSignal } from "@angular/core/rxjs-interop";
import { debounce, interval } from "rxjs";
import { ButtonLikeAbstraction, ButtonType } from "../shared/button-like.abstraction";
import { FocusableElement } from "../shared/focusable-element";
@ -34,9 +36,6 @@ const styles: Record<IconButtonType, string[]> = {
"hover:tw-bg-transparent-hover",
"hover:tw-border-text-contrast",
"focus-visible:before:tw-ring-text-contrast",
"disabled:tw-opacity-60",
"disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent",
...focusRing,
],
main: [
@ -46,9 +45,6 @@ const styles: Record<IconButtonType, string[]> = {
"hover:tw-bg-transparent-hover",
"hover:tw-border-primary-600",
"focus-visible:before:tw-ring-primary-600",
"disabled:!tw-text-secondary-300",
"disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent",
...focusRing,
],
muted: [
@ -60,11 +56,8 @@ const styles: Record<IconButtonType, string[]> = {
"hover:tw-bg-transparent-hover",
"hover:tw-border-primary-600",
"focus-visible:before:tw-ring-primary-600",
"disabled:!tw-text-secondary-300",
"aria-expanded:hover:tw-bg-secondary-700",
"aria-expanded:hover:tw-border-secondary-700",
"disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent",
...focusRing,
],
primary: [
@ -74,9 +67,6 @@ const styles: Record<IconButtonType, string[]> = {
"hover:tw-bg-primary-600",
"hover:tw-border-primary-600",
"focus-visible:before:tw-ring-primary-600",
"disabled:tw-opacity-60",
"disabled:hover:tw-border-primary-600",
"disabled:hover:tw-bg-primary-600",
...focusRing,
],
secondary: [
@ -86,10 +76,6 @@ const styles: Record<IconButtonType, string[]> = {
"hover:!tw-text-contrast",
"hover:tw-bg-text-muted",
"focus-visible:before:tw-ring-primary-600",
"disabled:tw-opacity-60",
"disabled:hover:tw-border-text-muted",
"disabled:hover:tw-bg-transparent",
"disabled:hover:!tw-text-muted",
...focusRing,
],
danger: [
@ -100,10 +86,6 @@ const styles: Record<IconButtonType, string[]> = {
"hover:tw-bg-transparent",
"hover:tw-border-primary-600",
"focus-visible:before:tw-ring-primary-600",
"disabled:!tw-text-secondary-300",
"disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent",
"disabled:hover:!tw-text-secondary-300",
...focusRing,
],
light: [
@ -113,10 +95,48 @@ const styles: Record<IconButtonType, string[]> = {
"hover:tw-bg-transparent-hover",
"hover:tw-border-text-alt2",
"focus-visible:before:tw-ring-text-alt2",
...focusRing,
],
unstyled: [],
};
const disabledStyles: Record<IconButtonType, string[]> = {
contrast: [
"disabled:tw-opacity-60",
"disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent",
],
main: [
"disabled:!tw-text-secondary-300",
"disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent",
],
muted: [
"disabled:!tw-text-secondary-300",
"disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent",
],
primary: [
"disabled:tw-opacity-60",
"disabled:hover:tw-border-primary-600",
"disabled:hover:tw-bg-primary-600",
],
secondary: [
"disabled:tw-opacity-60",
"disabled:hover:tw-border-text-muted",
"disabled:hover:tw-bg-transparent",
"disabled:hover:!tw-text-muted",
],
danger: [
"disabled:!tw-text-secondary-300",
"disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent",
"disabled:hover:!tw-text-secondary-300",
],
light: [
"disabled:tw-opacity-60",
"disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent",
...focusRing,
],
unstyled: [],
};
@ -137,11 +157,14 @@ const sizes: Record<IconButtonSize, string[]> = {
],
standalone: true,
imports: [NgClass],
host: {
"[attr.disabled]": "disabledAttr()",
},
})
export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement {
@Input("bitIconButton") icon: string;
@Input() buttonType: IconButtonType;
@Input() buttonType: IconButtonType = "main";
@Input() size: IconButtonSize = "default";
@ -155,22 +178,51 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE
"hover:tw-no-underline",
"focus:tw-outline-none",
]
.concat(styles[this.buttonType ?? "main"])
.concat(sizes[this.size]);
.concat(styles[this.buttonType])
.concat(sizes[this.size])
.concat(this.showDisabledStyles() || this.disabled() ? disabledStyles[this.buttonType] : []);
}
get iconClass() {
return [this.icon, "!tw-m-0"];
}
@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;
});
@Input() loading = false;
@Input() disabled = false;
/**
* 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);
});
loading = model(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<boolean>(false);
getFocusTarget() {
return this.elementRef.nativeElement;

View File

@ -1,8 +1,10 @@
// FIXME: Update this file to be type safe and remove this and next line
import { ModelSignal } from "@angular/core";
// @ts-strict-ignore
export type ButtonType = "primary" | "secondary" | "danger" | "unstyled";
export abstract class ButtonLikeAbstraction {
loading: boolean;
disabled: boolean;
loading: ModelSignal<boolean>;
disabled: ModelSignal<boolean>;
}

View File

@ -120,11 +120,11 @@ export class SendFormComponent implements AfterViewInit, OnInit, OnChanges, Send
ngAfterViewInit(): void {
if (this.submitBtn) {
this.bitSubmit.loading$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((loading) => {
this.submitBtn.loading = loading;
this.submitBtn.loading.set(loading);
});
this.bitSubmit.disabled$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((disabled) => {
this.submitBtn.disabled = disabled;
this.submitBtn.disabled.set(disabled);
});
}
}

View File

@ -103,7 +103,7 @@ describe("CipherAttachmentsComponent", () => {
fixture = TestBed.createComponent(CipherAttachmentsComponent);
component = fixture.componentInstance;
component.cipherId = "5555-444-3333" as CipherId;
component.submitBtn = {} as ButtonComponent;
component.submitBtn = TestBed.createComponent(ButtonComponent).componentInstance;
fixture.detectChanges();
});
@ -134,34 +134,38 @@ describe("CipherAttachmentsComponent", () => {
describe("bitSubmit", () => {
beforeEach(() => {
component.submitBtn.disabled = undefined;
component.submitBtn.loading = undefined;
component.submitBtn.disabled.set(undefined);
component.submitBtn.loading.set(undefined);
});
it("updates sets initial state of the submit button", async () => {
await component.ngOnInit();
expect(component.submitBtn.disabled).toBe(true);
expect(component.submitBtn.disabled()).toBe(true);
});
it("sets submitBtn loading state", () => {
jest.useFakeTimers();
component.bitSubmit.loading = true;
expect(component.submitBtn.loading).toBe(true);
jest.runAllTimers();
expect(component.submitBtn.loading()).toBe(true);
component.bitSubmit.loading = false;
expect(component.submitBtn.loading).toBe(false);
expect(component.submitBtn.loading()).toBe(false);
});
it("sets submitBtn disabled state", () => {
component.bitSubmit.disabled = true;
expect(component.submitBtn.disabled).toBe(true);
expect(component.submitBtn.disabled()).toBe(true);
component.bitSubmit.disabled = false;
expect(component.submitBtn.disabled).toBe(false);
expect(component.submitBtn.disabled()).toBe(false);
});
});
@ -169,7 +173,7 @@ describe("CipherAttachmentsComponent", () => {
let file: File;
beforeEach(() => {
component.submitBtn.disabled = undefined;
component.submitBtn.disabled.set(undefined);
file = new File([""], "attachment.txt", { type: "text/plain" });
const inputElement = fixture.debugElement.query(By.css("input[type=file]"));
@ -189,7 +193,7 @@ describe("CipherAttachmentsComponent", () => {
});
it("updates disabled state of submit button", () => {
expect(component.submitBtn.disabled).toBe(false);
expect(component.submitBtn.disabled()).toBe(false);
});
});

View File

@ -114,7 +114,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
return;
}
this.submitBtn.disabled = status !== "VALID";
this.submitBtn.disabled.set(status !== "VALID");
});
}
@ -127,7 +127,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
// Update the initial state of the submit button
if (this.submitBtn) {
this.submitBtn.disabled = !this.attachmentForm.valid;
this.submitBtn.disabled.set(!this.attachmentForm.valid);
}
}
@ -137,7 +137,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
return;
}
this.submitBtn.loading = loading;
this.submitBtn.loading.set(loading);
});
this.bitSubmit.disabled$.pipe(takeUntilDestroyed(this.destroy$)).subscribe((disabled) => {
@ -145,7 +145,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
return;
}
this.submitBtn.disabled = disabled;
this.submitBtn.disabled.set(disabled);
});
}

View File

@ -144,11 +144,11 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci
ngAfterViewInit(): void {
if (this.submitBtn) {
this.bitSubmit.loading$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((loading) => {
this.submitBtn.loading = loading;
this.submitBtn.loading.set(loading);
});
this.bitSubmit.disabled$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((disabled) => {
this.submitBtn.disabled = disabled;
this.submitBtn.disabled.set(disabled);
});
}
}

View File

@ -250,6 +250,7 @@ describe("ItemDetailsSectionComponent", () => {
describe("showOwnership", () => {
it("should return true if ownership change is allowed or in edit mode with at least one organization", () => {
component.config.allowPersonalOwnership = true;
jest.spyOn(component, "allowOwnershipChange", "get").mockReturnValue(true);
expect(component.showOwnership).toBe(true);
@ -261,6 +262,7 @@ describe("ItemDetailsSectionComponent", () => {
});
it("should hide the ownership control if showOwnership is false", async () => {
component.config.allowPersonalOwnership = true;
jest.spyOn(component, "showOwnership", "get").mockReturnValue(false);
fixture.detectChanges();
await fixture.whenStable();
@ -271,6 +273,7 @@ describe("ItemDetailsSectionComponent", () => {
});
it("should show the ownership control if showOwnership is true", async () => {
component.config.allowPersonalOwnership = true;
jest.spyOn(component, "allowOwnershipChange", "get").mockReturnValue(true);
fixture.detectChanges();
await fixture.whenStable();

View File

@ -104,7 +104,7 @@ export class AddEditFolderDialogComponent implements AfterViewInit, OnInit {
return;
}
this.submitBtn.loading = loading;
this.submitBtn.loading.set(loading);
});
}

View File

@ -213,7 +213,7 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI
return;
}
this.submitBtn.loading = loading;
this.submitBtn.loading.set(loading);
});
this.bitSubmit.disabled$.pipe(takeUntil(this.destroy$)).subscribe((disabled) => {
@ -221,7 +221,7 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI
return;
}
this.submitBtn.disabled = disabled;
this.submitBtn.disabled.set(disabled);
});
}