Skip to content

Commit 1730350

Browse files
pfaffeDevtools-frontend LUCI CQ
authored andcommitted
[styles] Make var() keyboard-interactable
Get rid of the VarSwatch indirection which wraps around the var link, so that we can focus the var link. Activating the link through the keyboard is possible. Fixed: 401212692 Change-Id: I95cfaade34795d89a7b5a07f828f6f44aba06efe Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6297887 Auto-Submit: Philip Pfaffe <[email protected]> Reviewed-by: Eric Leese <[email protected]> Reviewed-by: Changhao Han <[email protected]> Commit-Queue: Eric Leese <[email protected]>
1 parent 411a6ff commit 1730350

File tree

8 files changed

+118
-257
lines changed

8 files changed

+118
-257
lines changed

front_end/panels/elements/CSSValueTraceView.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ async function showTrace(
6767
treeElement: Elements.StylePropertyTreeElement.StylePropertyTreeElement):
6868
Promise<Elements.CSSValueTraceView.ViewInput> {
6969
const viewFunction = createViewFunctionStub(Elements.CSSValueTraceView.CSSValueTraceView);
70-
const view = new Elements.CSSValueTraceView.CSSValueTraceView(viewFunction);
70+
const view = new Elements.CSSValueTraceView.CSSValueTraceView(undefined, viewFunction);
7171
await viewFunction.nextInput;
7272
view.showTrace(
7373
property, matchedStyles, new Map(),

front_end/panels/elements/CSSValueTraceView.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ export class CSSValueTraceView extends UI.Widget.VBox {
8080
#evaluations: Node[][] = [];
8181
#substitutions: Node[][] = [];
8282

83-
constructor(view: View = defaultView) {
84-
super(true);
83+
constructor(element?: HTMLElement, view: View = defaultView) {
84+
super(true, false, element);
8585
this.registerRequiredCSS(
8686
cssValueTraceViewStyles,
8787
stylePropertiesTreeOutlineStyles,

front_end/panels/elements/StylePropertiesSection.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1454,7 +1454,7 @@ export class StylePropertiesSection {
14541454

14551455
startEditingSelector(): void {
14561456
const element = this.selectorElement;
1457-
if (UI.UIUtils.isBeingEdited(element)) {
1457+
if (UI.UIUtils.isBeingEdited(element) || this.titleElement.classList.contains('hidden')) {
14581458
return;
14591459
}
14601460

front_end/panels/elements/StylePropertyTreeElement.test.ts

Lines changed: 45 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,10 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
169169

170170
const colorMixSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-color-mix-swatch');
171171
const cssVarSwatches =
172-
Array.from(stylePropertyTreeElement.valueElement?.querySelectorAll('devtools-css-var-swatch') || []);
172+
Array.from(stylePropertyTreeElement.valueElement?.querySelectorAll('.css-var-link') || []);
173173
assert.exists(colorMixSwatch);
174-
assert.exists(cssVarSwatches.find(cssVarSwatch => cssVarSwatch.textContent === 'var(--a)'));
175-
assert.exists(cssVarSwatches.find(cssVarSwatch => cssVarSwatch.textContent === 'var(--b)'));
174+
assert.exists(cssVarSwatches.find(cssVarSwatch => cssVarSwatch.textContent === '--a'));
175+
assert.exists(cssVarSwatches.find(cssVarSwatch => cssVarSwatch.textContent === '--b'));
176176
});
177177

178178
it('should not show color mix swatch when color-mix is used with an unknown variable as color', () => {
@@ -519,12 +519,11 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
519519

520520
const varSwatch =
521521
renderValueSpy.returnValues
522-
.find(value => value.valueElement.firstChild instanceof InlineEditor.LinkSwatch.CSSVarSwatch)
523-
?.valueElement.firstChild as InlineEditor.LinkSwatch.CSSVarSwatch |
524-
undefined;
522+
.map(fragment => Array.from(fragment.valueElement.querySelectorAll('devtools-base-link-swatch')))
523+
.flat()[0];
525524
assert.exists(varSwatch);
526525
const revealPropertySpy = sinon.spy(stylesSidebarPane, 'revealProperty');
527-
varSwatch.link?.linkElement?.click();
526+
varSwatch.linkElement?.click();
528527
assert.isTrue(revealPropertySpy.calledWith(cssCustomPropertyDef));
529528
});
530529

@@ -566,12 +565,11 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
566565

567566
const varSwatch =
568567
renderValueSpy.returnValues
569-
.find(value => value.valueElement.firstChild instanceof InlineEditor.LinkSwatch.CSSVarSwatch)
570-
?.valueElement.firstChild as InlineEditor.LinkSwatch.CSSVarSwatch |
571-
undefined;
568+
.map(fragment => Array.from(fragment.valueElement.querySelectorAll('devtools-base-link-swatch')))
569+
.flat()[0];
572570
assert.exists(varSwatch);
573571
const jumpToPropertySpy = sinon.spy(stylesSidebarPane, 'jumpToProperty');
574-
varSwatch.link?.linkElement?.click();
572+
varSwatch.linkElement?.click();
575573
assert.isTrue(jumpToPropertySpy.calledWith(
576574
'initial-value', '--prop', Elements.StylesSidebarPane.REGISTERED_PROPERTY_SECTION_NAME));
577575
});
@@ -583,12 +581,12 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
583581
stylePropertyTreeElement.updateTitle();
584582
assert.exists(stylePropertyTreeElement.valueElement);
585583

586-
const cssVarSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-css-var-swatch');
587-
assert.exists(cssVarSwatch);
588-
589-
const linkSwatch = cssVarSwatch.shadowRoot?.querySelector('devtools-base-link-swatch');
584+
const linkSwatch = stylePropertyTreeElement.valueElement?.querySelector('.css-var-link');
590585
assert.exists(linkSwatch);
591586

587+
const cssVarSwatch = linkSwatch.parentElement;
588+
assert.exists(cssVarSwatch);
589+
592590
assert.strictEqual(cssVarSwatch.textContent, 'var(--a)');
593591
assert.strictEqual(linkSwatch.shadowRoot?.textContent, '--a');
594592
assert.strictEqual(stylePropertyTreeElement.valueElement.textContent, 'var(--a)');
@@ -599,12 +597,12 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
599597
stylePropertyTreeElement.updateTitle();
600598
assert.exists(stylePropertyTreeElement.valueElement);
601599

602-
const cssVarSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-css-var-swatch');
603-
assert.exists(cssVarSwatch);
604-
605-
const linkSwatch = cssVarSwatch.shadowRoot?.querySelector('devtools-base-link-swatch');
600+
const linkSwatch = stylePropertyTreeElement.valueElement?.querySelector('.css-var-link');
606601
assert.exists(linkSwatch);
607602

603+
const cssVarSwatch = linkSwatch.parentElement;
604+
assert.exists(cssVarSwatch);
605+
608606
assert.strictEqual(linkSwatch.shadowRoot?.textContent, '--not-existing');
609607
assert.strictEqual(cssVarSwatch.deepTextContent(), 'var(--not-existing, red)');
610608
assert.strictEqual(stylePropertyTreeElement.valueElement.textContent, 'var(--not-existing, red)');
@@ -615,12 +613,15 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
615613
stylePropertyTreeElement.updateTitle();
616614
assert.exists(stylePropertyTreeElement.valueElement);
617615

618-
const cssVarSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-css-var-swatch');
616+
const [firstLinkSwatch, secondLinkSwatch] =
617+
stylePropertyTreeElement.valueElement?.querySelectorAll('.css-var-link');
618+
assert.exists(firstLinkSwatch);
619+
assert.exists(secondLinkSwatch);
620+
const cssVarSwatch = firstLinkSwatch.parentElement;
619621
assert.exists(cssVarSwatch);
622+
const insideCssVarSwatch = secondLinkSwatch.parentElement;
623+
assert.exists(insideCssVarSwatch);
620624

621-
const firstLinkSwatch = cssVarSwatch.shadowRoot?.querySelector('devtools-base-link-swatch');
622-
const insideCssVarSwatch = cssVarSwatch.querySelector('devtools-css-var-swatch');
623-
const secondLinkSwatch = insideCssVarSwatch?.shadowRoot?.querySelector('devtools-base-link-swatch');
624625
assert.strictEqual(stylePropertyTreeElement.valueElement.textContent, 'var(--not-existing, var(--a))');
625626
assert.strictEqual(firstLinkSwatch?.shadowRoot?.textContent, '--not-existing');
626627
assert.strictEqual(cssVarSwatch.textContent, 'var(--not-existing, var(--a))');
@@ -633,13 +634,13 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
633634
stylePropertyTreeElement.updateTitle();
634635
assert.exists(stylePropertyTreeElement.valueElement);
635636

636-
const cssVarSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-css-var-swatch');
637+
const linkSwatch = stylePropertyTreeElement.valueElement?.querySelector('.css-var-link');
638+
assert.exists(linkSwatch);
639+
const cssVarSwatch = linkSwatch.parentElement;
637640
assert.exists(cssVarSwatch);
638641

639-
const firstLinkSwatch = cssVarSwatch.shadowRoot?.querySelector('devtools-base-link-swatch');
640-
641642
assert.strictEqual(stylePropertyTreeElement.valueElement.textContent, 'var(--not-existing, calc(15px + 20px))');
642-
assert.strictEqual(firstLinkSwatch?.shadowRoot?.textContent, '--not-existing');
643+
assert.strictEqual(linkSwatch?.shadowRoot?.textContent, '--not-existing');
643644
assert.strictEqual(cssVarSwatch.textContent, 'var(--not-existing, calc(15px + 20px))');
644645
});
645646

@@ -649,17 +650,17 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
649650
stylePropertyTreeElement.updateTitle();
650651
assert.exists(stylePropertyTreeElement.valueElement);
651652

652-
const cssVarSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-css-var-swatch');
653-
assert.exists(cssVarSwatch);
654-
655-
const colorSwatch = cssVarSwatch.querySelector('devtools-color-swatch');
653+
const colorSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-color-swatch');
656654
assert.exists(colorSwatch);
657655
assert.isTrue(InlineEditor.ColorSwatch.ColorSwatch.isColorSwatch(colorSwatch));
658656

659-
const firstLinkSwatch = cssVarSwatch.shadowRoot?.querySelector('devtools-base-link-swatch');
657+
const linkSwatch = stylePropertyTreeElement.valueElement?.querySelector('.css-var-link');
658+
assert.exists(linkSwatch);
659+
const cssVarSwatch = linkSwatch.parentElement;
660+
assert.exists(cssVarSwatch);
660661

661662
assert.strictEqual(stylePropertyTreeElement.valueElement.textContent, `var(${varName}, var(--blue))`);
662-
assert.strictEqual(firstLinkSwatch?.shadowRoot?.textContent, varName);
663+
assert.strictEqual(linkSwatch?.shadowRoot?.textContent, varName);
663664
assert.strictEqual(cssVarSwatch.textContent, `var(${varName}, var(--blue))`);
664665
}
665666
});
@@ -668,31 +669,20 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
668669
const stylePropertyTreeElement = getTreeElement('--shadow', 'var(--a) var(--b)');
669670
stylePropertyTreeElement.updateTitle();
670671

671-
const cssVarSwatches = stylePropertyTreeElement.valueElement?.querySelectorAll('devtools-css-var-swatch');
672+
const cssVarSwatches = stylePropertyTreeElement.valueElement?.querySelectorAll('.css-var-link');
672673
assert.strictEqual(cssVarSwatches?.length, 2);
673674
});
674675

675-
it('should render a CSSVarSwatch for var() with spaces', () => {
676-
const stylePropertyTreeElement = getTreeElement('color', 'var( --test )');
677-
stylePropertyTreeElement.updateTitle();
678-
assert.exists(stylePropertyTreeElement.valueElement);
679-
680-
const cssVarSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-css-var-swatch');
681-
assert.exists(cssVarSwatch);
682-
683-
const linkSwatch = cssVarSwatch.shadowRoot?.querySelector('devtools-base-link-swatch');
684-
assert.strictEqual(linkSwatch?.shadowRoot?.textContent, '--test');
685-
assert.strictEqual(cssVarSwatch.textContent, 'var( --test )');
686-
assert.strictEqual(stylePropertyTreeElement.valueElement.textContent, 'var( --test )');
687-
});
688-
689676
it('connects nested color swatches', () => {
690677
const stylePropertyTreeElement = getTreeElement('color', 'var(--void, red)');
691678
stylePropertyTreeElement.updateTitle();
692679
assert.exists(stylePropertyTreeElement.valueElement);
693680

694-
const cssVarSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-css-var-swatch');
681+
const linkSwatch = stylePropertyTreeElement.valueElement?.querySelector('.css-var-link');
682+
assert.exists(linkSwatch);
683+
const cssVarSwatch = linkSwatch.parentElement;
695684
assert.exists(cssVarSwatch);
685+
696686
const outerColorSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-color-swatch');
697687
assert.exists(outerColorSwatch);
698688
const innerColorSwatch = cssVarSwatch.querySelector('devtools-color-swatch');
@@ -708,8 +698,11 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
708698
stylePropertyTreeElement.updateTitle();
709699
assert.exists(stylePropertyTreeElement.valueElement);
710700

711-
const cssVarSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-css-var-swatch');
701+
const linkSwatch = stylePropertyTreeElement.valueElement?.querySelector('.css-var-link');
702+
assert.exists(linkSwatch);
703+
const cssVarSwatch = linkSwatch.parentElement;
712704
assert.exists(cssVarSwatch);
705+
713706
const outerColorSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-color-swatch');
714707
assert.exists(outerColorSwatch);
715708
const innerColorSwatch = cssVarSwatch.querySelector('devtools-color-swatch');
@@ -757,7 +750,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
757750
it('layers correctly with the font renderer', () => {
758751
const stylePropertyTreeElement = getTreeElement('font-size', 'calc(1 + var(--no))');
759752
stylePropertyTreeElement.updateTitle();
760-
assert.exists(stylePropertyTreeElement.valueElement?.querySelector('devtools-css-var-swatch'));
753+
assert.exists(stylePropertyTreeElement.valueElement?.querySelector('.css-var-link'));
761754
});
762755
});
763756

@@ -772,7 +765,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
772765
assert.exists(colorSwatch);
773766
assert.strictEqual(colorSwatch.getColor()?.asString(Common.Color.Format.HEX), '#ff0000');
774767

775-
const varSwatches = stylePropertyTreeElement.valueElement?.querySelectorAll('devtools-css-var-swatch');
768+
const varSwatches = stylePropertyTreeElement.valueElement?.querySelectorAll('.css-var-link');
776769
assert.exists(varSwatches);
777770
assert.lengthOf(varSwatches, 2);
778771
});

front_end/panels/elements/StylePropertyTreeElement.ts

Lines changed: 50 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import * as Tooltips from '../../ui/components/tooltips/tooltips.js';
1616
import * as ColorPicker from '../../ui/legacy/components/color_picker/color_picker.js';
1717
import * as InlineEditor from '../../ui/legacy/components/inline_editor/inline_editor.js';
1818
import * as UI from '../../ui/legacy/legacy.js';
19+
import {html, nothing, render} from '../../ui/lit/lit.js';
1920
import * as VisualLogging from '../../ui/visual_logging/visual_logging.js';
2021

2122
import {
@@ -253,6 +254,9 @@ export class VariableRenderer extends rendererBase(SDK.CSSPropertyParserMatchers
253254
const {declaration, value: variableValue} = match.resolveVariable() ?? {};
254255
const fromFallback = variableValue === undefined;
255256
const computedValue = variableValue ?? match.fallbackValue();
257+
const onLinkActivate = (name: string): void => this.#handleVarDefinitionActivate(declaration ?? name);
258+
const linkTitle = computedValue ?? i18nString(UIStrings.sIsNotDefined, {PH1: match.name});
259+
const varSwatch = document.createElement('span');
256260

257261
const substitution = context.tracing?.substitution();
258262
if (substitution) {
@@ -273,41 +277,57 @@ export class VariableRenderer extends rendererBase(SDK.CSSPropertyParserMatchers
273277
}
274278

275279
const renderedFallback = match.fallback.length > 0 ? Renderer.render(match.fallback, context) : undefined;
276-
277-
const varSwatch = new InlineEditor.LinkSwatch.CSSVarSwatch();
278-
varSwatch.data = {
279-
computedValue,
280-
variableName: match.name,
281-
fromFallback,
282-
fallbackText: match.fallback.map(n => context.ast.text(n)).join(' '),
283-
onLinkActivate: name => this.#handleVarDefinitionActivate(declaration ?? name),
284-
};
285-
286-
if (renderedFallback?.nodes.length) {
287-
// When slotting someting into the fallback slot, also emit text children so that .textContent produces the
288-
// correct var value.
289-
varSwatch.appendChild(document.createTextNode(`var(${match.name}`));
290-
const span = varSwatch.appendChild(document.createElement('span'));
291-
span.appendChild(document.createTextNode(', '));
292-
span.slot = 'fallback';
293-
renderedFallback.nodes.forEach(n => span.appendChild(n));
294-
varSwatch.appendChild(document.createTextNode(')'));
280+
let traceWidget: CSSValueTraceView|undefined;
281+
let tooltipContents;
282+
if (Root.Runtime.hostConfig.devToolsCssValueTracing?.enabled &&
283+
declaration?.declaration instanceof SDK.CSSProperty.CSSProperty) {
284+
tooltipContents = html`<devtools-widget
285+
.widgetConfig=${UI.Widget.widgetConfig(CSSValueTraceView, {})}
286+
${UI.Widget.widgetRef(CSSValueTraceView, e => {
287+
traceWidget = e;
288+
})}
289+
></devtools-widget>`;
295290
} else {
296-
UI.UIUtils.createTextChild(varSwatch, match.text);
297-
}
291+
tooltipContents =
292+
this.#stylesPane.getVariablePopoverContents(this.#matchedStyles, match.name, variableValue ?? null);
293+
}
294+
const toggleTooltip = (e: ToggleEvent): void => {
295+
if (e.newState === 'open' && Root.Runtime.hostConfig.devToolsCssValueTracing?.enabled &&
296+
declaration?.declaration instanceof SDK.CSSProperty.CSSProperty) {
297+
const property = declaration.declaration;
298+
traceWidget?.showTrace(
299+
property, this.#matchedStyles, this.#computedStyles,
300+
getPropertyRenderers(
301+
property.ownerStyle, this.#stylesPane, this.#matchedStyles, this.#treeElement, this.#computedStyles));
302+
}
303+
};
298304

299305
const tooltipId = swatchTooltipId();
300-
varSwatch.setAttribute('aria-details', tooltipId);
301-
const tooltip = new Tooltips.Tooltip.Tooltip(
302-
{anchor: varSwatch, variant: 'rich', id: tooltipId, jslogContext: 'elements.css-var'});
303-
tooltip.appendChild(
304-
(varSwatch.link &&
305-
this.#stylesPane.getVariablePopoverContents(this.#matchedStyles, match.name, variableValue ?? null)) ||
306-
document.createTextNode(i18nString(UIStrings.sIsNotDefined, {PH1: match.name})));
306+
render(
307+
html`<span data-title=${computedValue || ''}
308+
jslog=${VisualLogging.link('css-variable').track({
309+
click: true,
310+
hover: true
311+
})}>var(<devtools-base-link-swatch
312+
class=css-var-link
313+
aria-details=${tooltipId}
314+
.data=${{
315+
text: match.name,
316+
isDefined: computedValue !== null && !fromFallback,
317+
title: linkTitle,
318+
showTitle: false,
319+
onLinkActivate,
320+
} as InlineEditor.LinkSwatch.BaseLinkSwatchRenderData}>${match.name}</devtools-base-link-swatch>${
321+
renderedFallback?.nodes.length ? html`, ${renderedFallback.nodes}` : nothing})</span><devtools-tooltip
322+
variant=rich
323+
id=${tooltipId}
324+
@beforetoggle=${toggleTooltip}
325+
jslogContext=elements.css-var>${tooltipContents}</devtools-tooltip>`,
326+
varSwatch);
307327

308328
const color = computedValue && Common.Color.parse(computedValue);
309329
if (!color) {
310-
return [varSwatch, tooltip];
330+
return [varSwatch];
311331
}
312332

313333
const colorSwatch = new ColorRenderer(this.#stylesPane, this.#treeElement).renderColorSwatch(color, varSwatch);
@@ -320,7 +340,7 @@ export class VariableRenderer extends rendererBase(SDK.CSSPropertyParserMatchers
320340
}));
321341
}
322342

323-
return [colorSwatch, tooltip];
343+
return [colorSwatch];
324344
}
325345

326346
#handleVarDefinitionActivate(variable: string|SDK.CSSMatchedStyles.CSSValueSource): void {

0 commit comments

Comments
 (0)