Toolbar: fix title not updating on combo box

This CL fixes an issue where the title attribute does not update
properly as the user changes which value is selected.

Fixed: 407954458
Change-Id: Ic11f65b47994b6090231a38c93cb7d1e52d3deba
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6430024
Auto-Submit: Jack Franklin <[email protected]>
Reviewed-by: Andres Olivares <[email protected]>
Commit-Queue: Andres Olivares <[email protected]>

Author: jackfranklin

front_end/ui/legacy/Toolbar.test.ts

@@ -2,8 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import * as Common from '../../core/common/common.js';
 import {dispatchClickEvent, doubleRaf, renderElementIntoDOM} from '../../testing/DOMHelpers.js';
-import {describeWithLocale} from '../../testing/EnvironmentHelpers.js';
+import {describeWithEnvironment, describeWithLocale} from '../../testing/EnvironmentHelpers.js';
 import * as RenderCoordinator from '../components/render_coordinator/render_coordinator.js';
 
 import * as UI from './legacy.js';
@@ -209,4 +210,22 @@ describeWithLocale('Toolbar', () => {
       assert.isFalse(contextHandler.called);
     });
   });
+
+  describeWithEnvironment('ToolbarSettingComboBox', () => {
+    it('updates its title with the currently active setting', async () => {
+      const setting = Common.Settings.Settings.instance().createSetting<string>('test-combo-box-setting', 'option-1');
+      setting.set('option-1');
+      const box = new UI.Toolbar.ToolbarSettingComboBox(
+          [{value: 'option-1', label: 'Option 1'}, {value: 'option-2', label: 'Option 2'}], setting, 'title-value');
+      assert.strictEqual(box.element.title, 'Option 1');
+      const options = box.options();
+      // Ensure it works with select()
+      box.select(options[1]);
+      assert.strictEqual(box.element.title, 'Option 2');
+
+      // Ensure it works with setSelectedIndex()
+      box.setSelectedIndex(0);
+      assert.strictEqual(box.element.title, 'Option 1');
+    });
+  });
 });

front_end/ui/legacy/Toolbar.ts

@@ -1243,9 +1243,9 @@ export class ToolbarSettingComboBox extends ToolbarComboBox {
     super(null, accessibleName, undefined, setting.name);
     this.optionsInternal = options;
     this.setting = setting;
-    this.element.addEventListener('change', this.valueChanged.bind(this), false);
+    this.element.addEventListener('change', this.onSelectValueChange.bind(this), false);
     this.setOptions(options);
-    setting.addChangeListener(this.settingChanged, this);
+    setting.addChangeListener(this.onDevToolsSettingChanged, this);
   }
 
   setOptions(options: Option[]): void {
@@ -1265,7 +1265,29 @@ export class ToolbarSettingComboBox extends ToolbarComboBox {
     return this.optionsInternal[this.selectedIndex()].value;
   }
 
-  private settingChanged(): void {
+  override select(option: Element): void {
+    const index = Array.prototype.indexOf.call(this.element, option);
+    this.setSelectedIndex(index);
+  }
+
+  override setSelectedIndex(index: number): void {
+    super.setSelectedIndex(index);
+    const option = this.optionsInternal.at(index);
+    if (option) {
+      this.setTitle(option.label);
+    }
+  }
+
+  /**
+   * Note: wondering why there are two event listeners and what the difference is?
+   * It is because this combo box <select> is backed by a Devtools setting and
+   * at any time there could be multiple instances of these elements that are
+   * backed by the same setting. So they have to listen to two things:
+   * 1. When the setting is changed via a different method.
+   * 2. When the value of the select is changed, triggering a change to the setting.
+   */
+
+  private onDevToolsSettingChanged(): void {
     if (this.muteSettingListener) {
       return;
     }
@@ -1279,7 +1301,10 @@ export class ToolbarSettingComboBox extends ToolbarComboBox {
     }
   }
 
-  private valueChanged(_event: Event): void {
+  /**
+   * Run when the user interacts with the <select> element.
+   */
+  private onSelectValueChange(_event: Event): void {
     const option = this.optionsInternal[this.selectedIndex()];
     this.muteSettingListener = true;
     this.setting.set(option.value);

front_end/ui/visual_logging/KnownContextValues.ts

@@ -3433,6 +3433,7 @@ export const knownContextValues = new Set([
   'terms-of-service.console-insights',
   'test',
   'test-action',
+  'test-combo-box-setting',
   'test-device',
   'test-font',
   'test-setting',

test/e2e/performance/timeline/treeView_test.ts

@@ -153,7 +153,8 @@ describe('The Performance tool, Bottom-up panel', function() {
       if (!rootActivity) {
         assert.fail(`Could not find ${expectedActivities[0]} in frontend.`);
       }
-      const dropdown = await waitFor('select[aria-label="Group by"]');
+
+      const dropdown = await waitFor('select[aria-label="No grouping"]');
       await dropdown.evaluate(el => {
         (el as HTMLSelectElement).selectedIndex = 2;
         el.dispatchEvent(new Event('change'));