Skip to content

Commit 05f82da

Browse files
danilsomsikovDevtools-frontend LUCI CQ
authored andcommitted
Add more toolbar-related imperative DOM API conversions
This change enhances the `no-imperative-dom-api` rule to recognize and convert more imperative DOM API calls related to toolbars. It adds support for: * `appendSeparator` * `appendSpacer` * `setEnabled` * `createOption` * `addOption` * `ToolbarSeparator` constructor * The `wrappable` property on `devtools-toolbar` Bug: 407751409 Change-Id: I614860e226000a9ba4b2b4f6053b84a1177f1694 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7008196 Commit-Queue: Danil Somsikov <dsv@chromium.org> Reviewed-by: Philip Pfaffe <pfaffe@chromium.org> Auto-Submit: Danil Somsikov <dsv@chromium.org> Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
1 parent 5794dc5 commit 05f82da

File tree

2 files changed

+132
-2
lines changed

2 files changed

+132
-2
lines changed

scripts/eslint_rules/lib/no-imperative-dom-api/toolbar.ts

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,14 @@
77

88
import type {TSESTree} from '@typescript-eslint/utils';
99

10-
import {getEnclosingProperty, isIdentifier, isIdentifierChain, isMemberExpression, type RuleCreator} from './ast.ts';
10+
import {
11+
getEnclosingProperty,
12+
isIdentifier,
13+
isIdentifierChain,
14+
isLiteral,
15+
isMemberExpression,
16+
type RuleCreator
17+
} from './ast.ts';
1118
import {ClassMember} from './class-member.ts';
1219
import {DomFragment} from './dom-fragment.ts';
1320

@@ -30,11 +37,53 @@ export const toolbar: RuleCreator = {
3037
return null;
3138
}
3239
},
33-
methodCall(property, firstArg, _secondArg, domFragment) {
40+
methodCall(property, firstArg, secondArg, domFragment, call) {
3441
if (isIdentifier(property, 'appendToolbarItem')) {
3542
domFragment.appendChild(firstArg, sourceCode);
3643
return true;
3744
}
45+
if (isIdentifier(property, 'appendSeparator')) {
46+
const separator = domFragment.appendChild(call, sourceCode);
47+
separator.tagName = 'div';
48+
separator.classList.push('toolbar-divider');
49+
return true;
50+
}
51+
if (isIdentifier(property, 'appendSpacer')) {
52+
const spacer = domFragment.appendChild(call, sourceCode);
53+
spacer.tagName = 'div';
54+
spacer.classList.push('toolbar-spacer');
55+
return true;
56+
}
57+
if (isIdentifier(property, 'setEnabled')) {
58+
domFragment.booleanAttributes.push({
59+
key: 'disabled',
60+
value: firstArg,
61+
});
62+
return true;
63+
}
64+
if (isIdentifier(property, 'createOption')) {
65+
const optionFragment = domFragment.appendChild(call, sourceCode);
66+
optionFragment.tagName = 'option';
67+
optionFragment.textContent = firstArg;
68+
if (secondArg && !isIdentifier(secondArg, 'undefined')) {
69+
optionFragment.attributes.push({
70+
key: 'value',
71+
value: secondArg,
72+
});
73+
}
74+
const jslogContext = call.arguments[2] ?? secondArg;
75+
if (jslogContext) {
76+
optionFragment.attributes.push({
77+
key: 'jslog',
78+
value: `\${VisualLogging.item(` + sourceCode.getText(jslogContext) + `).track({click: true})}`,
79+
});
80+
}
81+
return true;
82+
}
83+
if (isIdentifier(property, 'addOption')) {
84+
domFragment.appendChild(firstArg, sourceCode);
85+
return true;
86+
}
3887
return false;
3988
},
4089
NewExpression(node) {
@@ -222,6 +271,26 @@ export const toolbar: RuleCreator = {
222271
});
223272
}
224273
}
274+
if (isIdentifier(toolbarItem, 'ToolbarSeparator')) {
275+
const domFragment = DomFragment.getOrCreate(node, sourceCode);
276+
domFragment.tagName = 'div';
277+
const spacer = node.arguments[0];
278+
if (spacer && isLiteral(spacer, true)) {
279+
domFragment.classList.push('toolbar-spacer');
280+
} else {
281+
domFragment.classList.push('toolbar-divider');
282+
}
283+
}
284+
},
285+
propertyAssignment(property, propertyValue, domFragment) {
286+
if (domFragment.tagName === 'devtools-toolbar' && isIdentifier(property, 'wrappable')) {
287+
domFragment.booleanAttributes.push({
288+
key: 'wrappable',
289+
value: propertyValue,
290+
});
291+
return true;
292+
}
293+
return false;
225294
},
226295
CallExpression(node) {
227296
const isGetAction = (node: CallExpression) => isMemberExpression(

scripts/eslint_rules/tests/no-imperative-dom-api.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,6 +1293,67 @@ export const DEFAULT_VIEW = (input, _output, target) => {
12931293
target, {host: input});
12941294
};
12951295
1296+
class SomeWidget extends UI.Widget.Widget {
1297+
constructor() {
1298+
super();
1299+
}
1300+
}`,
1301+
errors: [{messageId: 'preferTemplateLiterals'}],
1302+
},
1303+
{
1304+
filename: 'front_end/ui/components/component/file.ts',
1305+
code: `
1306+
class SomeWidget extends UI.Widget.Widget {
1307+
constructor() {
1308+
super();
1309+
const toolbar = this.contentElement.createChild('devtools-toolbar');
1310+
toolbar.wrappable = true;
1311+
toolbar.appendSeparator();
1312+
const combo = new UI.Toolbar.ToolbarComboBox(this.onSelect.bind(this), 'aria-label', undefined, 'combo-box');
1313+
combo.createOption('Option 1', '1', 'option-1');
1314+
const option2 = document.createElement('option');
1315+
option2.value = '2';
1316+
option2.textContent = 'Option 2';
1317+
combo.addOption(option2);
1318+
toolbar.appendToolbarItem(combo);
1319+
toolbar.appendSpacer();
1320+
const button = new UI.Toolbar.ToolbarButton('Click me', 'largeicon-add');
1321+
button.setEnabled(false);
1322+
toolbar.appendToolbarItem(button);
1323+
const otherButton = new UI.Toolbar.ToolbarButton('Other button', 'largeicon-delete');
1324+
otherButton.setEnabled(this.isEnabled);
1325+
toolbar.appendToolbarItem(otherButton);
1326+
toolbar.appendToolbarItem(new UI.Toolbar.ToolbarSeparator());
1327+
toolbar.appendToolbarItem(new UI.Toolbar.ToolbarSeparator(false));
1328+
toolbar.appendToolbarItem(new UI.Toolbar.ToolbarSeparator(true));
1329+
}
1330+
}`,
1331+
output: `
1332+
1333+
export const DEFAULT_VIEW = (input, _output, target) => {
1334+
render(html\`
1335+
<div>
1336+
<devtools-toolbar wrappable>
1337+
<div class="toolbar-divider"></div>
1338+
<select title="aria-label" aria-label="aria-label"
1339+
jslog=\${VisualLogging.dropDown('combo-box').track({change: true})}
1340+
@change=\${this.onSelect.bind(this)}>
1341+
<option value="1" jslog=\${VisualLogging.item('option-1').track({click: true})}>Option 1</option>
1342+
<option value="2">Option 2</option>
1343+
</select>
1344+
<div class="toolbar-spacer"></div>
1345+
<devtools-button title="Click me" .variant=\${Buttons.Button.Variant.TOOLBAR}
1346+
.iconName=\${'largeicon-add'}></devtools-button>
1347+
<devtools-button title="Other button" ?disabled=\${this.isEnabled}
1348+
.variant=\${Buttons.Button.Variant.TOOLBAR} .iconName=\${'largeicon-delete'}></devtools-button>
1349+
<div class="toolbar-divider"></div>
1350+
<div class="toolbar-divider"></div>
1351+
<div class="toolbar-spacer"></div>
1352+
</devtools-toolbar>
1353+
</div>\`,
1354+
target, {host: input});
1355+
};
1356+
12961357
class SomeWidget extends UI.Widget.Widget {
12971358
constructor() {
12981359
super();

0 commit comments

Comments
 (0)