fix(input-tel-dropdown): prevent jumping to input field on each arrow key in windows/linux

Apply suggestions from code review

.changeset/yellow-steaks-tie.md

@@ -0,0 +1,5 @@
+---
+'@lion/ui': patch
+---
+
+[input-tel-dropdown] prevent jumping to input field on each arrow key in windows/linux

packages/ui/components/input-tel-dropdown/src/LionInputTelDropdown.js

@@ -324,7 +324,10 @@ export class LionInputTelDropdown extends LionInputTel {
    */
   _onDropdownValueChange(event) {
     const isInitializing = event.detail?.initialize || !this._phoneUtil;
-    const dropdownValue = /** @type {RegionCode} */ (event.target.modelValue || event.target.value);
+    const dropdownElement = event.target;
+    const dropdownValue = /** @type {RegionCode} */ (
+      dropdownElement.modelValue || dropdownElement.value
+    );
 
     if (isInitializing || this.activeRegion === dropdownValue) {
       return;
@@ -355,8 +358,13 @@ export class LionInputTelDropdown extends LionInputTel {
     }
 
     // Put focus on text box
-    const overlayController = event.target._overlayCtrl;
+    //
-    if (overlayController?.isShown) {
+    // A LionSelectRich with interactionMode set on windows/linux
+    // will set each item on arrow key up/down to activeElement
+    // which causes the focus to jump every time to the inputNode
+    const overlayController = dropdownElement._overlayCtrl;
+    // @ts-ignore interactionMode only exists on LionSelectRich not on HTMLSelectElement
+    if (overlayController?.isShown && dropdownElement.interactionMode !== 'windows/linux') {
       setTimeout(() => {
         this._inputNode.focus();
       });

packages/ui/components/input-tel-dropdown/test-helpers/mimicUserChangingDropdown.js

@@ -0,0 +1,22 @@
+/**
+ * @typedef {HTMLSelectElement|HTMLElement & {modelValue:string}} DropdownElement
+ */
+
+/**
+ * @param {DropdownElement} dropdownEl
+ * @param {string} value
+ */
+export function mimicUserChangingDropdown(dropdownEl, value) {
+  if ('modelValue' in dropdownEl) {
+    // eslint-disable-next-line no-param-reassign
+    dropdownEl.modelValue = value;
+    dropdownEl.dispatchEvent(
+      new CustomEvent('model-value-changed', { detail: { isTriggeredByUser: true } }),
+    );
+  } else {
+    // eslint-disable-next-line no-param-reassign
+    dropdownEl.value = value;
+    dropdownEl.dispatchEvent(new Event('change'));
+    dropdownEl.dispatchEvent(new Event('input'));
+  }
+}

packages/ui/components/input-tel-dropdown/test-suites/LionInputTelDropdown.suite.js

@@ -11,6 +11,7 @@ import {
 } from '@open-wc/testing';
 import sinon from 'sinon';
 import { LionInputTelDropdown } from '@lion/ui/input-tel-dropdown.js';
+import { mimicUserChangingDropdown } from '@lion/ui/input-tel-dropdown-test-helpers.js';
 
 /**
  * @typedef {import('lit').TemplateResult} TemplateResult
@@ -36,24 +37,6 @@ function getDropdownValue(dropdownEl) {
   return dropdownEl.value;
 }
 
-/**
- * @param {DropdownElement} dropdownEl
- * @param {string} value
- */
-function mimicUserChangingDropdown(dropdownEl, value) {
-  if ('modelValue' in dropdownEl) {
-    // eslint-disable-next-line no-param-reassign
-    dropdownEl.modelValue = value;
-    dropdownEl.dispatchEvent(
-      new CustomEvent('model-value-changed', { detail: { isTriggeredByUser: true } }),
-    );
-  } else {
-    // eslint-disable-next-line no-param-reassign
-    dropdownEl.value = value;
-    dropdownEl.dispatchEvent(new Event('change'));
-  }
-}
-
 /**
  * @param {{ klass:LionInputTelDropdown }} config
  */
@@ -335,26 +318,6 @@ export function runInputTelDropdownSuite({ klass } = { klass: LionInputTelDropdo
         expect(el.value).to.equal('+32');
       });
 
-      it('focuses the textbox right after selection if selected via opened dropdown', async () => {
-        const el = await fixture(
-          html` <${tag} .allowedRegions="${[
-            'NL',
-            'BE',
-          ]}" .modelValue="${'+31612345678'}"></${tag}> `,
-        );
-        const dropdownElement = el.refs.dropdown.value;
-        // @ts-expect-error [allow-protected-in-tests]
-        if (dropdownElement?._overlayCtrl) {
-          // @ts-expect-error [allow-protected-in-tests]
-          dropdownElement._overlayCtrl.show();
-          mimicUserChangingDropdown(dropdownElement, 'BE');
-          await el.updateComplete;
-          await aTimeout(0);
-          // @ts-expect-error [allow-protected-in-tests]
-          expect(el._inputNode).to.equal(document.activeElement);
-        }
-      });
-
       it('keeps focus on dropdownElement after selection if selected via unopened dropdown', async () => {
         const el = await fixture(
           html` <${tag} .allowedRegions="${[

packages/ui/components/input-tel-dropdown/test/LionInputTelDropdown.test.js

@@ -1,10 +1,10 @@
 import { runInputTelSuite } from '@lion/ui/input-tel-test-suites.js';
-import { html } from 'lit';
 import { repeat } from 'lit/directives/repeat.js';
 import { ref } from 'lit/directives/ref.js';
-
+import { aTimeout, expect, fixture, html } from '@open-wc/testing';
 import { LionInputTelDropdown } from '@lion/ui/input-tel-dropdown.js';
 import { runInputTelDropdownSuite } from '@lion/ui/input-tel-dropdown-test-suites.js';
+import { mimicUserChangingDropdown } from '@lion/ui/input-tel-dropdown-test-helpers.js';
 
 import '@lion/ui/define/lion-option.js';
 import '@lion/ui/define/lion-select-rich.js';
@@ -53,4 +53,112 @@ describe('WithFormControlInputTelDropdown', () => {
   // @ts-expect-error
   // Runs it for LionSelectRich, which uses .modelValue/@model-value-changed instead of .value/@change
   runInputTelDropdownSuite({ klass: WithFormControlInputTelDropdown });
+
+  it('focuses the textbox right after selection if selected via opened dropdown if interaction-mode is mac', async () => {
+    class InputTelDropdownMac extends LionInputTelDropdown {
+      static templates = {
+        ...(super.templates || {}),
+        /**
+         * @param {TemplateDataForDropdownInputTel} templateDataForDropdown
+         */
+        dropdown: templateDataForDropdown => {
+          const { refs, data } = templateDataForDropdown;
+          // TODO: once spread directive available, use it per ref (like ref(refs?.dropdown?.ref))
+          return html`
+            <lion-select-rich
+              ${ref(refs?.dropdown?.ref)}
+              label="${refs?.dropdown?.labels?.country}"
+              label-sr-only
+              @model-value-changed="${refs?.dropdown?.listeners['model-value-changed']}"
+              style="${refs?.dropdown?.props?.style}"
+              interaction-mode="mac"
+            >
+              ${repeat(
+                data.regionMetaList,
+                regionMeta => regionMeta.regionCode,
+                regionMeta =>
+                  html` <lion-option .choiceValue="${regionMeta.regionCode}"> </lion-option> `,
+              )}
+            </lion-select-rich>
+          `;
+        },
+      };
+    }
+    customElements.define('input-tel-dropdown-mac', InputTelDropdownMac);
+    const el = /** @type {LionInputTelDropdown} */ (
+      await fixture(
+        html`
+          <input-tel-dropdown-mac
+            .allowedRegions="${['NL', 'BE']}"
+            .modelValue="${'+31612345678'}"
+          ></input-tel-dropdown-mac>
+        `,
+      )
+    );
+    const dropdownElement = el.refs.dropdown.value;
+    // @ts-expect-error [allow-protected-in-tests]
+    if (dropdownElement?._overlayCtrl) {
+      // @ts-expect-error [allow-protected-in-tests]
+      dropdownElement._overlayCtrl.show();
+      mimicUserChangingDropdown(dropdownElement, 'BE');
+      await el.updateComplete;
+      await aTimeout(0);
+      // @ts-expect-error [allow-protected-in-tests]
+      expect(el._inputNode).to.equal(document.activeElement);
+    }
+  });
+
+  it('does not focus the textbox right after selection if selected via opened dropdown if interaction-mode is windows/linux', async () => {
+    class InputTelDropdownWindows extends LionInputTelDropdown {
+      static templates = {
+        ...(super.templates || {}),
+        /**
+         * @param {TemplateDataForDropdownInputTel} templateDataForDropdown
+         */
+        dropdown: templateDataForDropdown => {
+          const { refs, data } = templateDataForDropdown;
+          // TODO: once spread directive available, use it per ref (like ref(refs?.dropdown?.ref))
+          return html`
+            <lion-select-rich
+              ${ref(refs?.dropdown?.ref)}
+              label="${refs?.dropdown?.labels?.country}"
+              label-sr-only
+              @model-value-changed="${refs?.dropdown?.listeners['model-value-changed']}"
+              style="${refs?.dropdown?.props?.style}"
+              interaction-mode="windows/linux"
+            >
+              ${repeat(
+                data.regionMetaList,
+                regionMeta => regionMeta.regionCode,
+                regionMeta =>
+                  html` <lion-option .choiceValue="${regionMeta.regionCode}"> </lion-option> `,
+              )}
+            </lion-select-rich>
+          `;
+        },
+      };
+    }
+    customElements.define('input-tel-dropdown-windows', InputTelDropdownWindows);
+    const el = /** @type {LionInputTelDropdown} */ (
+      await fixture(
+        html`
+          <input-tel-dropdown-windows
+            .allowedRegions="${['NL', 'BE']}"
+            .modelValue="${'+31612345678'}"
+          ></input-tel-dropdown-windows>
+        `,
+      )
+    );
+    const dropdownElement = el.refs.dropdown.value;
+    // @ts-expect-error [allow-protected-in-tests]
+    if (dropdownElement?._overlayCtrl) {
+      // @ts-expect-error [allow-protected-in-tests]
+      dropdownElement._overlayCtrl.show();
+      mimicUserChangingDropdown(dropdownElement, 'BE');
+      await el.updateComplete;
+      await aTimeout(0);
+      // @ts-expect-error [allow-protected-in-tests]
+      expect(el._inputNode).to.not.equal(document.activeElement);
+    }
+  });
 });

packages/ui/exports/input-tel-dropdown-test-helpers.js

@@ -0,0 +1 @@
+export { mimicUserChangingDropdown } from '../components/input-tel-dropdown/test-helpers/mimicUserChangingDropdown.js';