Skip to content
Open
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
a979277
Add tooltip to icon button to display label
BryanCunningham Sep 23, 2025
d587e36
remove legacy cdr variable
BryanCunningham Oct 1, 2025
ae4874f
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 2, 2025
e797a0b
create overlay on focus or hover
BryanCunningham Oct 2, 2025
ccaa6f7
attach describdedby ids
BryanCunningham Oct 3, 2025
dffc8e3
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 6, 2025
1249dc6
fix type errors
BryanCunningham Oct 6, 2025
4960aab
remove aria-describedby when not necessary
BryanCunningham Oct 6, 2025
f6ff777
fix failing tests
BryanCunningham Oct 6, 2025
c4043b7
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 6, 2025
f97b8f1
implement Claude feedback
BryanCunningham Oct 6, 2025
4611ed6
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 6, 2025
a2d9202
fixing broken specs
BryanCunningham Oct 7, 2025
0b1638f
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 7, 2025
9091038
remove host attr binding
BryanCunningham Oct 7, 2025
dcd8080
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 7, 2025
b7f454e
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 8, 2025
7d21320
Simplify directive aria logic
BryanCunningham Oct 8, 2025
6c89964
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 8, 2025
3ede3a6
Move id to statis number
BryanCunningham Oct 8, 2025
b434f33
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 8, 2025
767443c
do not render empty tooltip
BryanCunningham Oct 8, 2025
595cbb3
pass id to tooltip component
BryanCunningham Oct 8, 2025
f5e8648
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 8, 2025
cfa9edc
remove pointer-events none to allow tooltip on normal buttons
BryanCunningham Oct 8, 2025
81a6ff8
exclude some tooltip stories
BryanCunningham Oct 9, 2025
2ff37cb
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 9, 2025
1bdf199
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 13, 2025
307b0d2
change describedby input name
BryanCunningham Oct 13, 2025
e78c094
add story with tooltip on regular button
BryanCunningham Oct 13, 2025
fdc6ec3
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 13, 2025
e5b6835
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 14, 2025
c3e8e4c
enhanced tooltip docs
BryanCunningham Oct 14, 2025
30ebc63
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 14, 2025
8d2345b
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 14, 2025
19c19ea
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 14, 2025
a107c0b
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 14, 2025
8558cc7
set model directly
BryanCunningham Oct 14, 2025
77efb1b
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 15, 2025
cdf5a00
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 15, 2025
86b5db9
change model to input
BryanCunningham Oct 16, 2025
6169661
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 16, 2025
62bd86a
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 20, 2025
6ba5ab6
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 20, 2025
c10b194
merge main and resolve conflicts
BryanCunningham Oct 22, 2025
9add2f8
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 27, 2025
83cc425
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 28, 2025
4afd965
Merge remote-tracking branch 'origin/main' into uif/CL-879/tooltip-onโ€ฆ
BryanCunningham Oct 28, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion libs/components/src/button/button.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ export class ButtonComponent implements ButtonLikeAbstraction {
"hover:!tw-text-muted",
"aria-disabled:tw-cursor-not-allowed",
"hover:tw-no-underline",
"aria-disabled:tw-pointer-events-none",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is safe to remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willmartian As far as I can tell, yeah. We're still swallowing the click with our host directive. And, it's necessary to remove in order to apply tooltips to disabled buttons

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably scope creep for this ticket, but since we already have a team looking to use the tooltip on a button, we should make a story for it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a story here

]
: [],
)
Expand Down
19 changes: 15 additions & 4 deletions libs/components/src/icon-button/icon-button.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { setA11yTitleAndAriaLabel } from "../a11y/set-a11y-title-and-aria-label"
import { ButtonLikeAbstraction } from "../shared/button-like.abstraction";
import { FocusableElement } from "../shared/focusable-element";
import { SpinnerComponent } from "../spinner";
import { TooltipDirective } from "../tooltip";
import { ariaDisableElement } from "../utils";

export type IconButtonType = "primary" | "danger" | "contrast" | "main" | "muted" | "nav-contrast";
Expand Down Expand Up @@ -98,7 +99,10 @@ const sizes: Record<IconButtonSize, string[]> = {
*/
"[attr.bitIconButton]": "icon()",
},
hostDirectives: [AriaDisableDirective],
hostDirectives: [
AriaDisableDirective,
{ directive: TooltipDirective, inputs: ["tooltipPosition"] },
],
})
export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement {
readonly icon = model.required<string>({ alias: "bitIconButton" });
Expand All @@ -107,6 +111,9 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE

readonly size = model<IconButtonSize>("default");

private elementRef = inject(ElementRef);
private tooltip = inject(TooltipDirective, { host: true, optional: true });

/**
* 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.
Expand Down Expand Up @@ -184,8 +191,6 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE
return this.elementRef.nativeElement;
}

private elementRef = inject(ElementRef);

constructor() {
const element = this.elementRef.nativeElement;

Expand All @@ -196,9 +201,15 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableE
effect(() => {
setA11yTitleAndAriaLabel({
element: this.elementRef.nativeElement,
title: originalTitle ?? this.label(),
title: undefined,
label: this.label(),
});

const tooltipContent: string = originalTitle || this.label();

if (tooltipContent) {
this.tooltip?.setContent(tooltipContent);
}
});
}
}
19 changes: 10 additions & 9 deletions libs/components/src/tooltip/tooltip.component.html
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
<!-- eslint-disable-next-line tailwindcss/no-custom-classname -->
<div
class="bit-tooltip-container"
[attr.data-position]="tooltipData.tooltipPosition()"
[attr.data-visible]="tooltipData.isVisible()"
>
<div role="tooltip" class="bit-tooltip">
<ng-content>{{ tooltipData.content() }}</ng-content>
@if (tooltipData.content()) {
<div
class="bit-tooltip-container"
[attr.data-position]="tooltipData.tooltipPosition()"
[attr.data-visible]="tooltipData.isVisible()"
>
<div role="tooltip" class="bit-tooltip" [id]="tooltipData.id()">
<ng-content>{{ tooltipData.content() }}</ng-content>
</div>
</div>
</div>
}
1 change: 1 addition & 0 deletions libs/components/src/tooltip/tooltip.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type TooltipData = {
content: Signal<string>;
isVisible: Signal<boolean>;
tooltipPosition: Signal<TooltipPosition>;
id: Signal<string>;
};

export const TOOLTIP_DATA = new InjectionToken<TooltipData>("TOOLTIP_DATA");
Expand Down
63 changes: 43 additions & 20 deletions libs/components/src/tooltip/tooltip.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import {
ElementRef,
Injector,
input,
effect,
signal,
model,
computed,
} from "@angular/core";

import { TooltipPositionIdentifier, tooltipPositions } from "./tooltip-positions";
Expand All @@ -26,30 +27,36 @@ import { TooltipComponent, TOOLTIP_DATA } from "./tooltip.component";
"(mouseleave)": "hideTooltip()",
"(focus)": "showTooltip()",
"(blur)": "hideTooltip()",
"[attr.aria-describedby]": "resolvedDescribedByIds()",
},
})
export class TooltipDirective implements OnInit {
private static nextId = 0;
/**
* The value of this input is forwarded to the tooltip.component to render
*/
readonly bitTooltip = input.required<string>();
readonly bitTooltip = model<string>();
/**
* The value of this input is forwarded to the tooltip.component to set its position explicitly.
* @default "above-center"
*/
readonly tooltipPosition = input<TooltipPositionIdentifier>("above-center");

readonly isDescribedbyText = model<boolean>(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't always be true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willmartian I'm on the fence about it. In the case of the icon button, it's redundant as the tooltip is the same as the aria-label in most cases. I think there maybe some exceptions to this rule for icon buttons but, we can update those as we find them I think. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input name is a little confusing to me. Can we infer this from the presence of the describedby ids or we want to let the consumer be explicit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily. There could be existing/different aria-describedby IDs. This is for the consumer to explicitly opt-in to the tooltip being added to the host element's aria-describedby id list

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vleague2 That being said, certainly open to renaming to something more clear. Any thoughts on what that might be?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh okay I see that now. My very terrible first idea is addTooltipToDescribedby because I guess the usage would be something like

<div bitTooltip="Cool tooltip" addTooltipToDescribedby aria-describedby="some-other-element">some element</div>

to add the tooltip to the existing describedby list? (To be clear, I don't like the name I chose lol)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does seem more clear. I'd like to stick to the is or has prefix for a boolean input if possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can think of one, go for it ๐Ÿ˜‚ I think also a jsdoc comment on this input would help

Copy link
Contributor Author

@BryanCunningham BryanCunningham Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the name and added a comment here


private isVisible = signal(false);
private overlayRef: OverlayRef | undefined;
private elementRef = inject(ElementRef);
private elementRef = inject<ElementRef<HTMLElement>>(ElementRef);
private overlay = inject(Overlay);
private viewContainerRef = inject(ViewContainerRef);
private injector = inject(Injector);
private positionStrategy = this.overlay
.position()
.flexibleConnectedTo(this.elementRef)
.withFlexibleDimensions(false)
.withPush(true);
private tooltipId = `bit-tooltip-${TooltipDirective.nextId++}`;
private currentDescribedByIds =
this.elementRef.nativeElement.getAttribute("aria-describedby") || null;

private tooltipPortal = new ComponentPortal(
TooltipComponent,
Expand All @@ -62,20 +69,47 @@ export class TooltipDirective implements OnInit {
content: this.bitTooltip,
isVisible: this.isVisible,
tooltipPosition: this.tooltipPosition,
id: signal(this.tooltipId),
},
},
],
}),
);

private destroyTooltip = () => {
this.overlayRef?.dispose();
this.overlayRef = undefined;
this.isVisible.set(false);
};

private showTooltip = () => {
if (!this.overlayRef) {
this.overlayRef = this.overlay.create({
...this.defaultPopoverConfig,
positionStrategy: this.positionStrategy,
});

this.overlayRef.attach(this.tooltipPortal);
}
this.isVisible.set(true);
};

private hideTooltip = () => {
this.isVisible.set(false);
this.destroyTooltip();
};

private resolvedDescribedByIds = computed(() => {
if (this.isDescribedbyText()) {
if (this.currentDescribedByIds) {
return `${this.currentDescribedByIds || ""} ${this.tooltipId}`;
} else {
return this.tooltipId;
}
} else {
return this.currentDescribedByIds;
}
});

private computePositions(tooltipPosition: TooltipPositionIdentifier) {
const chosenPosition = tooltipPositions.find((position) => position.id === tooltipPosition);

Expand All @@ -89,22 +123,11 @@ export class TooltipDirective implements OnInit {
};
}

setContent(text: string) {
this.bitTooltip.set(text);
}

ngOnInit() {
this.positionStrategy.withPositions(this.computePositions(this.tooltipPosition()));

this.overlayRef = this.overlay.create({
...this.defaultPopoverConfig,
positionStrategy: this.positionStrategy,
});

this.overlayRef.attach(this.tooltipPortal);

effect(
() => {
this.positionStrategy.withPositions(this.computePositions(this.tooltipPosition()));
this.overlayRef?.updatePosition();
},
{ injector: this.injector },
);
}
}
9 changes: 8 additions & 1 deletion libs/components/src/tooltip/tooltip.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,14 @@ describe("TooltipDirective (visibility only)", () => {
};

const overlayRefStub: OverlayRefStub = {
attach: jest.fn(() => ({})),
attach: jest.fn(() => ({
changeDetectorRef: { detectChanges: jest.fn() },
location: {
nativeElement: {
querySelector: jest.fn().mockReturnValue({ id: "tip-123" }),
},
},
})),
updatePosition: jest.fn(),
};

Expand Down
23 changes: 16 additions & 7 deletions libs/components/src/tooltip/tooltip.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ type Story = StoryObj<TooltipDirective>;

export const Default: Story = {
args: {
bitTooltip: "This is a tooltip",
tooltipPosition: "above-center",
},
render: (args) => ({
Expand All @@ -81,6 +80,7 @@ export const Default: Story = {
<div class="tw-p-4">
<button
bitIconButton="bwi-ellipsis-v"
label="Your tooltip content here"
${formatArgsForCodeSnippet<TooltipDirective>(args)}
>
Button label here
Expand All @@ -98,26 +98,29 @@ export const Default: Story = {

export const AllPositions: Story = {
render: () => ({
parameters: {
chromatic: { disableSnapshot: true },
},
template: `
<div class="tw-p-16 tw-grid tw-grid-cols-2 tw-gap-8 tw-place-items-center">
<button
bitIconButton="bwi-angle-up"
bitTooltip="Top tooltip"
label="Top tooltip"
tooltipPosition="above-center"
></button>
<button
bitIconButton="bwi-angle-right"
bitTooltip="Right tooltip"
label="Right tooltip"
tooltipPosition="right-center"
></button>
<button
bitIconButton="bwi-angle-left"
bitTooltip="Left tooltip"
label="Left tooltip"
tooltipPosition="left-center"
></button>
<button
bitIconButton="bwi-angle-down"
bitTooltip="Bottom tooltip"
label="Bottom tooltip"
tooltipPosition="below-center"
></button>
</div>
Expand All @@ -127,11 +130,14 @@ export const AllPositions: Story = {

export const LongContent: Story = {
render: () => ({
parameters: {
chromatic: { disableSnapshot: true },
},
template: `
<div class="tw-p-16 tw-flex tw-items-center tw-justify-center">
<button
bitIconButton="bwi-ellipsis-v"
bitTooltip="This is a very long tooltip that will wrap to multiple lines to demonstrate how the tooltip handles long content. This is not recommended for usability."
label="This is a very long tooltip that will wrap to multiple lines to demonstrate how the tooltip handles long content. This is not recommended for usability."
></button>
</div>
`,
Expand All @@ -140,11 +146,14 @@ export const LongContent: Story = {

export const OnDisabledButton: Story = {
render: () => ({
parameters: {
chromatic: { disableSnapshot: true },
},
template: `
<div class="tw-p-16 tw-flex tw-items-center tw-justify-center">
<button
bitIconButton="bwi-ellipsis-v"
bitTooltip="Tooltip on disabled button"
label="Tooltip on disabled button"
[disabled]="true"
></button>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe("DeleteAttachmentComponent", () => {
it("renders delete button", () => {
const deleteButton = fixture.debugElement.query(By.css("button"));

expect(deleteButton.attributes["title"]).toBe("deleteAttachmentName");
expect(deleteButton.attributes["aria-label"]).toBe("deleteAttachmentName");
});

it("does not delete when the user cancels the dialog", async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,17 @@ describe("UriOptionComponent", () => {
expect(getMatchDetectionSelect()).not.toBeNull();
});

it("should update the match detection button title when the toggle is clicked", () => {
it("should update the match detection button aria-label when the toggle is clicked", () => {
component.writeValue({ uri: "https://example.com", matchDetection: UriMatchStrategy.Exact });
fixture.detectChanges();
expect(getToggleMatchDetectionBtn().title).toBe("showMatchDetection https://example.com");
expect(getToggleMatchDetectionBtn().getAttribute("aria-label")).toBe(
"showMatchDetection https://example.com",
);
getToggleMatchDetectionBtn().click();
fixture.detectChanges();
expect(getToggleMatchDetectionBtn().title).toBe("hideMatchDetection https://example.com");
expect(getToggleMatchDetectionBtn().getAttribute("aria-label")).toBe(
"hideMatchDetection https://example.com",
);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe("DownloadAttachmentComponent", () => {
it("renders delete button", () => {
const deleteButton = fixture.debugElement.query(By.css("button"));

expect(deleteButton.attributes["title"]).toBe("downloadAttachmentName");
expect(deleteButton.attributes["aria-label"]).toBe("downloadAttachmentName");
});

describe("download attachment", () => {
Expand Down
Loading