Skip to content

Commit dfd3749

Browse files
pfaffeDevtools-frontend LUCI CQ
authored andcommitted
Tooltip tweaks
- Move to tooltips directory to appease linting. - Add files to grd. - Simplify implementation a little bit. - Enforce ids on non-template construction and validate it against aria-details/describedby. - Show tooltip on focus. Bug: 392078321 Change-Id: I36485bcdd748ce99dd3a9ae5bcb375c0b5c1837d Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6305499 Commit-Queue: Philip Pfaffe <[email protected]> Auto-Submit: Philip Pfaffe <[email protected]> Reviewed-by: Kim-Anh Tran <[email protected]>
1 parent 9379e99 commit dfd3749

File tree

10 files changed

+106
-131
lines changed

10 files changed

+106
-131
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -766,6 +766,7 @@ grd_files_release_sources = [
766766
"front_end/ui/components/switch/switch.js",
767767
"front_end/ui/components/text_editor/text_editor.js",
768768
"front_end/ui/components/text_prompt/text_prompt.js",
769+
"front_end/ui/components/tooltips/tooltips.js",
769770
"front_end/ui/components/tree_outline/tree_outline.js",
770771
"front_end/ui/legacy/components/color_picker/color_picker.js",
771772
"front_end/ui/legacy/components/cookie_table/cookie_table.js",
@@ -2346,6 +2347,8 @@ grd_files_debug_sources = [
23462347
"front_end/ui/components/text_editor/theme.js",
23472348
"front_end/ui/components/text_prompt/TextPrompt.js",
23482349
"front_end/ui/components/text_prompt/textPrompt.css.js",
2350+
"front_end/ui/components/tooltips/Tooltip.js",
2351+
"front_end/ui/components/tooltips/tooltip.css.js",
23492352
"front_end/ui/components/tree_outline/TreeOutline.js",
23502353
"front_end/ui/components/tree_outline/TreeOutlineUtils.js",
23512354
"front_end/ui/components/tree_outline/treeOutline.css.js",

front_end/BUILD.gn

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ group("unittests") {
249249
"ui/components/switch:unittests",
250250
"ui/components/text_editor:unittests",
251251
"ui/components/text_prompt:unittests",
252-
"ui/components/tooltip:unittests",
252+
"ui/components/tooltips:unittests",
253253
"ui/components/tree_outline:unittests",
254254
"ui/legacy:copy_stylesheets_for_server",
255255
"ui/legacy:unittests",

front_end/panels/elements/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ devtools_module("elements") {
9393
"../../ui/components/floating_button:bundle",
9494
"../../ui/components/highlighting:bundle",
9595
"../../ui/components/text_editor:bundle",
96+
"../../ui/components/tooltips:bundle",
9697
"../../ui/components/tree_outline:bundle",
9798
"../../ui/legacy:bundle",
9899
"../../ui/legacy/components/color_picker:bundle",

front_end/ui/components/docs/tooltip/BUILD.gn

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ ts_library("ts") {
1212
deps = [
1313
"../../../../testing",
1414
"../../../lit:bundle",
15-
"../../tooltip:bundle",
15+
"../../tooltips:bundle",
1616
]
1717
}
1818

front_end/ui/components/docs/tooltip/basic.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import * as FrontendHelpers from '../../../../testing/EnvironmentHelpers.js';
66
import * as Lit from '../../../lit/lit.js';
77
import * as ComponentHelpers from '../../helpers/helpers.js';
8-
import {Tooltip} from '../../tooltip/Tooltip.js';
8+
import {Tooltip} from '../../tooltips/Tooltip.js';
99

1010
const {html} = Lit;
1111

front_end/ui/components/tooltip/BUILD.gn renamed to front_end/ui/components/tooltips/BUILD.gn

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ generate_css("css_files") {
1515
devtools_module("tooltip") {
1616
sources = [ "Tooltip.ts" ]
1717

18-
deps = [ "../../lit:bundle" ]
18+
deps = [
19+
"../../../ui/visual_logging:bundle",
20+
"../../lit:bundle",
21+
]
1922
}
2023

2124
devtools_entrypoint("bundle") {

front_end/ui/components/tooltip/Tooltip.test.ts renamed to front_end/ui/components/tooltips/Tooltip.test.ts

Lines changed: 48 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,12 @@ import {renderElementIntoDOM} from '../../../testing/DOMHelpers.js';
66
import {checkForPendingActivity} from '../../../testing/TrackAsyncOperations.js';
77
import * as Lit from '../../lit/lit.js';
88

9-
import * as TooltipModule from './Tooltip.js';
10-
import type {TooltipVariant} from './Tooltip.js';
9+
import * as Tooltips from './tooltips.js';
1110

12-
const {
13-
closestAnchor,
14-
Tooltip,
15-
} = TooltipModule;
16-
17-
const {html, Directives} = Lit;
18-
const {ref, createRef} = Directives;
11+
const {html} = Lit;
1912

2013
interface RenderProps {
21-
variant?: TooltipVariant;
14+
variant?: Tooltips.Tooltip.TooltipVariant;
2215
attribute?: 'aria-describedby'|'aria-details';
2316
useClick?: boolean;
2417
}
@@ -69,6 +62,38 @@ describe('Tooltip', () => {
6962
assert.isTrue(container.querySelector('devtools-tooltip')?.open);
7063
});
7164

65+
it('should be activated if focused', async () => {
66+
const container = renderTooltip();
67+
68+
const button = container.querySelector('button');
69+
button?.dispatchEvent(new FocusEvent('focus'));
70+
71+
await checkForPendingActivity();
72+
assert.isTrue(container.querySelector('devtools-tooltip')?.open);
73+
});
74+
75+
it('should not be activated if un-hovered', async () => {
76+
const container = renderTooltip();
77+
78+
const button = container.querySelector('button');
79+
button?.dispatchEvent(new MouseEvent('mouseenter'));
80+
button?.dispatchEvent(new MouseEvent('mouseleave'));
81+
82+
await checkForPendingActivity();
83+
assert.isFalse(container.querySelector('devtools-tooltip')?.open);
84+
});
85+
86+
it('should not be activated if un-focused', async () => {
87+
const container = renderTooltip();
88+
89+
const button = container.querySelector('button');
90+
button?.dispatchEvent(new FocusEvent('focus'));
91+
button?.dispatchEvent(new FocusEvent('blur'));
92+
93+
await checkForPendingActivity();
94+
assert.isFalse(container.querySelector('devtools-tooltip')?.open);
95+
});
96+
7297
it('should not open on hover if use-click is set', async () => {
7398
const container = renderTooltip({useClick: true});
7499

@@ -79,6 +104,16 @@ describe('Tooltip', () => {
79104
assert.isFalse(container.querySelector('devtools-tooltip')?.open);
80105
});
81106

107+
it('should not open on focus if use-click is set', async () => {
108+
const container = renderTooltip({useClick: true});
109+
110+
const button = container.querySelector('button');
111+
button?.dispatchEvent(new FocusEvent('focus'));
112+
113+
await checkForPendingActivity();
114+
assert.isFalse(container.querySelector('devtools-tooltip')?.open);
115+
});
116+
82117
it('should open with click if use-click is set', () => {
83118
const container = renderTooltip({useClick: true});
84119

@@ -113,13 +148,14 @@ describe('Tooltip', () => {
113148
it('can be instantiated programatically', () => {
114149
const container = document.createElement('div');
115150
const anchor = document.createElement('button');
116-
const tooltip = new Tooltip({id: 'tooltip-id', anchor});
151+
anchor.setAttribute('aria-describedby', 'tooltip-id');
152+
const tooltip = new Tooltips.Tooltip.Tooltip({id: 'tooltip-id', anchor});
117153
tooltip.append('Text content');
118154
container.appendChild(anchor);
119155
container.appendChild(tooltip);
120156
renderElementIntoDOM(container);
121157

122-
assert.strictEqual(anchor.style.anchorName, '--tooltip-id-anchor');
158+
assert.strictEqual(anchor.style.anchorName, '--devtools-tooltip-tooltip-id-anchor');
123159
});
124160

125161
it('should hide the tooltip if anchor is removed from DOM', async () => {
@@ -134,81 +170,3 @@ describe('Tooltip', () => {
134170
assert.isFalse(container.querySelector('devtools-tooltip')?.open);
135171
});
136172
});
137-
138-
describe('closestAnchor', () => {
139-
function renderTemplate(template: Lit.TemplateResult) {
140-
const container = document.createElement('div');
141-
Lit.render(template, container);
142-
renderElementIntoDOM(container);
143-
}
144-
145-
it('finds a previous sibling anchor', () => {
146-
const origin = createRef();
147-
const expectedAchnor = createRef();
148-
// clang-format off
149-
renderTemplate(html`
150-
<div class="anchor" ${ref(expectedAchnor)}></div>
151-
<div ${ref(origin)}></div>
152-
`);
153-
// clang-format on
154-
155-
const actual = closestAnchor(origin.value!, '.anchor');
156-
157-
assert.strictEqual(actual, expectedAchnor.value);
158-
});
159-
160-
it('finds a parent', () => {
161-
const origin = createRef();
162-
const expectedAchnor = createRef();
163-
// clang-format off
164-
renderTemplate(html`
165-
<div class="anchor" ${ref(expectedAchnor)}>
166-
<div ${ref(origin)}></div>
167-
</div>
168-
`);
169-
// clang-format on
170-
171-
const actual = closestAnchor(origin.value!, '.anchor');
172-
173-
assert.strictEqual(actual, expectedAchnor.value);
174-
});
175-
176-
it('finds an ancestors decendant', () => {
177-
const origin = createRef();
178-
const expectedAchnor = createRef();
179-
// clang-format off
180-
renderTemplate(html`
181-
<div>
182-
<div>
183-
<div class="anchor" ${ref(expectedAchnor)}></div>
184-
</div>
185-
<div>
186-
<div ${ref(origin)}></div>
187-
</div>
188-
</div>
189-
`);
190-
// clang-format on
191-
192-
const actual = closestAnchor(origin.value!, '.anchor');
193-
194-
assert.strictEqual(actual, expectedAchnor.value);
195-
});
196-
197-
it('takes the next anchor up the tree', () => {
198-
const origin = createRef();
199-
const expectedAchnor = createRef();
200-
// clang-format off
201-
renderTemplate(html`
202-
<div class="anchor a"></div>
203-
<div class="anchor b"></div>
204-
<div class="anchor c" ${ref(expectedAchnor)}></div>
205-
<div ${ref(origin)}></div>
206-
<div class="anchor d"></div>
207-
`);
208-
// clang-format on
209-
210-
const actual = closestAnchor(origin.value!, '.anchor');
211-
212-
assert.strictEqual(actual, expectedAchnor.value);
213-
});
214-
});

front_end/ui/components/tooltip/Tooltip.ts renamed to front_end/ui/components/tooltips/Tooltip.ts

Lines changed: 47 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const {html} = Lit;
1111
export type TooltipVariant = 'simple'|'rich';
1212

1313
export interface TooltipProperties {
14-
id?: string;
14+
id: string;
1515
variant?: TooltipVariant;
1616
anchor?: HTMLElement;
1717
}
@@ -65,15 +65,21 @@ export class Tooltip extends HTMLElement {
6565
this.setAttribute('variant', variant);
6666
}
6767

68-
constructor({id, variant, anchor}: TooltipProperties = {}) {
68+
constructor(properties?: TooltipProperties) {
6969
super();
70-
if (id) {
71-
this.id = id;
70+
if (properties) {
71+
this.id = properties.id;
7272
}
73-
if (variant) {
74-
this.variant = variant;
73+
if (properties?.variant) {
74+
this.variant = properties.variant;
75+
}
76+
if (properties?.anchor) {
77+
const ref = properties.anchor.getAttribute('aria-details') ?? properties.anchor.getAttribute('aria-describedby');
78+
if (ref !== properties.id) {
79+
throw new Error('aria-details or aria-describedby must be set on the anchor');
80+
}
81+
this.#anchor = properties.anchor;
7582
}
76-
this.#anchor = anchor ?? null;
7783
}
7884

7985
attributeChangedCallback(name: string): void {
@@ -117,7 +123,7 @@ export class Tooltip extends HTMLElement {
117123
}, this.hoverDelay);
118124
};
119125

120-
hideTooltip = (event: MouseEvent): void => {
126+
hideTooltip = (event: MouseEvent|FocusEvent): void => {
121127
if (this.#timeout) {
122128
window.clearTimeout(this.#timeout);
123129
}
@@ -167,6 +173,8 @@ export class Tooltip extends HTMLElement {
167173
this.#anchor.addEventListener('click', this.toggle);
168174
} else {
169175
this.#anchor.addEventListener('mouseenter', this.showTooltip);
176+
this.#anchor.addEventListener('focus', this.showTooltip);
177+
this.#anchor.addEventListener('blur', this.hideTooltip);
170178
this.#anchor.addEventListener('mouseleave', this.hideTooltip);
171179
this.addEventListener('mouseleave', this.hideTooltip);
172180
}
@@ -179,9 +187,14 @@ export class Tooltip extends HTMLElement {
179187
}
180188

181189
#removeEventListeners(): void {
190+
if (this.#timeout) {
191+
window.clearTimeout(this.#timeout);
192+
}
182193
if (this.#anchor) {
183194
this.#anchor.removeEventListener('click', this.toggle);
184195
this.#anchor.removeEventListener('mouseenter', this.showTooltip);
196+
this.#anchor.removeEventListener('focus', this.showTooltip);
197+
this.#anchor.removeEventListener('blur', this.hideTooltip);
185198
this.#anchor.removeEventListener('mouseleave', this.hideTooltip);
186199
}
187200
this.removeEventListener('mouseleave', this.hideTooltip);
@@ -192,29 +205,35 @@ export class Tooltip extends HTMLElement {
192205
}
193206

194207
#attachToAnchor(): void {
195-
const id = this.getAttribute('id');
196-
if (!id) {
197-
throw new Error('<devtools-tooltip> must have an id.');
198-
}
199-
const describedbyAnchor = closestAnchor(this, `[aria-describedby="${id}"]`);
200-
const detailsAnchor = closestAnchor(this, `[aria-details="${id}"]`);
201-
const anchor = this.#anchor ?? describedbyAnchor ?? detailsAnchor;
202-
if (!anchor) {
203-
throw new Error(`No anchor for tooltip with id ${id} found.`);
204-
}
205-
if (!(anchor instanceof HTMLElement)) {
206-
throw new Error('Anchor must be an HTMLElement.');
207-
}
208-
if (this.variant === 'rich' && describedbyAnchor) {
209-
console.warn(`The anchor for tooltip ${
210-
id} was defined with "aria-describedby". For rich tooltips "aria-details" is more appropriate.`);
208+
if (!this.#anchor) {
209+
const id = this.getAttribute('id');
210+
if (!id) {
211+
throw new Error('<devtools-tooltip> must have an id.');
212+
}
213+
const root = this.getRootNode() as Document | ShadowRoot;
214+
if (root.querySelectorAll(`#${id}`)?.length > 1) {
215+
throw new Error('Duplicate <devtools-tooltip> ids found.');
216+
}
217+
const describedbyAnchor = root.querySelector(`[aria-describedby="${id}"]`);
218+
const detailsAnchor = root.querySelector(`[aria-details="${id}"]`);
219+
const anchor = describedbyAnchor ?? detailsAnchor;
220+
if (!anchor) {
221+
throw new Error(`No anchor for tooltip with id ${id} found.`);
222+
}
223+
if (!(anchor instanceof HTMLElement)) {
224+
throw new Error('Anchor must be an HTMLElement.');
225+
}
226+
this.#anchor = anchor;
227+
if (this.variant === 'rich' && describedbyAnchor) {
228+
console.warn(`The anchor for tooltip ${
229+
id} was defined with "aria-describedby". For rich tooltips "aria-details" is more appropriate.`);
230+
}
211231
}
212232

213-
const anchorName = `--${id}-anchor`;
214-
anchor.style.anchorName = anchorName;
233+
const anchorName = `--devtools-tooltip-${this.id}-anchor`;
234+
this.#anchor.style.anchorName = anchorName;
215235
this.style.positionAnchor = anchorName;
216-
this.#observeAnchorRemoval(anchor);
217-
this.#anchor = anchor;
236+
this.#observeAnchorRemoval(this.#anchor);
218237
}
219238

220239
#observeAnchorRemoval(anchor: Element): void {
@@ -239,15 +258,6 @@ export class Tooltip extends HTMLElement {
239258
}
240259
}
241260

242-
export function closestAnchor(tooltip: Element, selector: string): Element|null {
243-
const anchors: NodeListOf<Element>|undefined = (tooltip.getRootNode() as Element)?.querySelectorAll(selector);
244-
// Find the last anchor with a matching selector that is before the tooltip in the document order.
245-
const anchor = [...anchors ?? []]
246-
.filter(anchor => tooltip.compareDocumentPosition(anchor) & Node.DOCUMENT_POSITION_PRECEDING)
247-
.at(-1);
248-
return anchor ?? null;
249-
}
250-
251261
customElements.define('devtools-tooltip', Tooltip);
252262

253263
declare global {

0 commit comments

Comments
 (0)