Merge pull request #1346 from ing-bank/feat/pasting

feat(form-core): support paste functionality in FormatMixin
This commit is contained in:
Thijs Louisse 2021-04-20 13:49:55 +02:00 committed by GitHub
commit 3e0eaaa186
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 159 additions and 37 deletions

View file

@ -0,0 +1,5 @@
---
'@lion/input-amount': patch
---
always format on paste

View file

@ -0,0 +1,5 @@
---
'@lion/form-core': minor
---
support paste functionality in FormatMixin

View file

@ -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

View file

@ -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 <input> or "change" for <select>(mainly for IE)) a different event should be
// used as source for the "user-input-changed" event (which can be seen as an abstraction
// layer on top of other events (input, change, whatever))
/** @protected */
/**
* This can be called whenever the view value should be updated. Dependent on component type
* ("input" for <input> or "change" for <select>(mainly for IE)) a different event should be
* used as source for the "user-input-changed" event (which can be seen as an abstraction
* layer on top of other events (input, change, whatever))
* @protected
*/
_proxyInputEvent() {
this.dispatchEvent(
new CustomEvent('user-input-changed', {
@ -419,8 +430,36 @@ const FormatMixinImplementation = superclass =>
super();
this.formatOn = 'change';
this.formatOptions = /** @type {FormatOptions} */ ({});
/**
* Whether the user is pasting content. Allows Subclassers to do this in their subclass:
* @example
* ```js
* _reflectBackOn() {
* return super._reflectBackOn() || this._isPasting;
* }
* ```
* @protected
*/
this._isPasting = false;
/**
* @private
* @type {string}
*/
this.__prevViewValue = '';
this.__onCompositionEvent = this.__onCompositionEvent.bind(this);
// This computes formattedValue
this.addEventListener('user-input-changed', this._onUserInputChanged);
// This sets the formatted viewValue after paste
this.addEventListener('paste', this.__onPaste);
}
__onPaste() {
this._isPasting = true;
this.formatOptions.mode = 'pasted';
setTimeout(() => {
this._isPasting = false;
this.formatOptions.mode = 'auto';
});
}
connectedCallback() {
@ -432,7 +471,7 @@ const FormatMixinImplementation = superclass =>
// is guaranteed to be calculated
setTimeout(this._reflectBackFormattedValueToUser);
};
this.addEventListener('user-input-changed', this._onUserInputChanged);
// Connect the value found in <input> 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(

View file

@ -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<import('@lion/core').LitElement>} 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);

View file

@ -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}><input slot="input"></${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}><input slot="input"></${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}><input slot="input"></${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}><input slot="input"></${reflectingTag}>
`));
// @ts-ignore [allow-protected] in test
const reflectBackSpy = sinon.spy(el, '_reflectBackOn');
paste(el);
expect(reflectBackSpy).to.have.been.called;
});
});
});
describe('Parser', () => {

View file

@ -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<T extends Constructor<LitElement>>(

View file

@ -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<T extends Constructor<LitE
): T &
Constructor<NativeTextFieldHost> &
Pick<typeof NativeTextFieldHost, keyof typeof NativeTextFieldHost> &
Constructor<FormatHost> &
Pick<typeof FormatHost, keyof typeof FormatHost> &
Constructor<FocusHost> &
Pick<typeof FocusHost, keyof typeof FocusHost> &
Constructor<FormControlHost> &

View file

@ -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;
}
/**