From 682f1f83d9c211e886d6f05fc557eddb25c4c5c2 Mon Sep 17 00:00:00 2001 From: Bryan Cunningham Date: Tue, 8 Jul 2025 16:13:25 -0400 Subject: [PATCH] [CL-295] Use aria-disabled on buttons (#15009) * Use aria-disabled for button disabled state * remove import from testing story * use aria-disabled attr on bitLink button * remove unnecessary story attrs * remove disabled attr if on button element * create caprture click util * use caprture click util and fix tests * fix lint errors * fix event type * combine click capture and attr modification * fix lint error. Commit spec changes left out of last commit in error * inject element ref * move aria-disabled styles to common * move disabled logic into util * fix broken async actions stories * fix broken tests asserting disabled attr * have test check for string true vlalue * fix Signal type * fix form-field story import * remove injector left in error * aria-disable icon buttons * update form component css selector to look for aria-disabled buttons * use correct types. pass nativeElement directly * add JSDoc comment for util function --------- Co-authored-by: Will Martin --- .../vault-generator-dialog.component.spec.ts | 18 ++--- .../web-generator-dialog.component.spec.ts | 18 ++--- .../src/async-actions/in-forms.stories.ts | 18 ++++- .../src/async-actions/standalone.stories.ts | 12 +++- .../src/button/button.component.spec.ts | 14 ++-- .../components/src/button/button.component.ts | 41 ++++++++---- .../src/form-field/form-field.component.html | 2 +- .../src/form-field/form-field.stories.ts | 1 + .../src/icon-button/icon-button.component.ts | 67 ++++++++++++------- libs/components/src/link/link.directive.ts | 27 +++++++- .../src/utils/aria-disable-element.ts | 29 ++++++++ libs/components/src/utils/index.ts | 1 + 12 files changed, 175 insertions(+), 73 deletions(-) create mode 100644 libs/components/src/utils/aria-disable-element.ts diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-generator-dialog/vault-generator-dialog.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-generator-dialog/vault-generator-dialog.component.spec.ts index b5d35e2005e..b65138dac3a 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-generator-dialog/vault-generator-dialog.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-generator-dialog/vault-generator-dialog.component.spec.ts @@ -76,10 +76,8 @@ describe("VaultGeneratorDialogComponent", () => { component.onValueGenerated("test-password"); fixture.detectChanges(); - const button = fixture.debugElement.query( - By.css("[data-testid='select-button']"), - ).nativeElement; - expect(button.disabled).toBe(false); + const button = fixture.debugElement.query(By.css("[data-testid='select-button']")); + expect(button.attributes["aria-disabled"]).toBe(undefined); }); it("should disable the button if no value has been generated", () => { @@ -90,10 +88,8 @@ describe("VaultGeneratorDialogComponent", () => { generator.algorithmSelected.emit({ useGeneratedValue: "Use Password" } as any); fixture.detectChanges(); - const button = fixture.debugElement.query( - By.css("[data-testid='select-button']"), - ).nativeElement; - expect(button.disabled).toBe(true); + const button = fixture.debugElement.query(By.css("[data-testid='select-button']")); + expect(button.attributes["aria-disabled"]).toBe("true"); }); it("should disable the button if no algorithm is selected", () => { @@ -104,10 +100,8 @@ describe("VaultGeneratorDialogComponent", () => { generator.valueGenerated.emit("test-password"); fixture.detectChanges(); - const button = fixture.debugElement.query( - By.css("[data-testid='select-button']"), - ).nativeElement; - expect(button.disabled).toBe(true); + const button = fixture.debugElement.query(By.css("[data-testid='select-button']")); + expect(button.attributes["aria-disabled"]).toBe("true"); }); it("should update button text when algorithm is selected", () => { diff --git a/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.spec.ts b/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.spec.ts index 085a3d0d4b0..afb32738901 100644 --- a/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.spec.ts +++ b/apps/web/src/app/vault/components/web-generator-dialog/web-generator-dialog.component.spec.ts @@ -70,10 +70,8 @@ describe("WebVaultGeneratorDialogComponent", () => { generator.valueGenerated.emit("test-password"); fixture.detectChanges(); - const button = fixture.debugElement.query( - By.css("[data-testid='select-button']"), - ).nativeElement; - expect(button.disabled).toBe(false); + const button = fixture.debugElement.query(By.css("[data-testid='select-button']")); + expect(button.attributes["aria-disabled"]).toBe(undefined); }); it("should disable the button if no value has been generated", () => { @@ -84,10 +82,8 @@ describe("WebVaultGeneratorDialogComponent", () => { generator.algorithmSelected.emit({ useGeneratedValue: "Use Password" } as any); fixture.detectChanges(); - const button = fixture.debugElement.query( - By.css("[data-testid='select-button']"), - ).nativeElement; - expect(button.disabled).toBe(true); + const button = fixture.debugElement.query(By.css("[data-testid='select-button']")); + expect(button.attributes["aria-disabled"]).toBe("true"); }); it("should disable the button if no algorithm is selected", () => { @@ -98,10 +94,8 @@ describe("WebVaultGeneratorDialogComponent", () => { generator.valueGenerated.emit("test-password"); fixture.detectChanges(); - const button = fixture.debugElement.query( - By.css("[data-testid='select-button']"), - ).nativeElement; - expect(button.disabled).toBe(true); + const button = fixture.debugElement.query(By.css("[data-testid='select-button']")); + expect(button.attributes["aria-disabled"]).toBe("true"); }); it("should close with selected value when confirmed", () => { diff --git a/libs/components/src/async-actions/in-forms.stories.ts b/libs/components/src/async-actions/in-forms.stories.ts index 857a23227f5..7f51a8bdad2 100644 --- a/libs/components/src/async-actions/in-forms.stories.ts +++ b/libs/components/src/async-actions/in-forms.stories.ts @@ -13,6 +13,7 @@ import { IconButtonModule } from "../icon-button"; import { InputModule } from "../input/input.module"; import { I18nMockService } from "../utils/i18n-mock.service"; +import { AsyncActionsModule } from "./async-actions.module"; import { BitActionDirective } from "./bit-action.directive"; import { BitSubmitDirective } from "./bit-submit.directive"; import { BitFormButtonDirective } from "./form-button.directive"; @@ -40,6 +41,13 @@ const template = ` @Component({ selector: "app-promise-example", template, + imports: [ + AsyncActionsModule, + ButtonModule, + FormFieldModule, + IconButtonModule, + ReactiveFormsModule, + ], }) class PromiseExampleComponent { formObj = this.formBuilder.group({ @@ -77,6 +85,13 @@ class PromiseExampleComponent { @Component({ selector: "app-observable-example", template, + imports: [ + AsyncActionsModule, + ButtonModule, + FormFieldModule, + IconButtonModule, + ReactiveFormsModule, + ], }) class ObservableExampleComponent { formObj = this.formBuilder.group({ @@ -109,7 +124,6 @@ export default { title: "Component Library/Async Actions/In Forms", decorators: [ moduleMetadata({ - declarations: [PromiseExampleComponent, ObservableExampleComponent], imports: [ BitSubmitDirective, BitFormButtonDirective, @@ -120,6 +134,8 @@ export default { ButtonModule, IconButtonModule, BitActionDirective, + PromiseExampleComponent, + ObservableExampleComponent, ], providers: [ { diff --git a/libs/components/src/async-actions/standalone.stories.ts b/libs/components/src/async-actions/standalone.stories.ts index d6f7f978bd5..542825eb17a 100644 --- a/libs/components/src/async-actions/standalone.stories.ts +++ b/libs/components/src/async-actions/standalone.stories.ts @@ -9,6 +9,7 @@ import { ValidationService } from "@bitwarden/common/platform/abstractions/valid import { ButtonModule } from "../button"; import { IconButtonModule } from "../icon-button"; +import { AsyncActionsModule } from "./async-actions.module"; import { BitActionDirective } from "./bit-action.directive"; const template = /*html*/ ` @@ -20,6 +21,8 @@ const template = /*html*/ ` @Component({ template, selector: "app-promise-example", + imports: [AsyncActionsModule, ButtonModule, IconButtonModule], + standalone: true, }) class PromiseExampleComponent { statusEmoji = "🟡"; @@ -36,6 +39,7 @@ class PromiseExampleComponent { @Component({ template, selector: "app-action-resolves-quickly", + imports: [AsyncActionsModule, ButtonModule, IconButtonModule], }) class ActionResolvesQuicklyComponent { statusEmoji = "🟡"; @@ -53,6 +57,7 @@ class ActionResolvesQuicklyComponent { @Component({ template, selector: "app-observable-example", + imports: [AsyncActionsModule, ButtonModule, IconButtonModule], }) class ObservableExampleComponent { action = () => { @@ -63,6 +68,7 @@ class ObservableExampleComponent { @Component({ template, selector: "app-rejected-promise-example", + imports: [AsyncActionsModule, ButtonModule, IconButtonModule], }) class RejectedPromiseExampleComponent { action = async () => { @@ -76,13 +82,15 @@ export default { title: "Component Library/Async Actions/Standalone", decorators: [ moduleMetadata({ - declarations: [ + imports: [ + ButtonModule, + IconButtonModule, + BitActionDirective, PromiseExampleComponent, ObservableExampleComponent, RejectedPromiseExampleComponent, ActionResolvesQuicklyComponent, ], - imports: [ButtonModule, IconButtonModule, BitActionDirective], providers: [ { provide: ValidationService, diff --git a/libs/components/src/button/button.component.spec.ts b/libs/components/src/button/button.component.spec.ts index 6ddbc172803..1651b6cf12a 100644 --- a/libs/components/src/button/button.component.spec.ts +++ b/libs/components/src/button/button.component.spec.ts @@ -34,23 +34,25 @@ describe("Button", () => { expect(buttonDebugElement.nativeElement.disabled).toBeFalsy(); }); - it("should be disabled when disabled is true", () => { + it("should be aria-disabled and not html attribute disabled when disabled is true", () => { testAppComponent.disabled = true; fixture.detectChanges(); - - expect(buttonDebugElement.nativeElement.disabled).toBeTruthy(); + expect(buttonDebugElement.attributes["aria-disabled"]).toBe("true"); + expect(buttonDebugElement.nativeElement.disabled).toBeFalsy(); // Anchor tags cannot be disabled. }); - it("should be disabled when attribute disabled is true", () => { - expect(disabledButtonDebugElement.nativeElement.disabled).toBeTruthy(); + it("should be aria-disabled not html attribute disabled when attribute disabled is true", () => { + fixture.detectChanges(); + expect(disabledButtonDebugElement.attributes["aria-disabled"]).toBe("true"); + expect(disabledButtonDebugElement.nativeElement.disabled).toBeFalsy(); }); it("should be disabled when loading is true", () => { testAppComponent.loading = true; fixture.detectChanges(); - expect(buttonDebugElement.nativeElement.disabled).toBeTruthy(); + expect(buttonDebugElement.attributes["aria-disabled"]).toBe("true"); }); }); diff --git a/libs/components/src/button/button.component.ts b/libs/components/src/button/button.component.ts index 011360db867..671b1dfb96d 100644 --- a/libs/components/src/button/button.component.ts +++ b/libs/components/src/button/button.component.ts @@ -1,10 +1,21 @@ import { coerceBooleanProperty } from "@angular/cdk/coercion"; import { NgClass } from "@angular/common"; -import { Input, HostBinding, Component, model, computed, input } from "@angular/core"; +import { + Input, + HostBinding, + Component, + model, + computed, + input, + ElementRef, + inject, + Signal, +} from "@angular/core"; import { toObservable, toSignal } from "@angular/core/rxjs-interop"; import { debounce, interval } from "rxjs"; import { ButtonLikeAbstraction, ButtonType, ButtonSize } from "../shared/button-like.abstraction"; +import { ariaDisableElement } from "../utils"; const focusRing = [ "focus-visible:tw-ring-2", @@ -52,7 +63,7 @@ const buttonStyles: Record = { providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }], imports: [NgClass], host: { - "[attr.disabled]": "disabledAttr()", + "[attr.aria-disabled]": "disabledAttr()", }, }) export class ButtonComponent implements ButtonLikeAbstraction { @@ -69,27 +80,28 @@ export class ButtonComponent implements ButtonLikeAbstraction { "focus:tw-outline-none", ] .concat(this.block ? ["tw-w-full", "tw-block"] : ["tw-inline-block"]) - .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", + "aria-disabled:!tw-bg-secondary-300", + "hover:tw-bg-secondary-300", + "aria-disabled:tw-border-secondary-300", + "hover:tw-border-secondary-300", + "aria-disabled:!tw-text-muted", + "hover:!tw-text-muted", + "aria-disabled:tw-cursor-not-allowed", + "hover:tw-no-underline", + "aria-disabled:tw-pointer-events-none", ] : [], ) + .concat(buttonStyles[this.buttonType ?? "secondary"]) .concat(buttonSizeStyles[this.size() || "default"]); } protected disabledAttr = computed(() => { const disabled = this.disabled() != null && this.disabled() !== false; - return disabled || this.loading() ? true : null; + return disabled || this.loading() ? true : undefined; }); /** @@ -138,4 +150,9 @@ export class ButtonComponent implements ButtonLikeAbstraction { ); disabled = model(false); + private el = inject(ElementRef); + + constructor() { + ariaDisableElement(this.el.nativeElement, this.disabledAttr as Signal); + } } diff --git a/libs/components/src/form-field/form-field.component.html b/libs/components/src/form-field/form-field.component.html index c4fd018b3ba..ccea0546f3a 100644 --- a/libs/components/src/form-field/form-field.component.html +++ b/libs/components/src/form-field/form-field.component.html @@ -46,7 +46,7 @@
= { const disabledStyles: Record = { contrast: [ - "disabled:tw-opacity-60", - "disabled:hover:tw-border-transparent", - "disabled:hover:tw-bg-transparent", + "aria-disabled:tw-opacity-60", + "aria-disabled:hover:tw-border-transparent", + "aria-disabled:hover:tw-bg-transparent", ], main: [ - "disabled:!tw-text-secondary-300", - "disabled:hover:tw-border-transparent", - "disabled:hover:tw-bg-transparent", + "aria-disabled:!tw-text-secondary-300", + "aria-disabled:hover:tw-border-transparent", + "aria-disabled:hover:tw-bg-transparent", ], muted: [ - "disabled:!tw-text-secondary-300", - "disabled:hover:tw-border-transparent", - "disabled:hover:tw-bg-transparent", + "aria-disabled:!tw-text-secondary-300", + "aria-disabled:hover:tw-border-transparent", + "aria-disabled:hover:tw-bg-transparent", ], primary: [ - "disabled:tw-opacity-60", - "disabled:hover:tw-border-primary-600", - "disabled:hover:tw-bg-primary-600", + "aria-disabled:tw-opacity-60", + "aria-disabled:hover:tw-border-primary-600", + "aria-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", + "aria-disabled:tw-opacity-60", + "aria-disabled:hover:tw-border-text-muted", + "aria-disabled:hover:tw-bg-transparent", + "aria-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", + "aria-disabled:!tw-text-secondary-300", + "aria-disabled:hover:tw-border-transparent", + "aria-disabled:hover:tw-bg-transparent", + "aria-disabled:hover:!tw-text-secondary-300", ], light: [ - "disabled:tw-opacity-60", - "disabled:hover:tw-border-transparent", - "disabled:hover:tw-bg-transparent", + "aria-disabled:tw-opacity-60", + "aria-disabled:hover:tw-border-transparent", + "aria-disabled:hover:tw-bg-transparent", ], unstyled: [], }; @@ -163,7 +173,7 @@ const sizes: Record = { ], imports: [NgClass], host: { - "[attr.disabled]": "disabledAttr()", + "[attr.aria-disabled]": "disabledAttr()", }, }) export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement { @@ -233,5 +243,10 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE return this.elementRef.nativeElement; } - constructor(private elementRef: ElementRef) {} + private elementRef = inject(ElementRef); + + constructor() { + const element = this.elementRef.nativeElement; + ariaDisableElement(element, this.disabledAttr as Signal); + } } diff --git a/libs/components/src/link/link.directive.ts b/libs/components/src/link/link.directive.ts index ad9c94b7831..1a653fd1c83 100644 --- a/libs/components/src/link/link.directive.ts +++ b/libs/components/src/link/link.directive.ts @@ -1,4 +1,14 @@ -import { Input, HostBinding, Directive } from "@angular/core"; +import { + Input, + HostBinding, + Directive, + inject, + ElementRef, + input, + booleanAttribute, +} from "@angular/core"; + +import { ariaDisableElement } from "../utils"; export type LinkType = "primary" | "secondary" | "contrast" | "light"; @@ -58,6 +68,11 @@ const commonStyles = [ "before:tw-transition", "focus-visible:before:tw-ring-2", "focus-visible:tw-z-10", + "aria-disabled:tw-no-underline", + "aria-disabled:tw-pointer-events-none", + "aria-disabled:!tw-text-secondary-300", + "aria-disabled:hover:!tw-text-secondary-300", + "aria-disabled:hover:tw-no-underline", ]; @Directive() @@ -89,9 +104,19 @@ export class AnchorLinkDirective extends LinkDirective { selector: "button[bitLink]", }) export class ButtonLinkDirective extends LinkDirective { + private el = inject(ElementRef); + + disabled = input(false, { transform: booleanAttribute }); + @HostBinding("class") get classList() { return ["before:-tw-inset-y-[0.25rem]"] .concat(commonStyles) .concat(linkStyles[this.linkType] ?? []); } + + constructor() { + super(); + + ariaDisableElement(this.el.nativeElement, this.disabled); + } } diff --git a/libs/components/src/utils/aria-disable-element.ts b/libs/components/src/utils/aria-disable-element.ts new file mode 100644 index 00000000000..f7e02f2cdd1 --- /dev/null +++ b/libs/components/src/utils/aria-disable-element.ts @@ -0,0 +1,29 @@ +import { Signal, effect } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { fromEvent } from "rxjs"; + +/** + * a11y helper util used to `aria-disable` elements as opposed to using the HTML `disabled` attr. + * - Removes HTML `disabled` attr and replaces it with `aria-disabled="true"` + * - Captures click events and prevents them from propagating + */ +export function ariaDisableElement(element: HTMLElement, isDisabled: Signal) { + effect(() => { + if (element.hasAttribute("disabled") || isDisabled()) { + // Remove native disabled and set aria-disabled. Capture click event + element.removeAttribute("disabled"); + + element.setAttribute("aria-disabled", "true"); + } + }); + + fromEvent(element, "click") + .pipe(takeUntilDestroyed()) + .subscribe((event: Event) => { + if (isDisabled()) { + event.stopPropagation(); + event.preventDefault(); + return false; + } + }); +} diff --git a/libs/components/src/utils/index.ts b/libs/components/src/utils/index.ts index afadd6b3b41..91fa71cf0e0 100644 --- a/libs/components/src/utils/index.ts +++ b/libs/components/src/utils/index.ts @@ -1,2 +1,3 @@ +export * from "./aria-disable-element"; export * from "./function-to-observable"; export * from "./i18n-mock.service";