Merge pull request #1450 from ing-bank/fix/input-safari

fix(form-core): only preserve caret if value changed

.changeset/thin-bulldogs-warn.md

@@ -0,0 +1,7 @@
+---
+'@lion/form-core': patch
+'@lion/input': patch
+'@lion/textarea': patch
+---
+
+only preserve caret if value changed, which fixes a safari bug

packages/form-core/src/NativeTextFieldMixin.js

@@ -67,16 +67,25 @@ const NativeTextFieldMixinImplementation = superclass =>
       }
     }
 
+    /**
+     * The view value. Will be delegated to `._inputNode.value`
+     * @override FormatMixin
+     */
     get value() {
       return (this._inputNode && this._inputNode.value) || this.__value || '';
     }
 
-    // We don't delegate, because we want to preserve caret position via _setValueAndPreserveCaret
-    /** @param {string} value */
+    /**
+     * @param {string} value
+     * @override FormatMixin - We don't delegate, because we want to preserve caret position via _setValueAndPreserveCaret
+     */
     set value(value) {
       // if not yet connected to dom can't change the value
       if (this._inputNode) {
+        // Only set if newValue is new, fix for Safari bug: https://github.com/ing-bank/lion/issues/1415
+        if (this._inputNode.value !== value) {
           this._setValueAndPreserveCaret(value);
+        }
         /** @type {string | undefined} */
         this.__value = undefined;
       } else {

packages/form-core/test-helpers/getFormControlMembers.js

@@ -12,7 +12,7 @@ export function getFormControlMembers(el) {
   // eslint-disable-next-line
   const { _inputNode, _helpTextNode, _labelNode, _feedbackNode, _allValidators } = el;
   return {
-    _inputNode,
+    _inputNode: /** @type {* & FormControlHost} */ (el)._inputNode,
     _helpTextNode,
     _labelNode,
     _feedbackNode,

packages/form-core/test-suites/NativeTextFieldMixin.suite.js

@@ -0,0 +1,69 @@
+import { LitElement } from '@lion/core';
+import { getFormControlMembers } from '@lion/form-core/test-helpers';
+import { defineCE, expect, fixture, html, triggerFocusFor, unsafeStatic } from '@open-wc/testing';
+import { sendKeys } from '@web/test-runner-commands';
+import { spy } from 'sinon';
+import { NativeTextFieldMixin } from '../src/NativeTextFieldMixin.js';
+
+/**
+ * @typedef {import('../types/FormControlMixinTypes').FormControlHost} FormControlHost
+ * @typedef {ArrayConstructor | ObjectConstructor | NumberConstructor | BooleanConstructor | StringConstructor | DateConstructor | 'iban' | 'email'} modelValueType
+ */
+
+/**
+ * @param {{tagString?: string, modelValueType?: modelValueType}} [customConfig]
+ */
+export function runNativeTextFieldMixinSuite(customConfig) {
+  const cfg = {
+    tagString: null,
+    ...customConfig,
+  };
+
+  describe('NativeTextFieldMixin', () => {
+    class NativeTextFieldClass extends NativeTextFieldMixin(LitElement) {
+      get slots() {
+        return {
+          // NativeTextFieldClass needs to have an _inputNode defined in order to work...
+          input: () => document.createElement('input'),
+        };
+      }
+    }
+
+    cfg.tagString = cfg.tagString ? cfg.tagString : defineCE(NativeTextFieldClass);
+    const tag = unsafeStatic(cfg.tagString);
+
+    it('preserves the caret position on value change', async () => {
+      const el = /** @type {NativeTextFieldClass} */ (await fixture(html`<${tag}></${tag}>`));
+      // @ts-ignore [allow-protected] in test
+      const setValueAndPreserveCaretSpy = spy(el, '_setValueAndPreserveCaret');
+      const { _inputNode } = getFormControlMembers(el);
+      await triggerFocusFor(el);
+      await el.updateComplete;
+      _inputNode.value = 'hello world';
+      _inputNode.selectionStart = 2;
+      _inputNode.selectionEnd = 2;
+      el.value = 'hey there universe';
+      expect(setValueAndPreserveCaretSpy.calledOnce).to.be.true;
+      expect(_inputNode.selectionStart).to.equal(2);
+      expect(_inputNode.selectionEnd).to.equal(2);
+    });
+
+    it('move focus to a next focusable element after writing some text', async () => {
+      const el = /** @type {NativeTextFieldClass} */ (await fixture(html`<${tag}></${tag}>`));
+      // @ts-ignore [allow-protected] in test
+      const setValueAndPreserveCaretSpy = spy(el, '_setValueAndPreserveCaret');
+      const { _inputNode } = getFormControlMembers(el);
+      await triggerFocusFor(el);
+      await el.updateComplete;
+      expect(document.activeElement).to.equal(_inputNode);
+      await sendKeys({
+        press: 'h',
+      });
+      await sendKeys({
+        press: 'Tab',
+      });
+      expect(document.activeElement).to.not.equal(_inputNode);
+      expect(setValueAndPreserveCaretSpy.calledOnce).to.be.false;
+    });
+  });
+}

packages/form-core/test-suites/index.js

@@ -5,5 +5,6 @@ export { runFormGroupMixinSuite } from './form-group/FormGroupMixin.suite.js';
 export { runFormatMixinSuite } from './FormatMixin.suite.js';
 export { runRegistrationSuite } from './FormRegistrationMixins.suite.js';
 export { runInteractionStateMixinSuite } from './InteractionStateMixin.suite.js';
+export { runNativeTextFieldMixinSuite } from './NativeTextFieldMixin.suite.js';
 export { runValidateMixinSuite } from './ValidateMixin.suite.js';
 export { runValidateMixinFeedbackPart } from './ValidateMixinFeedbackPart.suite.js';

packages/form-core/test/NativeTextFieldMixin.test.js

@@ -0,0 +1,3 @@
+import { runNativeTextFieldMixinSuite } from '../test-suites/NativeTextFieldMixin.suite.js';
+
+runNativeTextFieldMixinSuite();

packages/input/test/input-integrations.test.js

@@ -1,5 +1,9 @@
 import { defineCE } from '@open-wc/testing';
-import { runInteractionStateMixinSuite, runFormatMixinSuite } from '@lion/form-core/test-suites';
+import {
+  runInteractionStateMixinSuite,
+  runFormatMixinSuite,
+  runNativeTextFieldMixinSuite,
+} from '@lion/form-core/test-suites';
 
 import { LionInput } from '../src/LionInput.js';
 
@@ -23,4 +27,8 @@ describe('<lion-input> integrations', () => {
   runFormatMixinSuite({
     tagString: fieldTagString,
   });
+
+  runNativeTextFieldMixinSuite({
+    tagString: fieldTagString,
+  });
 });

packages/textarea/test/lion-textarea-integrations.test.js

@@ -1,11 +1,23 @@
-import { runFormatMixinSuite } from '@lion/form-core/test-suites';
+import {
+  runInteractionStateMixinSuite,
+  runFormatMixinSuite,
+  runNativeTextFieldMixinSuite,
+} from '@lion/form-core/test-suites';
 
 import '@lion/textarea/define';
 
 const tagString = 'lion-textarea';
 
 describe('<lion-textarea> integrations', () => {
+  runInteractionStateMixinSuite({
+    tagString,
+  });
+
   runFormatMixinSuite({
     tagString,
   });
+
+  runNativeTextFieldMixinSuite({
+    tagString,
+  });
 });