From b0f46004ff0ef30323ff4a181178eae752543020 Mon Sep 17 00:00:00 2001 From: Bryan Cunningham Date: Thu, 21 Aug 2025 09:14:08 -0400 Subject: [PATCH] [CL-796] unrevert aria disabled buttons (#15924) * 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 * WIP * WIP * inject service in directive * remove console log * remove disabled attr left in error * update comments * remove unnecessary logic * remove :disabled psuedo selector as its apparently not needed * fix event type * coerce disabled attr to boolean * remove duplicate style concat left by conflict resolution * add back buttonStyles default * move reactive logic back to helper * add test to ensure menu button doesn't open when trigger is disabled * remove menu toggle to fix tests * remove disabled menu story * Fix usage of bitLink in verify email component * Update varaible name * no longer pass destroyRef --- .../vault-generator-dialog.component.spec.ts | 18 +++----- .../auth/settings/verify-email.component.html | 3 +- .../web-generator-dialog.component.spec.ts | 18 +++----- .../src/a11y/aria-disable.directive.ts | 12 ++++++ .../aria-disabled-click-capture.service.ts | 30 ++++++++++++++ libs/components/src/a11y/index.ts | 2 + .../src/button/button.component.spec.ts | 14 ++++--- .../components/src/button/button.component.ts | 41 +++++++++++++------ .../src/form-field/form-field.component.html | 2 +- .../src/icon-button/icon-button.component.ts | 29 ++++++++++--- libs/components/src/link/link.directive.ts | 20 ++++++++- .../src/menu/menu.component.spec.ts | 8 ++++ libs/components/src/toast/toast.stories.ts | 1 + .../src/utils/aria-disable-element.ts | 19 +++++++++ libs/components/src/utils/index.ts | 1 + 15 files changed, 165 insertions(+), 53 deletions(-) create mode 100644 libs/components/src/a11y/aria-disable.directive.ts create mode 100644 libs/components/src/a11y/aria-disabled-click-capture.service.ts 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 b5d35e2005..b65138dac3 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/auth/settings/verify-email.component.html b/apps/web/src/app/auth/settings/verify-email.component.html index a691c30695..42a546fc28 100644 --- a/apps/web/src/app/auth/settings/verify-email.component.html +++ b/apps/web/src/app/auth/settings/verify-email.component.html @@ -4,10 +4,9 @@ id="sendBtn" bitLink linkType="secondary" - bitButton type="button" buttonType="unstyled" - [bitAction]="send" + (click)="send()" > {{ "sendEmail" | i18n }} 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 085a3d0d4b..afb3273890 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/a11y/aria-disable.directive.ts b/libs/components/src/a11y/aria-disable.directive.ts new file mode 100644 index 0000000000..8236e17899 --- /dev/null +++ b/libs/components/src/a11y/aria-disable.directive.ts @@ -0,0 +1,12 @@ +import { Directive, inject } from "@angular/core"; + +import { AriaDisabledClickCaptureService } from "./aria-disabled-click-capture.service"; + +@Directive({ + host: { + "[attr.bit-aria-disable]": "true", + }, +}) +export class AriaDisableDirective { + protected ariaDisabledClickCaptureService = inject(AriaDisabledClickCaptureService); +} diff --git a/libs/components/src/a11y/aria-disabled-click-capture.service.ts b/libs/components/src/a11y/aria-disabled-click-capture.service.ts new file mode 100644 index 0000000000..d828d15c87 --- /dev/null +++ b/libs/components/src/a11y/aria-disabled-click-capture.service.ts @@ -0,0 +1,30 @@ +import { DOCUMENT } from "@angular/common"; +import { Injectable, Inject, NgZone, OnDestroy } from "@angular/core"; + +@Injectable({ providedIn: "root" }) +export class AriaDisabledClickCaptureService implements OnDestroy { + private listener!: (e: MouseEvent | KeyboardEvent) => void; + + constructor( + @Inject(DOCUMENT) private document: Document, + private ngZone: NgZone, + ) { + this.ngZone.runOutsideAngular(() => { + this.listener = (e: MouseEvent | KeyboardEvent) => { + const btn = (e.target as HTMLElement).closest( + '[aria-disabled="true"][bit-aria-disable="true"]', + ); + if (btn) { + e.stopPropagation(); + e.preventDefault(); + return false; + } + }; + this.document.addEventListener("click", this.listener, /* capture */ true); + }); + } + + ngOnDestroy() { + this.document.removeEventListener("click", this.listener, true); + } +} diff --git a/libs/components/src/a11y/index.ts b/libs/components/src/a11y/index.ts index 6090fb65d4..2a723f14c9 100644 --- a/libs/components/src/a11y/index.ts +++ b/libs/components/src/a11y/index.ts @@ -1 +1,3 @@ export * from "./a11y-title.directive"; +export * from "./aria-disabled-click-capture.service"; +export * from "./aria-disable.directive"; diff --git a/libs/components/src/button/button.component.spec.ts b/libs/components/src/button/button.component.spec.ts index 6ddbc17280..1651b6cf12 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 635c269bd0..1dce792c96 100644 --- a/libs/components/src/button/button.component.ts +++ b/libs/components/src/button/button.component.ts @@ -1,9 +1,20 @@ import { NgClass } from "@angular/common"; -import { input, HostBinding, Component, model, computed, booleanAttribute } from "@angular/core"; +import { + input, + HostBinding, + Component, + model, + computed, + booleanAttribute, + inject, + ElementRef, +} from "@angular/core"; import { toObservable, toSignal } from "@angular/core/rxjs-interop"; import { debounce, interval } from "rxjs"; +import { AriaDisableDirective } from "../a11y"; import { ButtonLikeAbstraction, ButtonType, ButtonSize } from "../shared/button-like.abstraction"; +import { ariaDisableElement } from "../utils"; const focusRing = [ "focus-visible:tw-ring-2", @@ -50,9 +61,7 @@ const buttonStyles: Record = { templateUrl: "button.component.html", providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }], imports: [NgClass], - host: { - "[attr.disabled]": "disabledAttr()", - }, + hostDirectives: [AriaDisableDirective], }) export class ButtonComponent implements ButtonLikeAbstraction { @HostBinding("class") get classList() { @@ -72,14 +81,15 @@ export class ButtonComponent implements ButtonLikeAbstraction { .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", ] : [], ) @@ -88,7 +98,7 @@ export class ButtonComponent implements ButtonLikeAbstraction { protected disabledAttr = computed(() => { const disabled = this.disabled() != null && this.disabled() !== false; - return disabled || this.loading() ? true : null; + return disabled || this.loading(); }); /** @@ -128,4 +138,9 @@ export class ButtonComponent implements ButtonLikeAbstraction { ); disabled = model(false); + private el = inject(ElementRef); + + constructor() { + ariaDisableElement(this.el.nativeElement, this.disabledAttr); + } } diff --git a/libs/components/src/form-field/form-field.component.html b/libs/components/src/form-field/form-field.component.html index b40e16d4cd..ae3bad4069 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 @@
= { ], imports: [NgClass], host: { - "[attr.disabled]": "disabledAttr()", /** * When the `bitIconButton` input is dynamic from a consumer, Angular doesn't put the * `bitIconButton` attribute into the DOM. We use the attribute as a css selector in @@ -87,6 +97,7 @@ const sizes: Record = { */ "[attr.bitIconButton]": "icon()", }, + hostDirectives: [AriaDisableDirective], }) export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement { readonly icon = model.required({ alias: "bitIconButton" }); @@ -118,7 +129,7 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE .concat(sizes[this.size()]) .concat( this.showDisabledStyles() || this.disabled() - ? ["disabled:tw-opacity-60", "disabled:hover:!tw-bg-transparent"] + ? ["aria-disabled:tw-opacity-60", "aria-disabled:hover:!tw-bg-transparent"] : [], ); } @@ -129,7 +140,7 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE protected disabledAttr = computed(() => { const disabled = this.disabled() != null && this.disabled() !== false; - return disabled || this.loading() ? true : null; + return disabled || this.loading(); }); /** @@ -168,8 +179,14 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE return this.elementRef.nativeElement; } - constructor(private elementRef: ElementRef) { - const originalTitle = this.elementRef.nativeElement.getAttribute("title"); + private elementRef = inject(ElementRef); + + constructor() { + const element = this.elementRef.nativeElement; + + ariaDisableElement(element, this.disabledAttr); + + const originalTitle = element.getAttribute("title"); effect(() => { setA11yTitleAndAriaLabel({ diff --git a/libs/components/src/link/link.directive.ts b/libs/components/src/link/link.directive.ts index f2eb44bc3a..7c93b185a7 100644 --- a/libs/components/src/link/link.directive.ts +++ b/libs/components/src/link/link.directive.ts @@ -1,4 +1,7 @@ -import { input, HostBinding, Directive } from "@angular/core"; +import { input, HostBinding, Directive, inject, ElementRef, booleanAttribute } from "@angular/core"; + +import { AriaDisableDirective } from "../a11y"; +import { ariaDisableElement } from "../utils"; export type LinkType = "primary" | "secondary" | "contrast" | "light"; @@ -58,6 +61,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() @@ -86,11 +94,21 @@ export class AnchorLinkDirective extends LinkDirective { @Directive({ selector: "button[bitLink]", + hostDirectives: [AriaDisableDirective], }) 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/menu/menu.component.spec.ts b/libs/components/src/menu/menu.component.spec.ts index c6a54f1afa..3153fd4eb3 100644 --- a/libs/components/src/menu/menu.component.spec.ts +++ b/libs/components/src/menu/menu.component.spec.ts @@ -58,6 +58,14 @@ describe("Menu", () => { expect(getBitMenuPanel()).toBeFalsy(); }); + + it("should not open when the trigger button is disabled", () => { + const buttonDebugElement = fixture.debugElement.query(By.directive(MenuTriggerForDirective)); + buttonDebugElement.nativeElement.setAttribute("disabled", "true"); + (buttonDebugElement.nativeElement as HTMLButtonElement).click(); + + expect(getBitMenuPanel()).toBeFalsy(); + }); }); @Component({ diff --git a/libs/components/src/toast/toast.stories.ts b/libs/components/src/toast/toast.stories.ts index bd2c59a191..dee31f1d6a 100644 --- a/libs/components/src/toast/toast.stories.ts +++ b/libs/components/src/toast/toast.stories.ts @@ -20,6 +20,7 @@ const toastServiceExampleTemplate = ` @Component({ selector: "toast-service-example", template: toastServiceExampleTemplate, + imports: [ButtonModule], }) export class ToastServiceExampleComponent { @Input() 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 0000000000..0f7fb4ca20 --- /dev/null +++ b/libs/components/src/utils/aria-disable-element.ts @@ -0,0 +1,19 @@ +import { Signal } from "@angular/core"; +import { toObservable, takeUntilDestroyed } from "@angular/core/rxjs-interop"; + +/** + * 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"` + */ +export function ariaDisableElement(el: HTMLElement, disabled: Signal) { + toObservable(disabled) + .pipe(takeUntilDestroyed()) + .subscribe((isDisabled) => { + if (isDisabled) { + el.removeAttribute("disabled"); + el.setAttribute("aria-disabled", "true"); + } else { + el.removeAttribute("aria-disabled"); + } + }); +} diff --git a/libs/components/src/utils/index.ts b/libs/components/src/utils/index.ts index afadd6b3b4..91fa71cf0e 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";