From 963a9156fb88c42a50837c9f94224fddd7085160 Mon Sep 17 00:00:00 2001 From: Bryan Cunningham Date: Mon, 1 Dec 2025 11:59:20 -0500 Subject: [PATCH] [CL-910] Use tooltip in title directive (#17084) * use tooltip in a11y directive * remove commented code * add deprecation warning to appA11yTitle directive * use label for tooltip in carousel nav * wait for timeout before assertion * remove unnecessary title directive use * fix private variable lint errors * increase tooltip show delay * fix spec delay and export as constant * use delay constant --------- Co-authored-by: Vicki League --- .../setup-extension.component.html | 2 +- .../src/a11y/a11y-title.directive.ts | 34 +++++++++---------- .../src/icon-button/icon-button.component.ts | 2 +- .../src/icon-button/icon-button.mdx | 9 ++--- .../src/tooltip/tooltip.directive.ts | 12 ++++--- libs/components/src/tooltip/tooltip.spec.ts | 14 ++++---- .../carousel/carousel.component.html | 6 ++-- 7 files changed, 38 insertions(+), 41 deletions(-) diff --git a/apps/web/src/app/vault/components/setup-extension/setup-extension.component.html b/apps/web/src/app/vault/components/setup-extension/setup-extension.component.html index 1976321b4e..a4b2191562 100644 --- a/apps/web/src/app/vault/components/setup-extension/setup-extension.component.html +++ b/apps/web/src/app/vault/components/setup-extension/setup-extension.component.html @@ -2,7 +2,7 @@ *ngIf="state === SetupExtensionState.Loading" class="bwi bwi-spinner bwi-spin bwi-3x tw-text-muted" aria-hidden="true" - [appA11yTitle]="'loading' | i18n" + [title]="'loading' | i18n" >
diff --git a/libs/components/src/a11y/a11y-title.directive.ts b/libs/components/src/a11y/a11y-title.directive.ts index 75c2967805..fa038172cb 100644 --- a/libs/components/src/a11y/a11y-title.directive.ts +++ b/libs/components/src/a11y/a11y-title.directive.ts @@ -1,23 +1,21 @@ -import { Directive, effect, ElementRef, input } from "@angular/core"; +import { Directive } from "@angular/core"; -import { setA11yTitleAndAriaLabel } from "./set-a11y-title-and-aria-label"; +import { TooltipDirective } from "../tooltip/tooltip.directive"; +/** + * @deprecated This function is deprecated in favor of `bitTooltip`. + * Please use `bitTooltip` instead. + * + * Directive that provides accessible tooltips by internally using TooltipDirective. + * This maintains the appA11yTitle API while leveraging the enhanced tooltip functionality. + */ @Directive({ selector: "[appA11yTitle]", + hostDirectives: [ + { + directive: TooltipDirective, + inputs: ["bitTooltip: appA11yTitle", "tooltipPosition"], + }, + ], }) -export class A11yTitleDirective { - readonly title = input.required({ alias: "appA11yTitle" }); - - constructor(private el: ElementRef) { - const originalTitle = this.el.nativeElement.getAttribute("title"); - const originalAriaLabel = this.el.nativeElement.getAttribute("aria-label"); - - effect(() => { - setA11yTitleAndAriaLabel({ - element: this.el.nativeElement, - title: originalTitle ?? this.title(), - label: originalAriaLabel ?? this.title(), - }); - }); - } -} +export class A11yTitleDirective {} diff --git a/libs/components/src/icon-button/icon-button.component.ts b/libs/components/src/icon-button/icon-button.component.ts index ca0e3fb1de..d69bae98df 100644 --- a/libs/components/src/icon-button/icon-button.component.ts +++ b/libs/components/src/icon-button/icon-button.component.ts @@ -120,7 +120,7 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE * label input will be used to set the `aria-label` attributes on the button. * This is for accessibility purposes, as it provides a text alternative for the icon button. * - * NOTE: It will also be used to set the `title` attribute on the button if no `title` is provided. + * NOTE: It will also be used to set the content of the tooltip on the button if no `title` is provided. */ readonly label = input(); diff --git a/libs/components/src/icon-button/icon-button.mdx b/libs/components/src/icon-button/icon-button.mdx index 3fcd4a2358..3922f13758 100644 --- a/libs/components/src/icon-button/icon-button.mdx +++ b/libs/components/src/icon-button/icon-button.mdx @@ -81,10 +81,5 @@ with less padding around the icon, such as in the navigation component. Follow guidelines outlined in the [Button docs](?path=/docs/component-library-button--doc) -Always use the `appA11yTitle` directive set to a string that describes the action of the -icon-button. This will auto assign the same string to the `title` and `aria-label` attributes. - -`aria-label` allows assistive technology to announce the action the button takes to the users. - -`title` attribute provides a user with the browser tool tip if they do not understand what the icon -is indicating. +label input will be used to set the `aria-label` attributes on the button. This is for accessibility +purposes, as it provides a text alternative for the icon button. diff --git a/libs/components/src/tooltip/tooltip.directive.ts b/libs/components/src/tooltip/tooltip.directive.ts index bcf9fc5e17..12be865243 100644 --- a/libs/components/src/tooltip/tooltip.directive.ts +++ b/libs/components/src/tooltip/tooltip.directive.ts @@ -16,6 +16,7 @@ import { import { TooltipPositionIdentifier, tooltipPositions } from "./tooltip-positions"; import { TooltipComponent, TOOLTIP_DATA } from "./tooltip.component"; +export const TOOLTIP_DELAY_MS = 800; /** * Directive to add a tooltip to any element. The tooltip content is provided via the `bitTooltip` input. * The position of the tooltip can be set via the `tooltipPosition` input. Default position is "above-center". @@ -85,7 +86,7 @@ export class TooltipDirective implements OnInit { this.isVisible.set(false); }; - private showTooltip = () => { + protected showTooltip = () => { if (!this.overlayRef) { this.overlayRef = this.overlay.create({ ...this.defaultPopoverConfig, @@ -94,14 +95,17 @@ export class TooltipDirective implements OnInit { this.overlayRef.attach(this.tooltipPortal); } - this.isVisible.set(true); + + setTimeout(() => { + this.isVisible.set(true); + }, TOOLTIP_DELAY_MS); }; - private hideTooltip = () => { + protected hideTooltip = () => { this.destroyTooltip(); }; - private readonly resolvedDescribedByIds = computed(() => { + protected readonly resolvedDescribedByIds = computed(() => { if (this.addTooltipToDescribedby()) { if (this.currentDescribedByIds) { return `${this.currentDescribedByIds || ""} ${this.tooltipId}`; diff --git a/libs/components/src/tooltip/tooltip.spec.ts b/libs/components/src/tooltip/tooltip.spec.ts index a88424de3b..ff73911d29 100644 --- a/libs/components/src/tooltip/tooltip.spec.ts +++ b/libs/components/src/tooltip/tooltip.spec.ts @@ -6,11 +6,11 @@ import { } from "@angular/cdk/overlay"; import { ComponentPortal } from "@angular/cdk/portal"; import { Component } from "@angular/core"; -import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { ComponentFixture, TestBed, fakeAsync, tick } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { Observable, Subject } from "rxjs"; -import { TooltipDirective } from "./tooltip.directive"; +import { TooltipDirective, TOOLTIP_DELAY_MS } from "./tooltip.directive"; // FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush // eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @@ -90,23 +90,25 @@ describe("TooltipDirective (visibility only)", () => { return hostDE.injector.get(TooltipDirective); } - it("sets isVisible to true on mouseenter", () => { + it("sets isVisible to true on mouseenter", fakeAsync(() => { const button: HTMLButtonElement = fixture.debugElement.query(By.css("button")).nativeElement; const directive = getDirective(); const isVisible = (directive as unknown as { isVisible: () => boolean }).isVisible; button.dispatchEvent(new Event("mouseenter")); + tick(TOOLTIP_DELAY_MS); expect(isVisible()).toBe(true); - }); + })); - it("sets isVisible to true on focus", () => { + it("sets isVisible to true on focus", fakeAsync(() => { const button: HTMLButtonElement = fixture.debugElement.query(By.css("button")).nativeElement; const directive = getDirective(); const isVisible = (directive as unknown as { isVisible: () => boolean }).isVisible; button.dispatchEvent(new Event("focus")); + tick(TOOLTIP_DELAY_MS); expect(isVisible()).toBe(true); - }); + })); }); diff --git a/libs/vault/src/components/carousel/carousel.component.html b/libs/vault/src/components/carousel/carousel.component.html index 778b70e15e..999d7ef289 100644 --- a/libs/vault/src/components/carousel/carousel.component.html +++ b/libs/vault/src/components/carousel/carousel.component.html @@ -12,10 +12,9 @@ bitIconButton="bwi-angle-left" class="tw-size-6 tw-p-0 tw-flex tw-items-center tw-justify-center" size="small" - [attr.label]="'back' | i18n" (click)="prevSlide()" [disabled]="selectedIndex <= 0" - appA11yTitle="{{ 'back' | i18n }}" + label="{{ 'back' | i18n }}" >