1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 21:33:27 +00:00

[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
This commit is contained in:
Bryan Cunningham
2025-08-21 09:14:08 -04:00
committed by GitHub
parent 0daa6913d2
commit b0f46004ff
15 changed files with 165 additions and 53 deletions

View File

@@ -76,10 +76,8 @@ describe("VaultGeneratorDialogComponent", () => {
component.onValueGenerated("test-password"); component.onValueGenerated("test-password");
fixture.detectChanges(); fixture.detectChanges();
const button = fixture.debugElement.query( const button = fixture.debugElement.query(By.css("[data-testid='select-button']"));
By.css("[data-testid='select-button']"), expect(button.attributes["aria-disabled"]).toBe(undefined);
).nativeElement;
expect(button.disabled).toBe(false);
}); });
it("should disable the button if no value has been generated", () => { 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); generator.algorithmSelected.emit({ useGeneratedValue: "Use Password" } as any);
fixture.detectChanges(); fixture.detectChanges();
const button = fixture.debugElement.query( const button = fixture.debugElement.query(By.css("[data-testid='select-button']"));
By.css("[data-testid='select-button']"), expect(button.attributes["aria-disabled"]).toBe("true");
).nativeElement;
expect(button.disabled).toBe(true);
}); });
it("should disable the button if no algorithm is selected", () => { it("should disable the button if no algorithm is selected", () => {
@@ -104,10 +100,8 @@ describe("VaultGeneratorDialogComponent", () => {
generator.valueGenerated.emit("test-password"); generator.valueGenerated.emit("test-password");
fixture.detectChanges(); fixture.detectChanges();
const button = fixture.debugElement.query( const button = fixture.debugElement.query(By.css("[data-testid='select-button']"));
By.css("[data-testid='select-button']"), expect(button.attributes["aria-disabled"]).toBe("true");
).nativeElement;
expect(button.disabled).toBe(true);
}); });
it("should update button text when algorithm is selected", () => { it("should update button text when algorithm is selected", () => {

View File

@@ -4,10 +4,9 @@
id="sendBtn" id="sendBtn"
bitLink bitLink
linkType="secondary" linkType="secondary"
bitButton
type="button" type="button"
buttonType="unstyled" buttonType="unstyled"
[bitAction]="send" (click)="send()"
> >
{{ "sendEmail" | i18n }} {{ "sendEmail" | i18n }}
</button> </button>

View File

@@ -70,10 +70,8 @@ describe("WebVaultGeneratorDialogComponent", () => {
generator.valueGenerated.emit("test-password"); generator.valueGenerated.emit("test-password");
fixture.detectChanges(); fixture.detectChanges();
const button = fixture.debugElement.query( const button = fixture.debugElement.query(By.css("[data-testid='select-button']"));
By.css("[data-testid='select-button']"), expect(button.attributes["aria-disabled"]).toBe(undefined);
).nativeElement;
expect(button.disabled).toBe(false);
}); });
it("should disable the button if no value has been generated", () => { 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); generator.algorithmSelected.emit({ useGeneratedValue: "Use Password" } as any);
fixture.detectChanges(); fixture.detectChanges();
const button = fixture.debugElement.query( const button = fixture.debugElement.query(By.css("[data-testid='select-button']"));
By.css("[data-testid='select-button']"), expect(button.attributes["aria-disabled"]).toBe("true");
).nativeElement;
expect(button.disabled).toBe(true);
}); });
it("should disable the button if no algorithm is selected", () => { it("should disable the button if no algorithm is selected", () => {
@@ -98,10 +94,8 @@ describe("WebVaultGeneratorDialogComponent", () => {
generator.valueGenerated.emit("test-password"); generator.valueGenerated.emit("test-password");
fixture.detectChanges(); fixture.detectChanges();
const button = fixture.debugElement.query( const button = fixture.debugElement.query(By.css("[data-testid='select-button']"));
By.css("[data-testid='select-button']"), expect(button.attributes["aria-disabled"]).toBe("true");
).nativeElement;
expect(button.disabled).toBe(true);
}); });
it("should close with selected value when confirmed", () => { it("should close with selected value when confirmed", () => {

View File

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

View File

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

View File

@@ -1 +1,3 @@
export * from "./a11y-title.directive"; export * from "./a11y-title.directive";
export * from "./aria-disabled-click-capture.service";
export * from "./aria-disable.directive";

View File

@@ -34,23 +34,25 @@ describe("Button", () => {
expect(buttonDebugElement.nativeElement.disabled).toBeFalsy(); 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; testAppComponent.disabled = true;
fixture.detectChanges(); fixture.detectChanges();
expect(buttonDebugElement.attributes["aria-disabled"]).toBe("true");
expect(buttonDebugElement.nativeElement.disabled).toBeTruthy(); expect(buttonDebugElement.nativeElement.disabled).toBeFalsy();
// Anchor tags cannot be disabled. // Anchor tags cannot be disabled.
}); });
it("should be disabled when attribute disabled is true", () => { it("should be aria-disabled not html attribute disabled when attribute disabled is true", () => {
expect(disabledButtonDebugElement.nativeElement.disabled).toBeTruthy(); fixture.detectChanges();
expect(disabledButtonDebugElement.attributes["aria-disabled"]).toBe("true");
expect(disabledButtonDebugElement.nativeElement.disabled).toBeFalsy();
}); });
it("should be disabled when loading is true", () => { it("should be disabled when loading is true", () => {
testAppComponent.loading = true; testAppComponent.loading = true;
fixture.detectChanges(); fixture.detectChanges();
expect(buttonDebugElement.nativeElement.disabled).toBeTruthy(); expect(buttonDebugElement.attributes["aria-disabled"]).toBe("true");
}); });
}); });

View File

@@ -1,9 +1,20 @@
import { NgClass } from "@angular/common"; 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 { toObservable, toSignal } from "@angular/core/rxjs-interop";
import { debounce, interval } from "rxjs"; import { debounce, interval } from "rxjs";
import { AriaDisableDirective } from "../a11y";
import { ButtonLikeAbstraction, ButtonType, ButtonSize } from "../shared/button-like.abstraction"; import { ButtonLikeAbstraction, ButtonType, ButtonSize } from "../shared/button-like.abstraction";
import { ariaDisableElement } from "../utils";
const focusRing = [ const focusRing = [
"focus-visible:tw-ring-2", "focus-visible:tw-ring-2",
@@ -50,9 +61,7 @@ const buttonStyles: Record<ButtonType, string[]> = {
templateUrl: "button.component.html", templateUrl: "button.component.html",
providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }], providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }],
imports: [NgClass], imports: [NgClass],
host: { hostDirectives: [AriaDisableDirective],
"[attr.disabled]": "disabledAttr()",
},
}) })
export class ButtonComponent implements ButtonLikeAbstraction { export class ButtonComponent implements ButtonLikeAbstraction {
@HostBinding("class") get classList() { @HostBinding("class") get classList() {
@@ -72,14 +81,15 @@ export class ButtonComponent implements ButtonLikeAbstraction {
.concat( .concat(
this.showDisabledStyles() || this.disabled() this.showDisabledStyles() || this.disabled()
? [ ? [
"disabled:tw-bg-secondary-300", "aria-disabled:!tw-bg-secondary-300",
"disabled:hover:tw-bg-secondary-300", "hover:tw-bg-secondary-300",
"disabled:tw-border-secondary-300", "aria-disabled:tw-border-secondary-300",
"disabled:hover:tw-border-secondary-300", "hover:tw-border-secondary-300",
"disabled:!tw-text-muted", "aria-disabled:!tw-text-muted",
"disabled:hover:!tw-text-muted", "hover:!tw-text-muted",
"disabled:tw-cursor-not-allowed", "aria-disabled:tw-cursor-not-allowed",
"disabled:hover:tw-no-underline", "hover:tw-no-underline",
"aria-disabled:tw-pointer-events-none",
] ]
: [], : [],
) )
@@ -88,7 +98,7 @@ export class ButtonComponent implements ButtonLikeAbstraction {
protected disabledAttr = computed(() => { protected disabledAttr = computed(() => {
const disabled = this.disabled() != null && this.disabled() !== false; 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<boolean>(false); disabled = model<boolean>(false);
private el = inject(ElementRef<HTMLButtonElement>);
constructor() {
ariaDisableElement(this.el.nativeElement, this.disabledAttr);
}
} }

View File

@@ -46,7 +46,7 @@
</div> </div>
</div> </div>
<div <div
class="tw-gap-1 tw-bg-background tw-rounded-lg tw-flex tw-min-h-11 [&:not(:has(button:enabled)):has(input:read-only)]:tw-bg-secondary-100 [&:not(:has(button:enabled)):has(textarea:read-only)]:tw-bg-secondary-100" class="tw-gap-1 tw-bg-background tw-rounded-lg tw-flex tw-min-h-11 [&:has(input:read-only,textarea:read-only):not(:has(button:not([aria-disabled='true'])))]:tw-bg-secondary-100"
> >
<div <div
#prefixContainer #prefixContainer

View File

@@ -1,11 +1,22 @@
import { NgClass } from "@angular/common"; import { NgClass } from "@angular/common";
import { Component, computed, effect, ElementRef, HostBinding, input, model } from "@angular/core"; import {
Component,
computed,
effect,
ElementRef,
HostBinding,
inject,
input,
model,
} from "@angular/core";
import { toObservable, toSignal } from "@angular/core/rxjs-interop"; import { toObservable, toSignal } from "@angular/core/rxjs-interop";
import { debounce, interval } from "rxjs"; import { debounce, interval } from "rxjs";
import { AriaDisableDirective } from "../a11y";
import { setA11yTitleAndAriaLabel } from "../a11y/set-a11y-title-and-aria-label"; import { setA11yTitleAndAriaLabel } from "../a11y/set-a11y-title-and-aria-label";
import { ButtonLikeAbstraction } from "../shared/button-like.abstraction"; import { ButtonLikeAbstraction } from "../shared/button-like.abstraction";
import { FocusableElement } from "../shared/focusable-element"; import { FocusableElement } from "../shared/focusable-element";
import { ariaDisableElement } from "../utils";
export type IconButtonType = "primary" | "danger" | "contrast" | "main" | "muted" | "nav-contrast"; export type IconButtonType = "primary" | "danger" | "contrast" | "main" | "muted" | "nav-contrast";
@@ -78,7 +89,6 @@ const sizes: Record<IconButtonSize, string[]> = {
], ],
imports: [NgClass], imports: [NgClass],
host: { host: {
"[attr.disabled]": "disabledAttr()",
/** /**
* When the `bitIconButton` input is dynamic from a consumer, Angular doesn't put the * 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 * `bitIconButton` attribute into the DOM. We use the attribute as a css selector in
@@ -87,6 +97,7 @@ const sizes: Record<IconButtonSize, string[]> = {
*/ */
"[attr.bitIconButton]": "icon()", "[attr.bitIconButton]": "icon()",
}, },
hostDirectives: [AriaDisableDirective],
}) })
export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement { export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement {
readonly icon = model.required<string>({ alias: "bitIconButton" }); readonly icon = model.required<string>({ alias: "bitIconButton" });
@@ -118,7 +129,7 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE
.concat(sizes[this.size()]) .concat(sizes[this.size()])
.concat( .concat(
this.showDisabledStyles() || this.disabled() 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(() => { protected disabledAttr = computed(() => {
const disabled = this.disabled() != null && this.disabled() !== false; 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; return this.elementRef.nativeElement;
} }
constructor(private elementRef: ElementRef) { private elementRef = inject(ElementRef);
const originalTitle = this.elementRef.nativeElement.getAttribute("title");
constructor() {
const element = this.elementRef.nativeElement;
ariaDisableElement(element, this.disabledAttr);
const originalTitle = element.getAttribute("title");
effect(() => { effect(() => {
setA11yTitleAndAriaLabel({ setA11yTitleAndAriaLabel({

View File

@@ -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"; export type LinkType = "primary" | "secondary" | "contrast" | "light";
@@ -58,6 +61,11 @@ const commonStyles = [
"before:tw-transition", "before:tw-transition",
"focus-visible:before:tw-ring-2", "focus-visible:before:tw-ring-2",
"focus-visible:tw-z-10", "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() @Directive()
@@ -86,11 +94,21 @@ export class AnchorLinkDirective extends LinkDirective {
@Directive({ @Directive({
selector: "button[bitLink]", selector: "button[bitLink]",
hostDirectives: [AriaDisableDirective],
}) })
export class ButtonLinkDirective extends LinkDirective { export class ButtonLinkDirective extends LinkDirective {
private el = inject(ElementRef<HTMLButtonElement>);
disabled = input(false, { transform: booleanAttribute });
@HostBinding("class") get classList() { @HostBinding("class") get classList() {
return ["before:-tw-inset-y-[0.25rem]"] return ["before:-tw-inset-y-[0.25rem]"]
.concat(commonStyles) .concat(commonStyles)
.concat(linkStyles[this.linkType()] ?? []); .concat(linkStyles[this.linkType()] ?? []);
} }
constructor() {
super();
ariaDisableElement(this.el.nativeElement, this.disabled);
}
} }

View File

@@ -58,6 +58,14 @@ describe("Menu", () => {
expect(getBitMenuPanel()).toBeFalsy(); 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({ @Component({

View File

@@ -20,6 +20,7 @@ const toastServiceExampleTemplate = `
@Component({ @Component({
selector: "toast-service-example", selector: "toast-service-example",
template: toastServiceExampleTemplate, template: toastServiceExampleTemplate,
imports: [ButtonModule],
}) })
export class ToastServiceExampleComponent { export class ToastServiceExampleComponent {
@Input() @Input()

View File

@@ -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<boolean | undefined>) {
toObservable(disabled)
.pipe(takeUntilDestroyed())
.subscribe((isDisabled) => {
if (isDisabled) {
el.removeAttribute("disabled");
el.setAttribute("aria-disabled", "true");
} else {
el.removeAttribute("aria-disabled");
}
});
}

View File

@@ -1,2 +1,3 @@
export * from "./aria-disable-element";
export * from "./function-to-observable"; export * from "./function-to-observable";
export * from "./i18n-mock.service"; export * from "./i18n-mock.service";