[RPP Icicle blowtorch] Show the invalid status of current input

Bug: 382272853
Change-Id: Ib759477c02d97503634732f14da9fc4b7cb60de8
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6070382
Reviewed-by: Jack Franklin <[email protected]>
Commit-Queue: Nancy Li <[email protected]>

Author: Nancy Li

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

@@ -17,9 +17,10 @@ import * as Coordinator from '../../../ui/components/render_coordinator/render_c
 
 import * as TimelineComponents from './components.js';
 
+const coordinator = Coordinator.RenderCoordinator.RenderCoordinator.instance();
+
 describeWithEnvironment('Ignore List Setting', () => {
   async function renderIgnoreListSetting(): Promise<HTMLElement> {
-    const coordinator = Coordinator.RenderCoordinator.RenderCoordinator.instance();
     const component = new TimelineComponents.IgnoreListSetting.IgnoreListSetting();
     renderElementIntoDOM(component);
     await coordinator.done();
@@ -48,6 +49,14 @@ describeWithEnvironment('Ignore List Setting', () => {
     return newRegexInput;
   }
 
+  function getValidationResultElement(component: HTMLElement): HTMLDivElement {
+    assert.isNotNull(component.shadowRoot);
+    const validationResultElement = component.shadowRoot.querySelector<HTMLDivElement>('.input-validation');
+
+    assert.exists(validationResultElement);
+    return validationResultElement;
+  }
+
   before(() => {
     const targetManager = SDK.TargetManager.TargetManager.instance();
     const workspace = Workspace.Workspace.WorkspaceImpl.instance({forceNew: true});
@@ -169,6 +178,74 @@ describeWithEnvironment('Ignore List Setting', () => {
     assert.isFalse(isIgnoreRegexDisabled('rule 1'));
   });
 
+  it('Do not show warning message for valid rule', async () => {
+    const component = await renderIgnoreListSetting();
+    const newRegexInput = getNewRegexInput(component);
+
+    dispatchFocusEvent(newRegexInput);
+    newRegexInput.value = 'rule 2';
+    dispatchInputEvent(newRegexInput);
+    await coordinator.done();
+
+    const validationResultElement = component.shadowRoot?.querySelector<HTMLDivElement>('.input-validation');
+    assert.notExists(validationResultElement);
+
+    // We need this to simulate the 'finish editing' with empty input, so it can remove the temp regex. Otherwise the
+    // future tests will be messed up.
+    // The 'finish editing' part will be tested later
+    newRegexInput.value = '';
+    dispatchBlurEvent(newRegexInput);
+  });
+
+  it('Show error message for invalid rule', async () => {
+    // One example of invalid rule is duplicate input.
+    assert.isTrue(isRegexInIgnoredList('rule 1'));
+
+    const component = await renderIgnoreListSetting();
+    const newRegexInput = getNewRegexInput(component);
+
+    dispatchFocusEvent(newRegexInput);
+    newRegexInput.value = 'rule 1';
+    dispatchInputEvent(newRegexInput);
+    await coordinator.done();
+
+    const validationResultElement = getValidationResultElement(component);
+    assert.isFalse(validationResultElement.hidden);
+    assert.isTrue(validationResultElement.classList.contains('input-validation-error'));
+    assert.isNotEmpty(validationResultElement.textContent);
+
+    // We need this to simulate the 'finish editing' with empty input, so it can remove the temp regex. Otherwise the
+    // future tests will be messed up.
+    // The 'finish editing' part will be tested later
+    newRegexInput.value = '';
+    dispatchBlurEvent(newRegexInput);
+  });
+
+  it('Show warning message for valid rule with warning message', async () => {
+    // One example of valid rule with warning message is when a rule is disabled and it is added again.
+    disableIgnoreRegex('rule 1');
+    assert.isTrue(isIgnoreRegexDisabled('rule 1'));
+
+    const component = await renderIgnoreListSetting();
+    const newRegexInput = getNewRegexInput(component);
+
+    dispatchFocusEvent(newRegexInput);
+    newRegexInput.value = 'rule 1';
+    dispatchInputEvent(newRegexInput);
+    await coordinator.done();
+
+    const validationResultElement = getValidationResultElement(component);
+    assert.isFalse(validationResultElement.hidden);
+    assert.isFalse(validationResultElement.classList.contains('input-validation-error'));
+    assert.isNotEmpty(validationResultElement.textContent);
+
+    // We need this to simulate the 'finish editing' with empty input, so it can remove the temp regex. Otherwise the
+    // future tests will be messed up.
+    // The 'finish editing' part will be tested later
+    newRegexInput.value = '';
+    dispatchBlurEvent(newRegexInput);
+  });
+
   describe('preview the result', () => {
     it('Add an empty regex when focusing on the input', async () => {
       const regexPatterns = getIgnoredRegexes();
@@ -261,7 +338,7 @@ describeWithEnvironment('Pattern validator', () => {
     const emptyPattern = '';
     const result = TimelineComponents.IgnoreListSetting.patternValidator([], emptyPattern);
     assert.isFalse(result.valid);
-    assert.strictEqual(result.errorMessage, 'Rule can\'t be empty');
+    assert.strictEqual(result.message, 'Rule can\'t be empty');
   });
 
   it('Returns the reason for the existed pattern', () => {
@@ -270,7 +347,7 @@ describeWithEnvironment('Pattern validator', () => {
 
     const result = TimelineComponents.IgnoreListSetting.patternValidator([existedRegex], duplicatePattern);
     assert.isFalse(result.valid);
-    assert.strictEqual(result.errorMessage, 'Rule already exists');
+    assert.strictEqual(result.message, 'Rule already exists');
   });
 
   it('Returns true for the disabled existed pattern', () => {
@@ -279,13 +356,15 @@ describeWithEnvironment('Pattern validator', () => {
 
     const result = TimelineComponents.IgnoreListSetting.patternValidator([existedRegex], duplicatePattern);
     assert.isTrue(result.valid);
+    assert.strictEqual(
+        result.message, 'This rule already exists but is disabled. Saving this value will re-enable the rule');
   });
 
   it('Returns the reason for the invalid pattern', () => {
     const invalidPattern = '[';
     const result = TimelineComponents.IgnoreListSetting.patternValidator([], invalidPattern);
     assert.isFalse(result.valid);
-    assert.strictEqual(result.errorMessage, 'Rule must be a valid regular expression');
+    assert.strictEqual(result.message, 'Rule must be a valid regular expression');
   });
 });
 

front_end/panels/timeline/components/IgnoreListSetting.ts

@@ -62,6 +62,11 @@ const UIStrings = {
    *@description Error message in Framework Ignore List settings pane that declares pattern must be a valid regular expression
    */
   patternMustBeAValidRegular: 'Rule must be a valid regular expression',
+  /**
+   *@description Error message in Framework Ignore List settings pane that declares pattern already exits
+   */
+  patternAlreadyExistsWillBeEnables:
+      'This rule already exists but is disabled. Saving this value will re-enable the rule',
 };
 
 const str_ = i18n.i18n.registerUIStrings('panels/timeline/components/IgnoreListSetting.ts', UIStrings);
@@ -74,11 +79,13 @@ export class IgnoreListSetting extends HTMLElement {
       Common.Settings.Settings.instance().moduleSetting('enable-ignore-listing');
   readonly #regexPatterns = this.#getSkipStackFramesPatternSetting().getAsArray();
 
-  #newItemCheckbox = UI.UIUtils.CheckboxLabel.create(
+  #newRegexCheckbox = UI.UIUtils.CheckboxLabel.create(
       /* title*/ undefined, /* checked*/ false, /* subtitle*/ undefined,
       /* jslogContext*/ 'timeline.ignore-list-new-regex.checkbox');
-  #newItemInput = UI.UIUtils.createInput(
+  #newRegexInput = UI.UIUtils.createInput(
       /* className*/ 'new-regex-text-input', /* type*/ 'text', /* jslogContext*/ 'timeline.ignore-list-new-regex.text');
+  #newRegexIsValid = true;
+  #newRegexValidationMessage?: string;
 
   #editingRegexSetting: Common.Settings.RegExpSettingItem|null = null;
 
@@ -110,7 +117,7 @@ export class IgnoreListSetting extends HTMLElement {
   }
 
   #startEditing(): void {
-    this.#editingRegexSetting = {pattern: this.#newItemInput.value, disabled: false, disabledForUrl: undefined};
+    this.#editingRegexSetting = {pattern: this.#newRegexInput.value, disabled: false, disabledForUrl: undefined};
     // We need to push the temp regex here to update the flame chart.
     // We are using the "skip-stack-frames-pattern" setting to determine which is rendered on flame chart. And the push
     // here will update the setting's value.
@@ -135,12 +142,12 @@ export class IgnoreListSetting extends HTMLElement {
   }
 
   #resetInput(): void {
-    this.#newItemCheckbox.checkboxElement.checked = false;
-    this.#newItemInput.value = '';
+    this.#newRegexCheckbox.checkboxElement.checked = false;
+    this.#newRegexInput.value = '';
   }
 
   #onNewRegexAdded(): void {
-    const newRegex = this.#newItemInput.value.trim();
+    const newRegex = this.#newRegexInput.value.trim();
 
     this.#finishEditing();
     const {valid} = patternValidator(this.#getExistingRegexes(), newRegex);
@@ -179,25 +186,45 @@ export class IgnoreListSetting extends HTMLElement {
   }
 
   #handleInputChange(): void {
+    const newRegex = this.#newRegexInput.value.trim();
     // Enable the rule if the text input field is not empty.
-    this.#newItemCheckbox.checkboxElement.checked = Boolean(this.#newItemInput.value.trim());
+    this.#newRegexCheckbox.checkboxElement.checked = Boolean(newRegex);
+    const {valid, message} = patternValidator(this.#getExistingRegexes(), newRegex);
+
+    this.#newRegexInput.classList.toggle('error-input', !valid);
+    UI.ARIAUtils.setInvalid(this.#newRegexInput, !valid);
+    this.#newRegexIsValid = valid;
+    this.#newRegexValidationMessage = message;
 
     if (this.#editingRegexSetting) {
-      this.#editingRegexSetting.pattern = this.#newItemInput.value.trim();
+      this.#editingRegexSetting.pattern = this.#newRegexInput.value.trim();
       this.#getSkipStackFramesPatternSetting().setAsArray(this.#regexPatterns);
     }
   }
 
   #initAddNewItem(): void {
     const checkboxHelpText = i18nString(UIStrings.ignoreScriptsWhoseNamesMatchNewRegex);
     const inputHelpText = i18nString(UIStrings.addNewRegex);
-    UI.Tooltip.Tooltip.install(this.#newItemCheckbox, checkboxHelpText);
-    UI.Tooltip.Tooltip.install(this.#newItemInput, inputHelpText);
+    UI.Tooltip.Tooltip.install(this.#newRegexCheckbox, checkboxHelpText);
+    UI.Tooltip.Tooltip.install(this.#newRegexInput, inputHelpText);
 
-    this.#newItemInput.addEventListener('blur', this.#onNewRegexAdded.bind(this), false);
-    this.#newItemInput.addEventListener('keydown', this.#handleKeyDown.bind(this), false);
-    this.#newItemInput.addEventListener('input', this.#handleInputChange.bind(this), false);
-    this.#newItemInput.addEventListener('focus', this.#startEditing.bind(this), false);
+    this.#newRegexInput.addEventListener('blur', this.#onNewRegexAdded.bind(this), false);
+    this.#newRegexInput.addEventListener('keydown', this.#handleKeyDown.bind(this), false);
+    this.#newRegexInput.addEventListener('input', this.#handleInputChange.bind(this), false);
+    this.#newRegexInput.addEventListener('focus', this.#startEditing.bind(this), false);
+  }
+
+  #renderNewRegexRow(): LitHtml.TemplateResult {
+    const classes = LitHtml.Directives.classMap({
+      'input-validation': true,
+      'input-validation-error': !this.#newRegexIsValid,
+    });
+    return html`
+      <div class='new-regex-row'>${this.#newRegexCheckbox}${this.#newRegexInput}</div>
+      ${
+        this.#newRegexValidationMessage ? html`<div class=${classes}>${this.#newRegexValidationMessage}</div>` :
+                                          LitHtml.nothing}
+    `;
   }
 
   #onRegexEnableToggled(regex: Common.Settings.RegExpSettingItem, checkbox: UI.UIUtils.CheckboxLabel): void {
@@ -262,7 +289,7 @@ export class IgnoreListSetting extends HTMLElement {
         <div class='ignore-list-setting-content'>
           <div class='ignore-list-setting-description'>${i18nString(UIStrings.ignoreListDescription)}</div>
           ${this.#getExistingRegexes().map(this.#renderItem.bind(this))}
-          <div class='new-regex-row'>${this.#newItemCheckbox}${this.#newItemInput}</div>
+          ${this.#renderNewRegexRow()}
         </div>
       </devtools-button-dialog>
     `;
@@ -287,21 +314,23 @@ declare global {
  * @param inputValue new regex in string format
  * @returns if the regex is valid and if not, why it is invalid
  */
-export function patternValidator(
-    existingRegexes: Common.Settings.RegExpSettingItem[], inputValue: string): ({valid: true}|{
-  valid: false,
-  errorMessage: string,
-}) {
+export function patternValidator(existingRegexes: Common.Settings.RegExpSettingItem[], inputValue: string): {
+  valid: boolean,
+  message?: string,
+} {
   const pattern = inputValue.trim();
 
   if (!pattern.length) {
-    return {valid: false, errorMessage: i18nString(UIStrings.patternCannotBeEmpty)};
+    return {valid: false, message: i18nString(UIStrings.patternCannotBeEmpty)};
   }
 
   for (let i = 0; i < existingRegexes.length; ++i) {
     const regexPattern = existingRegexes[i];
-    if (regexPattern.pattern === pattern && !regexPattern.disabled && regexPattern.disabledForUrl === undefined) {
-      return {valid: false, errorMessage: i18nString(UIStrings.patternAlreadyExists)};
+    if (regexPattern.pattern === pattern) {
+      if (regexPattern.disabled || regexPattern.disabledForUrl) {
+        return {valid: true, message: i18nString(UIStrings.patternAlreadyExistsWillBeEnables)};
+      }
+      return {valid: false, message: i18nString(UIStrings.patternAlreadyExists)};
     }
   }
 
@@ -311,7 +340,7 @@ export function patternValidator(
   } catch (e) {
   }
   if (!regex) {
-    return {valid: false, errorMessage: i18nString(UIStrings.patternMustBeAValidRegular)};
+    return {valid: false, message: i18nString(UIStrings.patternMustBeAValidRegular)};
   }
   return {valid: true};
 }

front_end/panels/timeline/components/ignoreListSetting.css

@@ -30,4 +30,34 @@
   .new-regex-text-input {
     flex: auto;
   }
+
+  .harmony-input[type="text"] {
+    /* padding: 3px 6px; */
+    /* height: 24px; */
+    border: 1px solid var(--sys-color-neutral-outline);
+    border-radius: 4px;
+    outline: none;
+
+    &.error-input,
+    &:invalid {
+      border-color: var(--sys-color-error);
+    }
+
+    &:not(.error-input):not(:invalid):focus {
+      border-color: var(--sys-color-state-focus-ring);
+    }
+
+    &:not(.error-input):not(:invalid):hover:not(:focus) {
+      background: var(--sys-color-state-hover-on-subtle);
+    }
+  }
+}
+
+.input-validation {
+  /* 24px is the size of the checkbox, add this margin-left so the text can align with the input */
+  margin: 5px 0 5px 24px;
+
+  &.input-validation-error {
+    color: var(--sys-color-error);
+  }
 }