From c6fbe9208ad43db17d99543b9f732de783530b42 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Tue, 20 Apr 2021 10:32:54 +0200 Subject: [PATCH 1/2] feat(form-core): support format on paste --- .changeset/wicked-games-dress.md | 5 ++ packages/form-core/src/FormatMixin.js | 56 +++++++++++++--- .../form-core/src/NativeTextFieldMixin.js | 17 ++++- .../test-suites/FormatMixin.suite.js | 67 +++++++++++++++++++ .../form-core/types/FormatMixinTypes.d.ts | 13 +++- .../types/NativeTextFieldMixinTypes.d.ts | 3 + 6 files changed, 150 insertions(+), 11 deletions(-) create mode 100644 .changeset/wicked-games-dress.md diff --git a/.changeset/wicked-games-dress.md b/.changeset/wicked-games-dress.md new file mode 100644 index 000000000..0cc28273f --- /dev/null +++ b/.changeset/wicked-games-dress.md @@ -0,0 +1,5 @@ +--- +'@lion/form-core': minor +--- + +support paste functionality in FormatMixin diff --git a/packages/form-core/src/FormatMixin.js b/packages/form-core/src/FormatMixin.js index cf2b611ab..4907de5b0 100644 --- a/packages/form-core/src/FormatMixin.js +++ b/packages/form-core/src/FormatMixin.js @@ -129,7 +129,7 @@ const FormatMixinImplementation = superclass => } // We don't delegate, because we want to preserve caret position via _setValueAndPreserveCaret - /** @type {string} */ + /** @param {string} value */ set value(value) { // if not yet connected to dom can't change the value if (this._inputNode) { @@ -350,7 +350,16 @@ const FormatMixinImplementation = superclass => if (!this.__isHandlingComposition) { this.value = this.preprocessor(this.value); } + const prevFormatted = this.formattedValue; this.modelValue = this._callParser(this.value); + + // Sometimes, the formattedValue didn't change, but the viewValue did... + // We need this check to support pasting values that need to be formatted right on paste + if (prevFormatted === this.formattedValue && this.__prevViewValue !== this.value) { + this._calculateValues(); + } + /** @type {string} */ + this.__prevViewValue = this.value; } /** @@ -379,11 +388,13 @@ const FormatMixinImplementation = superclass => return !this._isHandlingUserInput; } - // This can be called whenever the view value should be updated. Dependent on component type - // ("input" for or "change" for or "change" for to the formatting/parsing/serializing loop as a // fallback mechanism. Assume the user uses the value property of the // `LionField`(recommended api) as the api (this is a downwards sync). @@ -453,7 +492,6 @@ const FormatMixinImplementation = superclass => disconnectedCallback() { super.disconnectedCallback(); - this.removeEventListener('user-input-changed', this._onUserInputChanged); if (this._inputNode) { this._inputNode.removeEventListener('input', this._proxyInputEvent); this._inputNode.removeEventListener( diff --git a/packages/form-core/src/NativeTextFieldMixin.js b/packages/form-core/src/NativeTextFieldMixin.js index 4c32bc071..47bd38b48 100644 --- a/packages/form-core/src/NativeTextFieldMixin.js +++ b/packages/form-core/src/NativeTextFieldMixin.js @@ -1,6 +1,7 @@ import { dedupeMixin } from '@lion/core'; import { FormControlMixin } from './FormControlMixin.js'; import { FocusMixin } from './FocusMixin.js'; +import { FormatMixin } from './FormatMixin.js'; /** * @typedef {import('../types/NativeTextFieldMixinTypes').NativeTextFieldMixin} NativeTextFieldMixin @@ -8,7 +9,7 @@ import { FocusMixin } from './FocusMixin.js'; * @param {import('@open-wc/dedupe-mixin').Constructor} superclass} superclass */ const NativeTextFieldMixinImplementation = superclass => - class NativeTextFieldMixin extends FocusMixin(FormControlMixin(superclass)) { + class NativeTextFieldMixin extends FormatMixin(FocusMixin(FormControlMixin(superclass))) { /** * @protected * @type {HTMLInputElement | HTMLTextAreaElement} @@ -95,6 +96,20 @@ const NativeTextFieldMixinImplementation = superclass => this._inputNode.value = newValue; } } + + /** + * @override FormatMixin + */ + _reflectBackFormattedValueToUser() { + super._reflectBackFormattedValueToUser(); + if (this._reflectBackOn() && this.focused) { + try { + // try/catch, because Safari is a bit sensitive here + this._inputNode.selectionStart = this._inputNode.value.length; + // eslint-disable-next-line no-empty + } catch (_) {} + } + } }; export const NativeTextFieldMixin = dedupeMixin(NativeTextFieldMixinImplementation); diff --git a/packages/form-core/test-suites/FormatMixin.suite.js b/packages/form-core/test-suites/FormatMixin.suite.js index f222ac7e3..6f88b333c 100644 --- a/packages/form-core/test-suites/FormatMixin.suite.js +++ b/packages/form-core/test-suites/FormatMixin.suite.js @@ -462,6 +462,73 @@ export function runFormatMixinSuite(customConfig) { expect(spyArg.locale).to.equal('en-GB'); expect(spyArg.decimalSeparator).to.equal('-'); }); + + describe('On paste', () => { + class ReflectOnPaste extends FormatClass { + _reflectBackOn() { + return super._reflectBackOn() || this._isPasting; + } + } + const reflectingTagString = defineCE(ReflectOnPaste); + const reflectingTag = unsafeStatic(reflectingTagString); + + /** + * @param {FormatClass} el + */ + function paste(el, val = 'lorem') { + const { _inputNode } = getFormControlMembers(el); + _inputNode.value = val; + _inputNode.dispatchEvent(new ClipboardEvent('paste', { bubbles: true })); + _inputNode.dispatchEvent(new Event('input', { bubbles: true })); + } + + it('sets formatOptions.mode to "pasted" (and restores to "auto")', async () => { + const el = /** @type {FormatClass} */ (await fixture(html` + <${reflectingTag}> + `)); + const formatterSpy = sinon.spy(el, 'formatter'); + paste(el); + expect(formatterSpy).to.be.called; + expect(/** @type {{mode: string}} */ (formatterSpy.args[0][1]).mode).to.equal('pasted'); + await aTimeout(0); + mimicUserInput(el, ''); + expect(/** @type {{mode: string}} */ (formatterSpy.args[0][1]).mode).to.equal('auto'); + }); + + it('sets protected value "_isPasting" for Subclassers', async () => { + const el = /** @type {FormatClass} */ (await fixture(html` + <${reflectingTag}> + `)); + const formatterSpy = sinon.spy(el, 'formatter'); + paste(el); + expect(formatterSpy).to.have.been.called; + // @ts-ignore [allow-protected] in test + expect(el._isPasting).to.be.true; + await aTimeout(0); + // @ts-ignore [allow-protected] in test + expect(el._isPasting).to.be.false; + }); + + it('calls formatter and "_reflectBackOn()"', async () => { + const el = /** @type {FormatClass} */ (await fixture(html` + <${tag}> + `)); + // @ts-ignore [allow-protected] in test + const reflectBackSpy = sinon.spy(el, '_reflectBackOn'); + paste(el); + expect(reflectBackSpy).to.have.been.called; + }); + + it(`updates viewValue when "_reflectBackOn()" configured to reflect`, async () => { + const el = /** @type {FormatClass} */ (await fixture(html` + <${reflectingTag}> + `)); + // @ts-ignore [allow-protected] in test + const reflectBackSpy = sinon.spy(el, '_reflectBackOn'); + paste(el); + expect(reflectBackSpy).to.have.been.called; + }); + }); }); describe('Parser', () => { diff --git a/packages/form-core/types/FormatMixinTypes.d.ts b/packages/form-core/types/FormatMixinTypes.d.ts index 1c19c336c..47500ff12 100644 --- a/packages/form-core/types/FormatMixinTypes.d.ts +++ b/packages/form-core/types/FormatMixinTypes.d.ts @@ -1,5 +1,5 @@ import { Constructor } from '@open-wc/dedupe-mixin'; -import { LitElement } from '@lion/core'; +import { BooleanAttributePart, LitElement } from '@lion/core'; import { FormatNumberOptions } from '@lion/localize/types/LocalizeMixinTypes'; import { ValidateHost } from './validate/ValidateMixinTypes'; import { FormControlHost } from './FormControlMixinTypes'; @@ -18,6 +18,16 @@ export declare class FormatHost { set value(value: string); protected _isHandlingUserInput: boolean; + /** + * Whether the user is pasting content. Allows Subclassers to do this in their subclass: + * @example + * ```js + * _reflectBackFormattedValueToUser() { + * return super._reflectBackFormattedValueToUser() || this._isPasting; + * } + * ``` + */ + protected _isPasting: boolean; protected _calculateValues(opts: { source: 'model' | 'serialized' | 'formatted' | null }): void; protected _onModelValueChanged(arg: { modelValue: unknown }): void; protected _dispatchModelValueChangedEvent(): void; @@ -31,6 +41,7 @@ export declare class FormatHost { protected _callFormatter(): string; private __preventRecursiveTrigger: boolean; + private __prevViewValue: string; } export declare function FormatImplementation>( diff --git a/packages/form-core/types/NativeTextFieldMixinTypes.d.ts b/packages/form-core/types/NativeTextFieldMixinTypes.d.ts index fce49290e..5f0df11de 100644 --- a/packages/form-core/types/NativeTextFieldMixinTypes.d.ts +++ b/packages/form-core/types/NativeTextFieldMixinTypes.d.ts @@ -2,6 +2,7 @@ import { Constructor } from '@open-wc/dedupe-mixin'; import { LitElement } from '@lion/core'; import { FocusHost } from '@lion/form-core/types/FocusMixinTypes'; import { FormControlHost } from '@lion/form-core/types/FormControlMixinTypes'; +import { FormatHost } from '@lion/form-core/types/FormatMixinTypes'; export declare class NativeTextFieldHost { get selectionStart(): number; @@ -15,6 +16,8 @@ export declare function NativeTextFieldImplementation & Pick & + Constructor & + Pick & Constructor & Pick & Constructor & From 756a1384330477f78618dfdc4cf28478e2438ff4 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Tue, 20 Apr 2021 10:33:09 +0200 Subject: [PATCH 2/2] fix(input-amount): always format on paste --- .changeset/friendly-pianos-pay.md | 5 ++++ .../inputs/input-amount/features.md | 2 +- packages/form-core/src/FormatMixin.js | 4 +-- packages/input-amount/src/LionInputAmount.js | 28 ++----------------- 4 files changed, 11 insertions(+), 28 deletions(-) create mode 100644 .changeset/friendly-pianos-pay.md diff --git a/.changeset/friendly-pianos-pay.md b/.changeset/friendly-pianos-pay.md new file mode 100644 index 000000000..d33d9c79e --- /dev/null +++ b/.changeset/friendly-pianos-pay.md @@ -0,0 +1,5 @@ +--- +'@lion/input-amount': patch +--- + +always format on paste diff --git a/docs/components/inputs/input-amount/features.md b/docs/components/inputs/input-amount/features.md index a0c69d031..59de27318 100644 --- a/docs/components/inputs/input-amount/features.md +++ b/docs/components/inputs/input-amount/features.md @@ -107,7 +107,7 @@ export const noDecimals = () => html` For copy pasting numbers into the input-amount, there is slightly different parsing behavior. Normally, when it receives an input with only 1 separator character, we check the locale to determine whether this character is a thousand separator, or a decimal separator. -When a user pastes the input from a different source, we find this approach (locale-based) quite unreliable, because it may have been copied from somewhere with a different locale. +When a user pastes the input from a different source, we find this approach (locale-based) quite unreliable, because it may have been copied from a 'mathematical context' (like an Excel sheet) or a context with a different locale. Therefore, we use the heuristics based method to parse the input when it is pasted by the user. ### What this means diff --git a/packages/form-core/src/FormatMixin.js b/packages/form-core/src/FormatMixin.js index 4907de5b0..84d01ec28 100644 --- a/packages/form-core/src/FormatMixin.js +++ b/packages/form-core/src/FormatMixin.js @@ -434,8 +434,8 @@ const FormatMixinImplementation = superclass => * Whether the user is pasting content. Allows Subclassers to do this in their subclass: * @example * ```js - * _reflectBackFormattedValueToUser() { - * return super._reflectBackFormattedValueToUser() || this._isPasting; + * _reflectBackOn() { + * return super._reflectBackOn() || this._isPasting; * } * ``` * @protected diff --git a/packages/input-amount/src/LionInputAmount.js b/packages/input-amount/src/LionInputAmount.js index 2045a9201..746c2e1a4 100644 --- a/packages/input-amount/src/LionInputAmount.js +++ b/packages/input-amount/src/LionInputAmount.js @@ -68,16 +68,6 @@ export class LionInputAmount extends LocalizeMixin(LionInput) { this.formatter = formatAmount; /** @type {string | undefined} */ this.currency = undefined; - /** @private */ - this.__isPasting = false; - - this.addEventListener('paste', () => { - /** @private */ - this.__isPasting = true; - /** @private */ - this.__parserCallcountSincePaste = 0; - }); - this.defaultValidators.push(new IsNumber()); } @@ -101,24 +91,12 @@ export class LionInputAmount extends LocalizeMixin(LionInput) { } /** - * @override of FormatMixin - * @protected - */ - _callParser(value = this.formattedValue) { - // TODO: (@daKmor) input and change events both trigger parsing therefore we need to handle the second parse - this.__parserCallcountSincePaste += 1; - this.__isPasting = this.__parserCallcountSincePaste === 2; - this.formatOptions.mode = this.__isPasting === true ? 'pasted' : 'auto'; - // @ts-ignore [allow-protected] - return super._callParser(value); - } - - /** - * @override of FormatMixin + * @enhance FormatMixin: instead of only formatting on blur, also format when a user pasted + * content * @protected */ _reflectBackOn() { - return super._reflectBackOn() || this.__isPasting; + return super._reflectBackOn() || this._isPasting; } /**