From e87b6293fe519da65ad436b21dd059b4cb845fd4 Mon Sep 17 00:00:00 2001 From: qa46hx Date: Thu, 8 Jul 2021 09:21:57 +0200 Subject: [PATCH] fix(form-core): only preserve caret if value changed --- .changeset/thin-bulldogs-warn.md | 7 ++ .../form-core/src/NativeTextFieldMixin.js | 15 +++- .../test-helpers/getFormControlMembers.js | 2 +- .../test-suites/NativeTextFieldMixin.suite.js | 69 +++++++++++++++++++ packages/form-core/test-suites/index.js | 1 + .../test/NativeTextFieldMixin.test.js | 3 + .../input/test/input-integrations.test.js | 10 ++- .../test/lion-textarea-integrations.test.js | 14 +++- 8 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 .changeset/thin-bulldogs-warn.md create mode 100644 packages/form-core/test-suites/NativeTextFieldMixin.suite.js create mode 100644 packages/form-core/test/NativeTextFieldMixin.test.js diff --git a/.changeset/thin-bulldogs-warn.md b/.changeset/thin-bulldogs-warn.md new file mode 100644 index 000000000..a4987c752 --- /dev/null +++ b/.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 diff --git a/packages/form-core/src/NativeTextFieldMixin.js b/packages/form-core/src/NativeTextFieldMixin.js index 69747ea51..37726237a 100644 --- a/packages/form-core/src/NativeTextFieldMixin.js +++ b/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) { - this._setValueAndPreserveCaret(value); + // 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 { diff --git a/packages/form-core/test-helpers/getFormControlMembers.js b/packages/form-core/test-helpers/getFormControlMembers.js index aae6c009b..a3cf97e16 100644 --- a/packages/form-core/test-helpers/getFormControlMembers.js +++ b/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, diff --git a/packages/form-core/test-suites/NativeTextFieldMixin.suite.js b/packages/form-core/test-suites/NativeTextFieldMixin.suite.js new file mode 100644 index 000000000..52c1e0430 --- /dev/null +++ b/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}>`)); + // @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}>`)); + // @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; + }); + }); +} diff --git a/packages/form-core/test-suites/index.js b/packages/form-core/test-suites/index.js index c54f2b627..a8197c560 100644 --- a/packages/form-core/test-suites/index.js +++ b/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'; diff --git a/packages/form-core/test/NativeTextFieldMixin.test.js b/packages/form-core/test/NativeTextFieldMixin.test.js new file mode 100644 index 000000000..3a58383b6 --- /dev/null +++ b/packages/form-core/test/NativeTextFieldMixin.test.js @@ -0,0 +1,3 @@ +import { runNativeTextFieldMixinSuite } from '../test-suites/NativeTextFieldMixin.suite.js'; + +runNativeTextFieldMixinSuite(); diff --git a/packages/input/test/input-integrations.test.js b/packages/input/test/input-integrations.test.js index 2e1570fcb..20d8b7086 100644 --- a/packages/input/test/input-integrations.test.js +++ b/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(' integrations', () => { runFormatMixinSuite({ tagString: fieldTagString, }); + + runNativeTextFieldMixinSuite({ + tagString: fieldTagString, + }); }); diff --git a/packages/textarea/test/lion-textarea-integrations.test.js b/packages/textarea/test/lion-textarea-integrations.test.js index 321a26406..330fb7c31 100644 --- a/packages/textarea/test/lion-textarea-integrations.test.js +++ b/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(' integrations', () => { + runInteractionStateMixinSuite({ + tagString, + }); + runFormatMixinSuite({ tagString, }); + + runNativeTextFieldMixinSuite({ + tagString, + }); });