[keyboard-alt]: Group keys under Modifier/Other sections

Screenshot: http://screen/6rhpREWBMiNWozi

Bug: b/279503977
Change-Id: I24c0960632733a36e59ac1b0d5ee6100d9735505
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4583639
Reviewed-by: David Padlipsky <dpad@google.com>
Commit-Queue: Michael Checo <michaelcheco@google.com>
Cr-Commit-Position: refs/heads/main@{#1159201}

chrome/app/os_settings_strings.grdp

@@ -3018,6 +3018,13 @@ Press an assigned switch or key to remove assignment.
   <message name="IDS_SETTINGS_TOUCHPAD_SIMULATE_RIGHT_CLICK_OPTION_ALT" translateable="false" desc="In the touchpad settings subpage, the dropdown list item for the Alt + Click option.">
     alt + click
   </message>
+  <!-- TODO(b/286937607): Finalize and translate these strings -->
+  <message name="IDS_SETTINGS_MODIFIER_KEYS_LABEL" translateable="false" desc="In the remap keyboard keys settings subpage, the label for the section that contains all of the modifier keys (Ctrl, Alt, etc).">
+    Modifier keys
+  </message>
+  <message name="IDS_SETTINGS_OTHER_KEYS_LABEL" translateable="false" desc="In the remap keyboard keys settings subpage, the label for the section that contains all of the non-modifier keys (Backspace, Home, etc).">
+    Other keys
+  </message>
 
   <!-- Printing Page (OS Settings)-->
   <message name="IDS_SETTINGS_PRINT_AND_SCAN" desc="Title of the printing and scanning page and navigation item to get there.">

chrome/browser/resources/settings/chromeos/device_page/per_device_keyboard_remap_keys.html

@@ -14,34 +14,104 @@
     color: var(--cr-secondary-text-color);
     margin-inline-start: 20px;
   }
+
+  .subsection-header {
+    padding-inline-start: 0;
+  }
 </style>
 <div id="header">
   <div id="description">[[computeKeyboardKeysDescription(keyboard.*)]]</div>
 </div>
 <div class="subsection">
-  <keyboard-remap-modifier-key-row
+  <template is="dom-if" if="[[!isAltClickAndSixPackCustomizationEnabled]]">
+    <keyboard-remap-modifier-key-row
+        default-remappings="[[defaultRemappings]]"
+        key="[[modifierKey.kMeta]]" id="metaKey"
+        meta-key="[[keyboard.metaKey]]"
+        aria-label="$i18n{perDeviceKeyboardKeySearch}"
+        pref="{{fakeMetaPref}}"
+        remove-top-border>
+    </keyboard-remap-modifier-key-row>
+    <keyboard-remap-modifier-key-row
+        default-remappings="[[defaultRemappings]]"
+        key="[[modifierKey.kControl]]" id="ctrlKey"
+        meta-key="[[keyboard.metaKey]]"
+        aria-label="$i18n{perDeviceKeyboardKeyCtrl}"
+        pref="{{fakeCtrlPref}}">
+    </keyboard-remap-modifier-key-row>
+    <keyboard-remap-modifier-key-row
+        default-remappings="[[defaultRemappings]]"
+        key="[[modifierKey.kAlt]]" id="altKey"
+        meta-key="[[keyboard.metaKey]]"
+        aria-label="$i18n{perDeviceKeyboardKeyAlt}"
+        pref="{{fakeAltPref}}">
+    </keyboard-remap-modifier-key-row>
+    <keyboard-remap-modifier-key-row
+        default-remappings="[[defaultRemappings]]"
+        key="[[modifierKey.kEscape]]" id="escapeKey"
+        meta-key="[[keyboard.metaKey]]"
+        aria-label="$i18n{perDeviceKeyboardKeyEscape}"
+        pref="{{fakeEscPref}}">
+    </keyboard-remap-modifier-key-row>
+    <keyboard-remap-modifier-key-row
+        default-remappings="[[defaultRemappings]]"
+        key="[[modifierKey.kBackspace]]" id="backspaceKey"
+        meta-key="[[keyboard.metaKey]]"
+        aria-label="$i18n{perDeviceKeyboardKeyBackspace}"
+        pref="{{fakeBackspacePref}}">
+    </keyboard-remap-modifier-key-row>
+    <template is="dom-if" if="[[hasAssistantKey]]" restamp>
+      <keyboard-remap-modifier-key-row
+          default-remappings="[[defaultRemappings]]"
+          key="[[modifierKey.kAssistant]]" id="assistantKey"
+          meta-key="[[keyboard.metaKey]]"
+          aria-label="$i18n{perDeviceKeyboardKeyAssistant}"
+          pref="{{fakeAssistantPref}}">
+      </keyboard-remap-modifier-key-row>
+    </template>
+    <template is="dom-if" if="[[hasCapsLockKey]]" restamp>
+      <keyboard-remap-modifier-key-row
+          default-remappings="[[defaultRemappings]]"
+          key="[[modifierKey.kCapsLock]]" id="capsLockKey"
+          meta-key="[[keyboard.metaKey]]"
+          aria-label="$i18n{perDeviceKeyboardKeyCapsLock}"
+          pref="{{fakeCapsLockPref}}">
+      </keyboard-remap-modifier-key-row>
+    </template>
+  </template>
+  <template is="dom-if" if="[[isAltClickAndSixPackCustomizationEnabled]]">
+    <h2 class="subsection-header" id="modifierKeysHeader">
+      $i18n{modifierKeysLabel}
+    </h2>
+    <div class="subsection">
+      <keyboard-remap-modifier-key-row
       default-remappings="[[defaultRemappings]]"
       key="[[modifierKey.kMeta]]" id="metaKey"
       meta-key="[[keyboard.metaKey]]"
       aria-label="$i18n{perDeviceKeyboardKeySearch}"
       pref="{{fakeMetaPref}}"
       remove-top-border>
   </keyboard-remap-modifier-key-row>
-  <keyboard-remap-modifier-key-row
-      default-remappings="[[defaultRemappings]]"
-      key="[[modifierKey.kControl]]" id="ctrlKey"
-      meta-key="[[keyboard.metaKey]]"
-      aria-label="$i18n{perDeviceKeyboardKeyCtrl}"
-      pref="{{fakeCtrlPref}}">
-  </keyboard-remap-modifier-key-row>
-  <keyboard-remap-modifier-key-row
-      default-remappings="[[defaultRemappings]]"
-      key="[[modifierKey.kAlt]]" id="altKey"
-      meta-key="[[keyboard.metaKey]]"
-      aria-label="$i18n{perDeviceKeyboardKeyAlt}"
-      pref="{{fakeAltPref}}">
-  </keyboard-remap-modifier-key-row>
-  <keyboard-remap-modifier-key-row
+    <keyboard-remap-modifier-key-row
+        default-remappings="[[defaultRemappings]]"
+        key="[[modifierKey.kControl]]" id="ctrlKey"
+        meta-key="[[keyboard.metaKey]]"
+        aria-label="$i18n{perDeviceKeyboardKeyCtrl}"
+        pref="{{fakeCtrlPref}}">
+    </keyboard-remap-modifier-key-row>
+    <keyboard-remap-modifier-key-row
+        default-remappings="[[defaultRemappings]]"
+        key="[[modifierKey.kAlt]]" id="altKey"
+        meta-key="[[keyboard.metaKey]]"
+        aria-label="$i18n{perDeviceKeyboardKeyAlt}"
+        pref="{{fakeAltPref}}">
+    </keyboard-remap-modifier-key-row>
+    </div>
+    <h2 class="subsection-header" id="otherKeysHeader">
+      $i18n{otherKeysLabel}
+    </h2>
+    <div class="subsection">
+      <keyboard-remap-modifier-key-row
       default-remappings="[[defaultRemappings]]"
       key="[[modifierKey.kEscape]]" id="escapeKey"
       meta-key="[[keyboard.metaKey]]"
@@ -57,20 +127,21 @@
   </keyboard-remap-modifier-key-row>
   <template is="dom-if" if="[[hasAssistantKey]]" restamp>
     <keyboard-remap-modifier-key-row
-      default-remappings="[[defaultRemappings]]"
-      key="[[modifierKey.kAssistant]]" id="assistantKey"
-      meta-key="[[keyboard.metaKey]]"
-      aria-label="$i18n{perDeviceKeyboardKeyAssistant}"
-      pref="{{fakeAssistantPref}}">
-  </keyboard-remap-modifier-key-row>
+        default-remappings="[[defaultRemappings]]"
+        key="[[modifierKey.kAssistant]]" id="assistantKey"
+        meta-key="[[keyboard.metaKey]]"
+        aria-label="$i18n{perDeviceKeyboardKeyAssistant}"
+        pref="{{fakeAssistantPref}}">
+    </keyboard-remap-modifier-key-row>
   </template>
   <template is="dom-if" if="[[hasCapsLockKey]]" restamp>
     <keyboard-remap-modifier-key-row
-      default-remappings="[[defaultRemappings]]"
-      key="[[modifierKey.kCapsLock]]" id="capsLockKey"
-      meta-key="[[keyboard.metaKey]]"
-      aria-label="$i18n{perDeviceKeyboardKeyCapsLock}"
-      pref="{{fakeCapsLockPref}}">
-  </keyboard-remap-modifier-key-row>
+        default-remappings="[[defaultRemappings]]"
+        key="[[modifierKey.kCapsLock]]" id="capsLockKey"
+        meta-key="[[keyboard.metaKey]]"
+        aria-label="$i18n{perDeviceKeyboardKeyCapsLock}"
+        pref="{{fakeCapsLockPref}}">
+    </keyboard-remap-modifier-key-row>
+    </div>
   </template>
-</div>
+</div>

chrome/browser/resources/settings/chromeos/device_page/per_device_keyboard_remap_keys.ts

@@ -17,6 +17,7 @@ import './keyboard_remap_modifier_key_row.js';
 
 import {I18nMixin, I18nMixinInterface} from 'chrome://resources/cr_elements/i18n_mixin.js';
 import {assert} from 'chrome://resources/js/assert_ts.js';
+import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
 import {PolymerElementProperties} from 'chrome://resources/polymer/v3_0/polymer/interfaces.js';
 import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
@@ -165,6 +166,15 @@ export class SettingsPerDeviceKeyboardRemapKeysElement extends
         type: Number,
         value: -1,
       },
+
+      isAltClickAndSixPackCustomizationEnabled: {
+        type: Boolean,
+        value() {
+          return loadTimeData.getBoolean(
+              'enableAltClickAndSixPackCustomization');
+        },
+        readOnly: true,
+      },
     };
   }
 
@@ -186,6 +196,7 @@ export class SettingsPerDeviceKeyboardRemapKeysElement extends
   }
 
   keyboard: Keyboard;
+  isAltClickAndSixPackCustomizationEnabled: boolean;
   private keyboards: Keyboard[];
   protected keyboardId: number;
   protected defaultRemappings: {[key: number]: ModifierKey} = {

chrome/browser/ui/webui/settings/ash/device_section.cc

@@ -1724,6 +1724,8 @@ void DeviceSection::AddDevicePointersStrings(
       {"touchpadSimulateRightClickOptionSearch",
        IDS_SETTINGS_TOUCHPAD_SIMULATE_RIGHT_CLICK_OPTION_SEARCH},
       {"learnMoreLabel", IDS_SETTINGS_LEARN_MORE_LABEL},
+      {"modifierKeysLabel", IDS_SETTINGS_MODIFIER_KEYS_LABEL},
+      {"otherKeysLabel", IDS_SETTINGS_OTHER_KEYS_LABEL},
   };
   html_source->AddLocalizedStrings(kPointersStrings);
 

chrome/test/data/webui/settings/chromeos/device_page/per_device_keyboard_remap_keys_test.ts

@@ -3,19 +3,24 @@
 // found in the LICENSE file.
 
 import {FakeInputDeviceSettingsProvider, fakeKeyboards, Keyboard, KeyboardRemapModifierKeyRowElement, MetaKey, ModifierKey, Router, routes, setInputDeviceSettingsProviderForTesting, SettingsPerDeviceKeyboardRemapKeysElement} from 'chrome://os-settings/os_settings.js';
+import {loadTimeData} from 'chrome://resources/ash/common/load_time_data.m.js';
 import {assert} from 'chrome://resources/js/assert_ts.js';
 import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
 import {flushTasks} from 'chrome://webui-test/polymer_test_util.js';
+import {isVisible} from 'chrome://webui-test/test_util.js';
 
 suite('<settings-per-device-keyboard-remap-keys>', () => {
   let page: SettingsPerDeviceKeyboardRemapKeysElement;
   let provider: FakeInputDeviceSettingsProvider;
 
-  setup(async () => {
+  teardown(() => {
+    page.remove();
+  });
+
+  async function initializePerDeviceKeyboardRemapKeys(): Promise<void> {
     provider = new FakeInputDeviceSettingsProvider();
     provider.setFakeKeyboards(fakeKeyboards);
     setInputDeviceSettingsProviderForTesting(provider);
-
     page = document.createElement('settings-per-device-keyboard-remap-keys');
     page.set('keyboards', fakeKeyboards);
     assertFalse(page.get('isInitialized'));
@@ -29,12 +34,8 @@ suite('<settings-per-device-keyboard-remap-keys>', () => {
 
     document.body.appendChild(page);
     provider.observeKeyboardSettings(page);
-    await flushTasks();
-  });
-
-  teardown(() => {
-    page.remove();
-  });
+    return flushTasks();
+  }
 
   function changeKeyboardExternalState(isExternal: boolean): Promise<void> {
     page.keyboard = {...page.keyboard, isExternal};
@@ -73,6 +74,7 @@ suite('<settings-per-device-keyboard-remap-keys>', () => {
    * Verify that the remap subpage is correctly loaded with keyboard data.
    */
   test('keyboard remap subpage loaded', async () => {
+    await initializePerDeviceKeyboardRemapKeys();
     assert(page.get('keyboard'));
 
     // Verify that the dropdown menu for unremapped key is displayed as default.
@@ -127,6 +129,7 @@ suite('<settings-per-device-keyboard-remap-keys>', () => {
    * keyboardId is passed through the query url.
    */
   test('keyboard remap subpage updated for different keyboard', async () => {
+    await initializePerDeviceKeyboardRemapKeys();
     // Update the subpage with a new keyboard.
     const url = new URLSearchParams(
         'keyboardId=' + encodeURIComponent(fakeKeyboards[2]!.id));
@@ -200,6 +203,7 @@ suite('<settings-per-device-keyboard-remap-keys>', () => {
    * Verify that the restore defaults button will restore the remapping keys.
    */
   test('keyboard remap subpage restore defaults', async () => {
+    await initializePerDeviceKeyboardRemapKeys();
     page.restoreDefaults();
     await flushTasks();
 
@@ -265,6 +269,7 @@ suite('<settings-per-device-keyboard-remap-keys>', () => {
    * the remapping page, it will switch back to per device keyboard page.
    */
   test('re-route to back page when keyboard disconnected', async () => {
+    await initializePerDeviceKeyboardRemapKeys();
     // Check it's currently in the modifier remapping page.
     assertEquals(
         routes.PER_DEVICE_KEYBOARD_REMAP_KEYS,
@@ -280,6 +285,7 @@ suite('<settings-per-device-keyboard-remap-keys>', () => {
    * prefs settings change.
    */
   test('Update keyboard settings', async () => {
+    await initializePerDeviceKeyboardRemapKeys();
     assertTrue(page.get('isInitialized'));
     // Set the modifier remappings to default stage.
     page.restoreDefaults();
@@ -305,6 +311,7 @@ suite('<settings-per-device-keyboard-remap-keys>', () => {
   });
 
   test('Keyboard description populated correctly', async () => {
+    await initializePerDeviceKeyboardRemapKeys();
     assertTrue(page.get('isInitialized'));
     assertEquals(
         'For ERGO K860, choose an action for each key', getPageDescription());
@@ -313,4 +320,17 @@ suite('<settings-per-device-keyboard-remap-keys>', () => {
         'For Built-in Keyboard, choose an action for each key',
         getPageDescription());
   });
+
+  test(
+      'Keys grouped under Modifier/Other sections when alt flag enabled',
+      async () => {
+        loadTimeData.overrideValues({
+          enableAltClickAndSixPackCustomization: true,
+        });
+        await initializePerDeviceKeyboardRemapKeys();
+        assertTrue(
+            isVisible(page.shadowRoot!.querySelector('#modifierKeysHeader')));
+        assertTrue(
+            isVisible(page.shadowRoot!.querySelector('#otherKeysHeader')));
+      });
 });