[throttling] Update rule list items more cleanly to preserve focus

pfaffe · Devtools-frontend LUCI CQ · commit 3acf7240456c · 2025-11-21T05:36:06.000-08:00
This requires teaching ListWidget to update items in-place. Fixed: 461577228 Change-Id: Ie3aa514779c7d2844691d1f2afb635f0994c498d Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7185563 Auto-Submit: Philip Pfaffe <pfaffe@chromium.org> Reviewed-by: Eric Leese <leese@chromium.org> Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
diff --git a/front_end/panels/network/RequestConditionsDrawer.ts b/front_end/panels/network/RequestConditionsDrawer.ts
@@ -401,6 +401,12 @@ export class RequestConditionsDrawer extends UI.Widget.VBox implements
     const element = document.createElement('div');
     this.#listElements.set(condition, element);
     element.classList.add('blocked-url');
+    this.updateItem(element, condition, editable, index);
+    return element;
+  }
+
+  updateItem(element: HTMLElement, condition: SDK.NetworkManager.RequestCondition, editable: boolean, index: number):
+      void {
     const toggle = (e: Event): void => {
       if (editable) {
         e.consume(true);
@@ -437,9 +443,9 @@ export class RequestConditionsDrawer extends UI.Widget.VBox implements
       @click=${toggle}
       type=checkbox
       title=${i18nString(UIStrings.enableThrottlingToggleLabel, {PH1: constructorStringOrWildcardURL})}
-      ?checked=${enabled}
-      ?disabled=${!editable || !originalOrUpgradedURLPattern}
-      .jslog=${VisualLogging.toggle().track({ change: true })}>
+      .checked=${enabled}
+      .disabled=${!editable || !originalOrUpgradedURLPattern}
+      jslog=${VisualLogging.toggle().track({ change: true })}>
     <devtools-button
       .iconName=${'arrow-up'}
       .variant=${Buttons.Button.Variant.ICON}
@@ -516,15 +522,14 @@ export class RequestConditionsDrawer extends UI.Widget.VBox implements
     <input class=blocked-url-checkbox
       @click=${toggle}
       type=checkbox
-      ?checked=${condition.enabled}
-      ?disabled=${!editable}
-      .jslog=${VisualLogging.toggle().track({ change: true })}>
+      .checked=${condition.enabled}
+      .disabled=${!editable}
+      jslog=${VisualLogging.toggle().track({ change: true })}>
     <div @click=${toggle} class=blocked-url-label>${wildcardURL}</div>
     <devtools-widget .widgetConfig=${widgetConfig(AffectedCountWidget, {condition, drawer: this})}></devtools-widget>`,
           // clang-format on
           element);
     }
-    return element;
   }
 
   private toggleEnabled(): void {
@@ -609,19 +614,26 @@ export class RequestConditionsDrawer extends UI.Widget.VBox implements
 
   update(): void {
     const enabled = this.manager.requestConditions.conditionsEnabled;
-    this.list.clear();
-    for (const pattern of this.manager.requestConditions.conditions) {
-      if (Root.Runtime.hostConfig.devToolsIndividualRequestThrottling?.enabled || pattern.wildcardURL) {
-        this.list.appendItem(
-            pattern,
-            enabled,
-            /* focusable=*/ false,
-            {
-              edit: i18nString(UIStrings.editPattern, {PH1: pattern.constructorStringOrWildcardURL}),
-              delete: i18nString(UIStrings.removePattern, {PH1: pattern.constructorStringOrWildcardURL})
-            },
-        );
-      }
+    const newItems = Array.from(this.manager.requestConditions.conditions.filter(
+        pattern => Root.Runtime.hostConfig.devToolsIndividualRequestThrottling?.enabled || pattern.wildcardURL));
+
+    let oldIndex = 0;
+    for (; oldIndex < newItems.length; ++oldIndex) {
+      const pattern = newItems[oldIndex];
+      this.list.updateItem(
+          oldIndex,
+          pattern,
+          enabled,
+          /* focusable=*/ false,
+          {
+            edit: i18nString(UIStrings.editPattern, {PH1: pattern.constructorStringOrWildcardURL}),
+            delete: i18nString(UIStrings.removePattern, {PH1: pattern.constructorStringOrWildcardURL})
+          },
+      );
+    }
+
+    while (oldIndex < this.list.items.length) {
+      this.list.removeItem(oldIndex);
     }
     this.requestUpdate();
   }
diff --git a/front_end/ui/legacy/ListWidget.test.ts b/front_end/ui/legacy/ListWidget.test.ts
@@ -75,4 +75,110 @@ describeWithLocale('ListWidget', () => {
     list.addNewItem(0, '');
     assert.isTrue(delegate.committed);
   });
+
+  class MockDelegate implements UI.ListWidget.Delegate<string> {
+    updateItem(content: Element, item: string): void {
+      content.textContent = item;
+    }
+
+    renderItem(item: string): Element {
+      const element = document.createElement('div');
+      element.textContent = item;
+      return element;
+    }
+
+    removeItemRequested(): void {
+    }
+
+    beginEdit(): UI.ListWidget.Editor<string> {
+      return new UI.ListWidget.Editor<string>();
+    }
+
+    commitEdit(): void {
+    }
+  }
+
+  describe('updates', () => {
+    it('an item at the beginning', () => {
+      const listWidget = new UI.ListWidget.ListWidget(new MockDelegate());
+
+      listWidget.appendItem('a', true);
+      listWidget.appendItem('b', true);
+      listWidget.appendItem('c', true);
+
+      listWidget.updateItem(0, 'd', true);
+
+      const items = Array.from(listWidget.element.shadowRoot!.querySelectorAll('.list-item')).map(i => i.textContent);
+      assert.deepEqual(items, ['d', 'b', 'c']);
+    });
+
+    it('an item in the middle', () => {
+      const listWidget = new UI.ListWidget.ListWidget(new MockDelegate());
+
+      listWidget.appendItem('a', true);
+      listWidget.appendItem('b', true);
+      listWidget.appendItem('c', true);
+
+      listWidget.updateItem(1, 'd', true);
+
+      const items = Array.from(listWidget.element.shadowRoot!.querySelectorAll('.list-item')).map(i => i.textContent);
+      assert.deepEqual(items, ['a', 'd', 'c']);
+    });
+
+    it('an item at the end', () => {
+      const listWidget = new UI.ListWidget.ListWidget(new MockDelegate());
+
+      listWidget.appendItem('a', true);
+      listWidget.appendItem('b', true);
+      listWidget.appendItem('c', true);
+
+      listWidget.updateItem(2, 'd', true);
+
+      const items = Array.from(listWidget.element.shadowRoot!.querySelectorAll('.list-item')).map(i => i.textContent);
+      assert.deepEqual(items, ['a', 'b', 'd']);
+    });
+
+    it('an item with an index < 0', () => {
+      const listWidget = new UI.ListWidget.ListWidget(new MockDelegate());
+
+      listWidget.appendItem('a', true);
+      listWidget.appendItem('b', true);
+      listWidget.appendItem('c', true);
+
+      listWidget.updateItem(-1, 'd', true);
+
+      const items = Array.from(listWidget.element.shadowRoot!.querySelectorAll('.list-item')).map(i => i.textContent);
+      assert.deepEqual(items, ['a', 'b', 'c', 'd']);
+    });
+
+    it('an item with an index > list length', () => {
+      const listWidget = new UI.ListWidget.ListWidget(new MockDelegate());
+
+      listWidget.appendItem('a', true);
+      listWidget.appendItem('b', true);
+      listWidget.appendItem('c', true);
+
+      listWidget.updateItem(10, 'd', true);
+
+      const items = Array.from(listWidget.element.shadowRoot!.querySelectorAll('.list-item')).map(i => i.textContent);
+      assert.deepEqual(items, ['a', 'b', 'c', 'd']);
+
+      listWidget.element.remove();
+    });
+
+    it('reorders items', () => {
+      const listWidget = new UI.ListWidget.ListWidget(new MockDelegate());
+
+      listWidget.appendItem('a', true);
+      listWidget.appendItem('b', true);
+      listWidget.appendItem('c', true);
+
+      listWidget.updateItem(2, 'a', true);
+      listWidget.updateItem(1, 'c', true);
+      listWidget.updateItem(0, 'b', true);
+
+      const items = Array.from(listWidget.element.shadowRoot!.querySelectorAll('.list-item')).map(i => i.textContent);
+      assert.deepEqual(items, ['b', 'c', 'a']);
+    });
+  });
 });
diff --git a/front_end/ui/legacy/ListWidget.ts b/front_end/ui/legacy/ListWidget.ts
@@ -56,12 +56,12 @@ export class ListWidget<T> extends VBox {
   private readonly list: HTMLElement;
   private lastSeparator: boolean;
   private focusRestorer: ElementFocusRestorer|null;
-  private items: T[];
+  #items: T[];
   private editable: boolean[];
-  private elements: Element[];
+  private elements: HTMLElement[];
   private editor: Editor<T>|null;
   private editItem: T|null;
-  private editElement: Element|null;
+  private editElement: HTMLElement|null;
   private emptyPlaceholder: Element|null;
   private isTable: boolean;
   constructor(delegate: Delegate<T>, delegatesFocus = true, isTable = false) {
@@ -73,7 +73,7 @@ export class ListWidget<T> extends VBox {
 
     this.lastSeparator = false;
     this.focusRestorer = null;
-    this.items = [];
+    this.#items = [];
     this.editable = [];
     this.elements = [];
     this.editor = null;
@@ -90,8 +90,12 @@ export class ListWidget<T> extends VBox {
     this.updatePlaceholder();
   }
 
+  get items(): T[] {
+    return this.#items;
+  }
+
   clear(): void {
-    this.items = [];
+    this.#items = [];
     this.editable = [];
     this.elements = [];
     this.lastSeparator = false;
@@ -100,8 +104,34 @@ export class ListWidget<T> extends VBox {
     this.stopEditing();
   }
 
+  updateItem(index: number, newItem: T, editable: boolean, focusable = true, controlLabels: {
+    edit?: string,
+    delete?: string,
+  } = {}): void {
+    if (index < 0 || index >= this.#items.length) {
+      this.appendItem(newItem, editable, focusable, controlLabels);
+      return;
+    }
+
+    this.#items[index] = newItem;
+    this.editable[index] = editable;
+    const element = this.elements[index];
+    const [content, controls] = element.children;
+    if (controls) {
+      element.removeChild(controls);
+    }
+    this.delegate.updateItem?.(content, newItem, editable, index);
+    element.classList.toggle('editable', editable);
+    if (editable) {
+      if (focusable) {
+        element.tabIndex = 0;
+      }
+      element.appendChild(this.createControls(newItem, element, controlLabels));
+    }
+  }
+
   appendItem(item: T, editable: boolean, focusable = true, controlLabels: {edit?: string, delete?: string} = {}): void {
-    if (this.lastSeparator && this.items.length) {
+    if (this.lastSeparator && this.#items.length) {
       const element = document.createElement('div');
       element.classList.add('list-separator');
       if (this.isTable) {
@@ -111,14 +141,14 @@ export class ListWidget<T> extends VBox {
     }
     this.lastSeparator = false;
 
-    this.items.push(item);
+    this.#items.push(item);
     this.editable.push(editable);
 
     const element = this.list.createChild('div', 'list-item');
     if (this.isTable) {
       element.role = 'rowgroup';
     }
-    const content = this.delegate.renderItem(item, editable, this.items.length - 1);
+    const content = this.delegate.renderItem(item, editable, this.#items.length - 1);
     if (!content.hasAttribute('jslog')) {
       element.setAttribute('jslog', `${VisualLogging.item()}`);
     }
@@ -139,7 +169,7 @@ export class ListWidget<T> extends VBox {
   }
 
   removeItem(index: number): void {
-    if (this.editItem === this.items[index]) {
+    if (this.editItem === this.#items[index]) {
       this.stopEditing();
     }
 
@@ -160,7 +190,7 @@ export class ListWidget<T> extends VBox {
     element.remove();
 
     this.elements.splice(index, 1);
-    this.items.splice(index, 1);
+    this.#items.splice(index, 1);
     this.editable.splice(index, 1);
     this.updatePlaceholder();
   }
@@ -174,7 +204,7 @@ export class ListWidget<T> extends VBox {
     this.updatePlaceholder();
   }
 
-  private createControls(item: T, element: Element, controlLabels: {edit?: string, delete?: string}): Element {
+  private createControls(item: T, element: HTMLElement, controlLabels: {edit?: string, delete?: string}): Element {
     const controls = document.createElement('div');
     controls.classList.add('controls-container');
     controls.classList.add('fill');
@@ -211,7 +241,7 @@ export class ListWidget<T> extends VBox {
     function onRemoveClicked(this: ListWidget<T>): void {
       const index = this.elements.indexOf(element);
       this.element.focus();
-      this.delegate.removeItemRequested(this.items[index], index);
+      this.delegate.removeItemRequested(this.#items[index], index);
       ARIAUtils.LiveAnnouncer.alert(i18nString(UIStrings.removedItem));
       if (this.elements.length >= 1) {
         // focus on the next item in the list, or the last item if we're removing the last item
@@ -237,7 +267,7 @@ export class ListWidget<T> extends VBox {
     }
   }
 
-  private startEditing(item: T, element: Element|null, insertionPoint: Element|null): void {
+  private startEditing(item: T, element: HTMLElement|null, insertionPoint: Element|null): void {
     if (element && this.editElement === element) {
       return;
     }
@@ -299,6 +329,7 @@ export class ListWidget<T> extends VBox {
 }
 
 export interface Delegate<T> {
+  updateItem?(content: Element, newItem: T, editable: boolean, index: number): void;
   renderItem(item: T, editable: boolean, index: number): Element;
   removeItemRequested(item: T, index: number): void;
   beginEdit(item: T): Editor<T>;
