Skip to content

Commit c83eac3

Browse files
Nancy LiDevtools-frontend LUCI CQ
authored andcommitted
[RPP Icicle blowtorch] Preview the regex result when users are typing
The idea is when user focus on the input, add a new rule to the end of existing rules. And update it when user is typing. Then when user finish editing, pop the last regex, then do the normal add regex to ignore process including the validation check. Screencast: https://screencast.googleplex.com/cast/NDYzNjgxMTI4NzU5Mjk2MHxhYTBhZDEyZi05YQ Bug:376657824 Change-Id: Ie57241c99fc2865e08586a6e70b79d284c4ec4d5 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6070377 Reviewed-by: Jack Franklin <[email protected]> Commit-Queue: Nancy Li <[email protected]>
1 parent d3fe5eb commit c83eac3

File tree

3 files changed

+167
-25
lines changed

3 files changed

+167
-25
lines changed

front_end/panels/timeline/components/IgnoreListSetting.test.ts

Lines changed: 98 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@ import * as Common from '../../../core/common/common.js';
66
import * as SDK from '../../../core/sdk/sdk.js';
77
import * as Bindings from '../../../models/bindings/bindings.js';
88
import * as Workspace from '../../../models/workspace/workspace.js';
9-
import {renderElementIntoDOM} from '../../../testing/DOMHelpers.js';
9+
import {
10+
dispatchBlurEvent,
11+
dispatchFocusEvent,
12+
dispatchInputEvent,
13+
renderElementIntoDOM,
14+
} from '../../../testing/DOMHelpers.js';
1015
import {describeWithEnvironment} from '../../../testing/EnvironmentHelpers.js';
1116
import * as Coordinator from '../../../ui/components/render_coordinator/render_coordinator.js';
1217

@@ -146,7 +151,7 @@ describeWithEnvironment('Ignore List Setting', () => {
146151
const newRegexInput = getNewRegexInput(component);
147152

148153
newRegexInput.value = 'rule 1';
149-
newRegexInput.dispatchEvent(new FocusEvent('blur'));
154+
dispatchBlurEvent(newRegexInput);
150155

151156
assert.isTrue(isRegexInIgnoredList('rule 1'));
152157
});
@@ -159,10 +164,90 @@ describeWithEnvironment('Ignore List Setting', () => {
159164
const newRegexInput = getNewRegexInput(component);
160165

161166
newRegexInput.value = 'rule 1';
162-
newRegexInput.dispatchEvent(new FocusEvent('blur'));
167+
dispatchBlurEvent(newRegexInput);
163168

164169
assert.isFalse(isIgnoreRegexDisabled('rule 1'));
165170
});
171+
172+
describe('preview the result', () => {
173+
it('Add an empty regex when focusing on the input', async () => {
174+
const regexPatterns = getIgnoredRegexes();
175+
// There is a default rule `/node_modules/|/bower_components/`, and the 'rule 1' we added.
176+
assert.strictEqual(regexPatterns.length, 2);
177+
178+
const component = await renderIgnoreListSetting();
179+
const newRegexInput = getNewRegexInput(component);
180+
181+
dispatchFocusEvent(newRegexInput);
182+
assert.strictEqual(regexPatterns.length, 3);
183+
184+
// We need this to simulate the 'finish editing', so it can remove the temp regex. Otherwise the future tests will
185+
// be messed up.
186+
// The 'finish editing' part will be tested later
187+
dispatchBlurEvent(newRegexInput);
188+
});
189+
190+
it('Update the regex when user typing', async () => {
191+
const regexPatterns = getIgnoredRegexes();
192+
// There is a default rule `/node_modules/|/bower_components/`, and the 'rule 1' we added.
193+
assert.strictEqual(regexPatterns.length, 2);
194+
195+
const component = await renderIgnoreListSetting();
196+
const newRegexInput = getNewRegexInput(component);
197+
198+
dispatchFocusEvent(newRegexInput);
199+
assert.strictEqual(regexPatterns.length, 3);
200+
// After the focus event, the temp regex (last one) is still empty.
201+
assert.strictEqual(regexPatterns[2].pattern, '');
202+
// Simulate user's typing
203+
newRegexInput.value = 'r';
204+
dispatchInputEvent(newRegexInput);
205+
// After the input event, the temp regex (last one) is updated.
206+
assert.strictEqual(regexPatterns[2].pattern, 'r');
207+
208+
// We need this to simulate the 'finish editing' with empty input, so it can remove the temp regex. Otherwise the
209+
// future tests will be messed up.
210+
// The 'finish editing' part will be tested later
211+
newRegexInput.value = '';
212+
dispatchBlurEvent(newRegexInput);
213+
});
214+
215+
it('Add the regex when user finish typing', async () => {
216+
const regexPatterns = getIgnoredRegexes();
217+
// There is a default rule `/node_modules/|/bower_components/`, and the 'rule 1' we added.
218+
assert.strictEqual(regexPatterns.length, 2);
219+
220+
const component = await renderIgnoreListSetting();
221+
const newRegexInput = getNewRegexInput(component);
222+
223+
dispatchFocusEvent(newRegexInput);
224+
newRegexInput.value = 'rule 2';
225+
assert.strictEqual(regexPatterns.length, 3);
226+
227+
dispatchBlurEvent(newRegexInput);
228+
// When add a valid rule, the temp regex won't be removed.
229+
assert.strictEqual(regexPatterns.length, 3);
230+
assert.strictEqual(regexPatterns[2].pattern, 'rule 2');
231+
});
232+
233+
it('Remove the invalid regex when user finish typing', async () => {
234+
const regexPatterns = getIgnoredRegexes();
235+
// There is a default rule `/node_modules/|/bower_components/`, and the 'rule 1', 'rule 2' we added.
236+
assert.strictEqual(regexPatterns.length, 3);
237+
238+
const component = await renderIgnoreListSetting();
239+
const newRegexInput = getNewRegexInput(component);
240+
241+
dispatchFocusEvent(newRegexInput);
242+
// This is a duplicate rule, so it is invalid.
243+
newRegexInput.value = 'rule 2';
244+
assert.strictEqual(regexPatterns.length, 4);
245+
246+
dispatchBlurEvent(newRegexInput);
247+
// When add an invalid rule, the temp regex will be removed.
248+
assert.strictEqual(regexPatterns.length, 3);
249+
});
250+
});
166251
});
167252

168253
describeWithEnvironment('Pattern validator', () => {
@@ -204,17 +289,19 @@ describeWithEnvironment('Pattern validator', () => {
204289
});
205290
});
206291

292+
function getIgnoredRegexes(): Common.Settings.RegExpSettingItem[] {
293+
return (Common.Settings.Settings.instance().moduleSetting('skip-stack-frames-pattern') as
294+
Common.Settings.RegExpSetting)
295+
.getAsArray();
296+
}
297+
207298
function ignoreRegex(regexValue: string): void {
208-
const regexPatterns =
209-
(Common.Settings.Settings.instance().moduleSetting('skip-stack-frames-pattern') as Common.Settings.RegExpSetting)
210-
.getAsArray();
299+
const regexPatterns = getIgnoredRegexes();
211300
regexPatterns.push({pattern: regexValue, disabled: false});
212301
}
213302

214303
function disableIgnoreRegex(regexValue: string): void {
215-
const regexPatterns =
216-
(Common.Settings.Settings.instance().moduleSetting('skip-stack-frames-pattern') as Common.Settings.RegExpSetting)
217-
.getAsArray();
304+
const regexPatterns = getIgnoredRegexes();
218305
for (const regexPattern of regexPatterns) {
219306
if (regexPattern.pattern === regexValue) {
220307
regexPattern.disabled = true;
@@ -224,9 +311,7 @@ function disableIgnoreRegex(regexValue: string): void {
224311
}
225312

226313
function isIgnoreRegexDisabled(regexValue: string): boolean {
227-
const regexPatterns =
228-
(Common.Settings.Settings.instance().moduleSetting('skip-stack-frames-pattern') as Common.Settings.RegExpSetting)
229-
.getAsArray();
314+
const regexPatterns = getIgnoredRegexes();
230315
for (const regexPattern of regexPatterns) {
231316
if (regexPattern.pattern === regexValue) {
232317
return regexPattern.disabled ?? false;
@@ -239,9 +324,7 @@ function isIgnoreRegexDisabled(regexValue: string): boolean {
239324
* Returns if the regex is in the ignore list, no matter if it is disabled.
240325
*/
241326
function isRegexInIgnoredList(regexValue: string): boolean {
242-
const regexPatterns =
243-
(Common.Settings.Settings.instance().moduleSetting('skip-stack-frames-pattern') as Common.Settings.RegExpSetting)
244-
.getAsArray();
327+
const regexPatterns = getIgnoredRegexes();
245328
for (const regexPattern of regexPatterns) {
246329
if (regexPattern.pattern === regexValue) {
247330
return true;

front_end/panels/timeline/components/IgnoreListSetting.ts

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ export class IgnoreListSetting extends HTMLElement {
8080
#newItemInput = UI.UIUtils.createInput(
8181
/* className*/ 'new-regex-text-input', /* type*/ 'text', /* jslogContext*/ 'timeline.ignore-list-new-regex.text');
8282

83+
#editingRegexSetting: Common.Settings.RegExpSettingItem|null = null;
84+
8385
constructor() {
8486
super();
8587

@@ -107,31 +109,83 @@ export class IgnoreListSetting extends HTMLElement {
107109
Common.Settings.RegExpSetting;
108110
}
109111

112+
#startEditing(): void {
113+
this.#editingRegexSetting = {pattern: this.#newItemInput.value, disabled: false, disabledForUrl: undefined};
114+
// We need to push the temp regex here to update the flame chart.
115+
// We are using the "skip-stack-frames-pattern" setting to determine which is rendered on flame chart. And the push
116+
// here will update the setting's value.
117+
this.#regexPatterns.push(this.#editingRegexSetting);
118+
}
119+
120+
#finishEditing(): void {
121+
if (!this.#editingRegexSetting) {
122+
return;
123+
}
124+
125+
const lastRegex = this.#regexPatterns.pop();
126+
// Add a sanity check to make sure the last one is the editing one.
127+
// In case the check fails, add back the last element.
128+
if (lastRegex && lastRegex !== this.#editingRegexSetting) {
129+
console.warn('The last regex is not the editing one.');
130+
this.#regexPatterns.push(lastRegex);
131+
}
132+
133+
this.#editingRegexSetting = null;
134+
this.#getSkipStackFramesPatternSetting().setAsArray(this.#regexPatterns);
135+
}
136+
137+
#resetInput(): void {
138+
this.#newItemCheckbox.checkboxElement.checked = false;
139+
this.#newItemInput.value = '';
140+
}
141+
110142
#onNewRegexAdded(): void {
111143
const newRegex = this.#newItemInput.value.trim();
112-
const {valid} = patternValidator(this.#regexPatterns, newRegex);
144+
145+
this.#finishEditing();
146+
const {valid} = patternValidator(this.#getExistingRegexes(), newRegex);
113147
if (!valid) {
114148
// It the new regex is invalid, let's skip it.
115149
return;
116150
}
117151
Bindings.IgnoreListManager.IgnoreListManager.instance().addRegexToIgnoreList(newRegex);
118-
this.#newItemCheckbox.checkboxElement.checked = false;
119-
this.#newItemInput.value = '';
152+
this.#resetInput();
120153
}
121154

122155
#handleKeyDown(event: KeyboardEvent): void {
123-
// We do not want to create multi-line labels.
124-
// Therefore, if the new key is `Enter` key, treat it
125-
// as the end of the label input and blur the input field.
156+
// When user press the 'Enter' or 'Escape', the current regex will be added and user can keep adding more regexes.
126157
if (event.key === 'Enter' || event.key === 'Escape') {
127-
this.#newItemInput.dispatchEvent(new FocusEvent('blur'));
158+
this.#onNewRegexAdded();
159+
this.#startEditing();
128160
return;
129161
}
130162
}
131163

164+
/**
165+
* When it is in the 'preview' mode, the last regex in the array is the editing one.
166+
* So we want to remove it for some usage, like rendering the existed rules or validating the rules.
167+
*/
168+
#getExistingRegexes(): Common.Settings.RegExpSettingItem[] {
169+
if (this.#editingRegexSetting) {
170+
const lastRegex = this.#regexPatterns[this.#regexPatterns.length - 1];
171+
172+
// Add a sanity check to make sure the last one is the editing one.
173+
if (lastRegex && lastRegex === this.#editingRegexSetting) {
174+
// We don't want to modify the array itself, so just return a shadow copy of it.
175+
return this.#regexPatterns.slice(0, -1);
176+
}
177+
}
178+
return this.#regexPatterns;
179+
}
180+
132181
#handleInputChange(): void {
133182
// Enable the rule if the text input field is not empty.
134183
this.#newItemCheckbox.checkboxElement.checked = Boolean(this.#newItemInput.value.trim());
184+
185+
if (this.#editingRegexSetting) {
186+
this.#editingRegexSetting.pattern = this.#newItemInput.value.trim();
187+
this.#getSkipStackFramesPatternSetting().setAsArray(this.#regexPatterns);
188+
}
135189
}
136190

137191
#initAddNewItem(): void {
@@ -143,6 +197,7 @@ export class IgnoreListSetting extends HTMLElement {
143197
this.#newItemInput.addEventListener('blur', this.#onNewRegexAdded.bind(this), false);
144198
this.#newItemInput.addEventListener('keydown', this.#handleKeyDown.bind(this), false);
145199
this.#newItemInput.addEventListener('input', this.#handleInputChange.bind(this), false);
200+
this.#newItemInput.addEventListener('focus', this.#startEditing.bind(this), false);
146201
}
147202

148203
#onRegexEnableToggled(regex: Common.Settings.RegExpSettingItem, checkbox: UI.UIUtils.CheckboxLabel): void {
@@ -206,7 +261,7 @@ export class IgnoreListSetting extends HTMLElement {
206261
} as Dialogs.ButtonDialog.ButtonDialogData}>
207262
<div class='ignore-list-setting-content'>
208263
<div class='ignore-list-setting-description'>${i18nString(UIStrings.ignoreListDescription)}</div>
209-
${this.#regexPatterns.map(this.#renderItem.bind(this))}
264+
${this.#getExistingRegexes().map(this.#renderItem.bind(this))}
210265
<div class='new-regex-row'>${this.#newItemCheckbox}${this.#newItemInput}</div>
211266
</div>
212267
</devtools-button-dialog>
@@ -245,8 +300,7 @@ export function patternValidator(
245300

246301
for (let i = 0; i < existingRegexes.length; ++i) {
247302
const regexPattern = existingRegexes[i];
248-
if (regexPattern.pattern === pattern && regexPattern.disabled === false &&
249-
regexPattern.disabledForUrl === undefined) {
303+
if (regexPattern.pattern === pattern && !regexPattern.disabled && regexPattern.disabledForUrl === undefined) {
250304
return {valid: false, errorMessage: i18nString(UIStrings.patternAlreadyExists)};
251305
}
252306
}

front_end/testing/DOMHelpers.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,11 @@ export function dispatchMouseUpEvent<T extends Element>(element: T, options: Mou
142142
element.dispatchEvent(clickEvent);
143143
}
144144

145+
export function dispatchBlurEvent<T extends Element>(element: T, options: FocusEventInit = {}) {
146+
const focusEvent = new FocusEvent('blur', options);
147+
element.dispatchEvent(focusEvent);
148+
}
149+
145150
export function dispatchFocusEvent<T extends Element>(element: T, options: FocusEventInit = {}) {
146151
const focusEvent = new FocusEvent('focus', options);
147152
element.dispatchEvent(focusEvent);

0 commit comments

Comments
 (0)