1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 05:43:41 +00:00

[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 <contact@willmartian.com>
This commit is contained in:
Bryan Cunningham
2025-07-08 16:13:25 -04:00
committed by GitHub
parent d5e7f3bd04
commit 682f1f83d9
12 changed files with 175 additions and 73 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

@@ -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

@@ -13,6 +13,7 @@ import { IconButtonModule } from "../icon-button";
import { InputModule } from "../input/input.module"; import { InputModule } from "../input/input.module";
import { I18nMockService } from "../utils/i18n-mock.service"; import { I18nMockService } from "../utils/i18n-mock.service";
import { AsyncActionsModule } from "./async-actions.module";
import { BitActionDirective } from "./bit-action.directive"; import { BitActionDirective } from "./bit-action.directive";
import { BitSubmitDirective } from "./bit-submit.directive"; import { BitSubmitDirective } from "./bit-submit.directive";
import { BitFormButtonDirective } from "./form-button.directive"; import { BitFormButtonDirective } from "./form-button.directive";
@@ -40,6 +41,13 @@ const template = `
@Component({ @Component({
selector: "app-promise-example", selector: "app-promise-example",
template, template,
imports: [
AsyncActionsModule,
ButtonModule,
FormFieldModule,
IconButtonModule,
ReactiveFormsModule,
],
}) })
class PromiseExampleComponent { class PromiseExampleComponent {
formObj = this.formBuilder.group({ formObj = this.formBuilder.group({
@@ -77,6 +85,13 @@ class PromiseExampleComponent {
@Component({ @Component({
selector: "app-observable-example", selector: "app-observable-example",
template, template,
imports: [
AsyncActionsModule,
ButtonModule,
FormFieldModule,
IconButtonModule,
ReactiveFormsModule,
],
}) })
class ObservableExampleComponent { class ObservableExampleComponent {
formObj = this.formBuilder.group({ formObj = this.formBuilder.group({
@@ -109,7 +124,6 @@ export default {
title: "Component Library/Async Actions/In Forms", title: "Component Library/Async Actions/In Forms",
decorators: [ decorators: [
moduleMetadata({ moduleMetadata({
declarations: [PromiseExampleComponent, ObservableExampleComponent],
imports: [ imports: [
BitSubmitDirective, BitSubmitDirective,
BitFormButtonDirective, BitFormButtonDirective,
@@ -120,6 +134,8 @@ export default {
ButtonModule, ButtonModule,
IconButtonModule, IconButtonModule,
BitActionDirective, BitActionDirective,
PromiseExampleComponent,
ObservableExampleComponent,
], ],
providers: [ providers: [
{ {

View File

@@ -9,6 +9,7 @@ import { ValidationService } from "@bitwarden/common/platform/abstractions/valid
import { ButtonModule } from "../button"; import { ButtonModule } from "../button";
import { IconButtonModule } from "../icon-button"; import { IconButtonModule } from "../icon-button";
import { AsyncActionsModule } from "./async-actions.module";
import { BitActionDirective } from "./bit-action.directive"; import { BitActionDirective } from "./bit-action.directive";
const template = /*html*/ ` const template = /*html*/ `
@@ -20,6 +21,8 @@ const template = /*html*/ `
@Component({ @Component({
template, template,
selector: "app-promise-example", selector: "app-promise-example",
imports: [AsyncActionsModule, ButtonModule, IconButtonModule],
standalone: true,
}) })
class PromiseExampleComponent { class PromiseExampleComponent {
statusEmoji = "🟡"; statusEmoji = "🟡";
@@ -36,6 +39,7 @@ class PromiseExampleComponent {
@Component({ @Component({
template, template,
selector: "app-action-resolves-quickly", selector: "app-action-resolves-quickly",
imports: [AsyncActionsModule, ButtonModule, IconButtonModule],
}) })
class ActionResolvesQuicklyComponent { class ActionResolvesQuicklyComponent {
statusEmoji = "🟡"; statusEmoji = "🟡";
@@ -53,6 +57,7 @@ class ActionResolvesQuicklyComponent {
@Component({ @Component({
template, template,
selector: "app-observable-example", selector: "app-observable-example",
imports: [AsyncActionsModule, ButtonModule, IconButtonModule],
}) })
class ObservableExampleComponent { class ObservableExampleComponent {
action = () => { action = () => {
@@ -63,6 +68,7 @@ class ObservableExampleComponent {
@Component({ @Component({
template, template,
selector: "app-rejected-promise-example", selector: "app-rejected-promise-example",
imports: [AsyncActionsModule, ButtonModule, IconButtonModule],
}) })
class RejectedPromiseExampleComponent { class RejectedPromiseExampleComponent {
action = async () => { action = async () => {
@@ -76,13 +82,15 @@ export default {
title: "Component Library/Async Actions/Standalone", title: "Component Library/Async Actions/Standalone",
decorators: [ decorators: [
moduleMetadata({ moduleMetadata({
declarations: [ imports: [
ButtonModule,
IconButtonModule,
BitActionDirective,
PromiseExampleComponent, PromiseExampleComponent,
ObservableExampleComponent, ObservableExampleComponent,
RejectedPromiseExampleComponent, RejectedPromiseExampleComponent,
ActionResolvesQuicklyComponent, ActionResolvesQuicklyComponent,
], ],
imports: [ButtonModule, IconButtonModule, BitActionDirective],
providers: [ providers: [
{ {
provide: ValidationService, provide: ValidationService,

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,10 +1,21 @@
import { coerceBooleanProperty } from "@angular/cdk/coercion"; import { coerceBooleanProperty } from "@angular/cdk/coercion";
import { NgClass } from "@angular/common"; 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 { toObservable, toSignal } from "@angular/core/rxjs-interop";
import { debounce, interval } from "rxjs"; import { debounce, interval } from "rxjs";
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",
@@ -52,7 +63,7 @@ const buttonStyles: Record<ButtonType, string[]> = {
providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }], providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }],
imports: [NgClass], imports: [NgClass],
host: { host: {
"[attr.disabled]": "disabledAttr()", "[attr.aria-disabled]": "disabledAttr()",
}, },
}) })
export class ButtonComponent implements ButtonLikeAbstraction { export class ButtonComponent implements ButtonLikeAbstraction {
@@ -69,27 +80,28 @@ export class ButtonComponent implements ButtonLikeAbstraction {
"focus:tw-outline-none", "focus:tw-outline-none",
] ]
.concat(this.block ? ["tw-w-full", "tw-block"] : ["tw-inline-block"]) .concat(this.block ? ["tw-w-full", "tw-block"] : ["tw-inline-block"])
.concat(buttonStyles[this.buttonType ?? "secondary"])
.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",
] ]
: [], : [],
) )
.concat(buttonStyles[this.buttonType ?? "secondary"])
.concat(buttonSizeStyles[this.size() || "default"]); .concat(buttonSizeStyles[this.size() || "default"]);
} }
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() ? true : undefined;
}); });
/** /**
@@ -138,4 +150,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 as Signal<boolean | undefined>);
}
} }

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:disabled,input:read-only,textarea:read-only):not(:has(button:not([aria-disabled='true'])))]:tw-bg-secondary-100"
> >
<div <div
#prefixContainer #prefixContainer

View File

@@ -88,6 +88,7 @@ export default {
SectionComponent, SectionComponent,
TextFieldModule, TextFieldModule,
BadgeModule, BadgeModule,
A11yTitleDirective,
], ],
providers: [ providers: [
{ {

View File

@@ -1,12 +1,22 @@
// FIXME: Update this file to be type safe and remove this and next line // FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore // @ts-strict-ignore
import { NgClass } from "@angular/common"; import { NgClass } from "@angular/common";
import { Component, computed, ElementRef, HostBinding, Input, model } from "@angular/core"; import {
Component,
computed,
ElementRef,
HostBinding,
inject,
Input,
model,
Signal,
} 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 { ButtonLikeAbstraction, ButtonType } from "../shared/button-like.abstraction"; import { ButtonLikeAbstraction, ButtonType } from "../shared/button-like.abstraction";
import { FocusableElement } from "../shared/focusable-element"; import { FocusableElement } from "../shared/focusable-element";
import { ariaDisableElement } from "../utils";
export type IconButtonType = ButtonType | "contrast" | "main" | "muted" | "light"; export type IconButtonType = ButtonType | "contrast" | "main" | "muted" | "light";
@@ -102,41 +112,41 @@ const styles: Record<IconButtonType, string[]> = {
const disabledStyles: Record<IconButtonType, string[]> = { const disabledStyles: Record<IconButtonType, string[]> = {
contrast: [ contrast: [
"disabled:tw-opacity-60", "aria-disabled:tw-opacity-60",
"disabled:hover:tw-border-transparent", "aria-disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent", "aria-disabled:hover:tw-bg-transparent",
], ],
main: [ main: [
"disabled:!tw-text-secondary-300", "aria-disabled:!tw-text-secondary-300",
"disabled:hover:tw-border-transparent", "aria-disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent", "aria-disabled:hover:tw-bg-transparent",
], ],
muted: [ muted: [
"disabled:!tw-text-secondary-300", "aria-disabled:!tw-text-secondary-300",
"disabled:hover:tw-border-transparent", "aria-disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent", "aria-disabled:hover:tw-bg-transparent",
], ],
primary: [ primary: [
"disabled:tw-opacity-60", "aria-disabled:tw-opacity-60",
"disabled:hover:tw-border-primary-600", "aria-disabled:hover:tw-border-primary-600",
"disabled:hover:tw-bg-primary-600", "aria-disabled:hover:tw-bg-primary-600",
], ],
secondary: [ secondary: [
"disabled:tw-opacity-60", "aria-disabled:tw-opacity-60",
"disabled:hover:tw-border-text-muted", "aria-disabled:hover:tw-border-text-muted",
"disabled:hover:tw-bg-transparent", "aria-disabled:hover:tw-bg-transparent",
"disabled:hover:!tw-text-muted", "aria-disabled:hover:!tw-text-muted",
], ],
danger: [ danger: [
"disabled:!tw-text-secondary-300", "aria-disabled:!tw-text-secondary-300",
"disabled:hover:tw-border-transparent", "aria-disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent", "aria-disabled:hover:tw-bg-transparent",
"disabled:hover:!tw-text-secondary-300", "aria-disabled:hover:!tw-text-secondary-300",
], ],
light: [ light: [
"disabled:tw-opacity-60", "aria-disabled:tw-opacity-60",
"disabled:hover:tw-border-transparent", "aria-disabled:hover:tw-border-transparent",
"disabled:hover:tw-bg-transparent", "aria-disabled:hover:tw-bg-transparent",
], ],
unstyled: [], unstyled: [],
}; };
@@ -163,7 +173,7 @@ const sizes: Record<IconButtonSize, string[]> = {
], ],
imports: [NgClass], imports: [NgClass],
host: { host: {
"[attr.disabled]": "disabledAttr()", "[attr.aria-disabled]": "disabledAttr()",
}, },
}) })
export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement { export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement {
@@ -233,5 +243,10 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE
return this.elementRef.nativeElement; return this.elementRef.nativeElement;
} }
constructor(private elementRef: ElementRef) {} private elementRef = inject(ElementRef);
constructor() {
const element = this.elementRef.nativeElement;
ariaDisableElement(element, this.disabledAttr as Signal<boolean | undefined>);
}
} }

View File

@@ -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"; export type LinkType = "primary" | "secondary" | "contrast" | "light";
@@ -58,6 +68,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()
@@ -89,9 +104,19 @@ export class AnchorLinkDirective extends LinkDirective {
selector: "button[bitLink]", selector: "button[bitLink]",
}) })
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

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

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";