From 6dbf671f9f5754c1145e02b068f2a3c5b140a4db Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Mon, 13 Feb 2023 11:51:21 +0100 Subject: [PATCH] wip --- .../components/form-core/src/FormatMixin.js | 55 +++++-- .../form-group/FormGroupMixin.suite.js | 2 +- .../form-core/test/lion-field.test.js | 26 ++- .../input-date/src/LionInputDate.js | 31 +++- .../input-date/test/lion-input-date.test.js | 151 ++++++++++-------- 5 files changed, 165 insertions(+), 100 deletions(-) diff --git a/packages/ui/components/form-core/src/FormatMixin.js b/packages/ui/components/form-core/src/FormatMixin.js index d80636db00..2440d0bca3 100644 --- a/packages/ui/components/form-core/src/FormatMixin.js +++ b/packages/ui/components/form-core/src/FormatMixin.js @@ -4,6 +4,7 @@ import { dedupeMixin } from '@open-wc/dedupe-mixin'; import { FormControlMixin } from './FormControlMixin.js'; import { Unparseable } from './validate/Unparseable.js'; import { ValidateMixin } from './validate/ValidateMixin.js'; +import { SyncUpdatableMixin } from './utils/SyncUpdatableMixin.js'; /** * @typedef {import('../types/FormatMixinTypes.js').FormatMixin} FormatMixin @@ -59,7 +60,7 @@ import { ValidateMixin } from './validate/ValidateMixin.js'; */ const FormatMixinImplementation = superclass => // @ts-ignore https://github.com/microsoft/TypeScript/issues/36821#issuecomment-588375051 - class FormatMixin extends ValidateMixin(FormControlMixin(superclass)) { + class FormatMixin extends ValidateMixin(FormControlMixin(SyncUpdatableMixin(superclass))) { /** @type {any} */ static get properties() { return { @@ -69,22 +70,40 @@ const FormatMixinImplementation = superclass => }; } + // /** + // * @param {string} [name] + // * @param {unknown} [oldValue] + // * @param {import('lit').PropertyDeclaration} [options] + // * @returns {void} + // */ + // requestUpdate(name, oldValue, options) { + // super.requestUpdate(name, oldValue, options); + + // // if (name === 'modelValue' && this.modelValue !== oldValue) { + // // this._onModelValueChanged({ modelValue: this.modelValue }, { modelValue: oldValue }); + // // } + // // if (name === 'serializedValue' && this.serializedValue !== oldValue) { + // // this._calculateValues({ source: 'serialized' }); + // // } + // // if (name === 'formattedValue' && this.formattedValue !== oldValue) { + // // this._calculateValues({ source: 'formatted' }); + // // } + // } + /** - * @param {string} [name] + * @param {string} name * @param {unknown} [oldValue] - * @param {import('lit').PropertyDeclaration} [options] - * @returns {void} */ - requestUpdate(name, oldValue, options) { - super.requestUpdate(name, oldValue, options); + updateSync(name, oldValue) { + super.updateSync(name, oldValue); - if (name === 'modelValue' && this.modelValue !== oldValue) { + if (name === 'modelValue') { this._onModelValueChanged({ modelValue: this.modelValue }, { modelValue: oldValue }); } - if (name === 'serializedValue' && this.serializedValue !== oldValue) { + if (name === 'serializedValue') { this._calculateValues({ source: 'serialized' }); } - if (name === 'formattedValue' && this.formattedValue !== oldValue) { + if (name === 'formattedValue') { this._calculateValues({ source: 'formatted' }); } } @@ -93,7 +112,7 @@ const FormatMixinImplementation = superclass => * The view value. Will be delegated to `._inputNode.value` */ get value() { - return (this._inputNode && this._inputNode.value) || this.__value || ''; + return this._inputNode?.value || this.__value || ''; } /** @param {string} value */ @@ -269,12 +288,14 @@ const FormatMixinImplementation = superclass => // imperatively, we DO want to format a value (it is the only way to get meaningful // input into `._inputNode` with modelValue as input) - if ( - this._isHandlingUserInput && - this.hasFeedbackFor?.length && - this.hasFeedbackFor.includes('error') && - this._inputNode - ) { + console.debug( + '_callFormatter', + this._isHandlingUserInput, + this.hasFeedbackFor, + this._inputNode, + ); + + if (this._isHandlingUserInput && this.hasFeedbackFor?.includes('error') && this._inputNode) { return this._inputNode ? this.value : undefined; } @@ -416,6 +437,7 @@ const FormatMixinImplementation = superclass => * @protected */ _proxyInputEvent() { + console.debug('_proxyInputEvent'); // TODO: [v1] remove composed (and bubbles as well if possible) /** @protectedEvent user-input-changed meant for usage by Subclassers only */ this.dispatchEvent(new Event('user-input-changed', { bubbles: true })); @@ -423,6 +445,7 @@ const FormatMixinImplementation = superclass => /** @protected */ _onUserInputChanged() { + console.debug('_onUserInputChanged'); // Upwards syncing. Most properties are delegated right away, value is synced to // `LionField`, to be able to act on (imperatively set) value changes this._isHandlingUserInput = true; diff --git a/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js b/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js index 13ef9e4e13..aa6c1d5e40 100644 --- a/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js +++ b/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js @@ -1086,7 +1086,7 @@ export function runFormGroupMixinSuite(cfg = {}) { expect(input.modelValue).to.equal('Foo'); }); - it('clears interaction state', async () => { + it.only('clears interaction state', async () => { const el = /** @type {FormGroup} */ ( await fixture(html`<${tag} touched dirty>${inputSlots}`) ); diff --git a/packages/ui/components/form-core/test/lion-field.test.js b/packages/ui/components/form-core/test/lion-field.test.js index 195d3c7874..8fda5d036f 100644 --- a/packages/ui/components/form-core/test/lion-field.test.js +++ b/packages/ui/components/form-core/test/lion-field.test.js @@ -357,40 +357,32 @@ describe('', () => { expect(el.validationStates.error.Required).to.not.exist; }); - it('will only update formattedValue when valid on `user-input-changed`', async () => { + it('for best DX, on `user-input-changed` (during typing/interacting): will only update formattedValue when no errors', async () => { const formatterSpy = sinon.spy(value => `foo: ${value}`); - const Bar = class extends Validator { - static get validatorName() { - return 'Bar'; - } + const EqualsBar = class extends Validator { + static validatorName = 'EqualsBar'; - /** - * @param {string} value - */ - execute(value) { - const hasError = value !== 'bar'; - return hasError; - } + execute = (/** @type {string} */ value) => value !== 'bar'; }; const el = /** @type {LionField} */ ( await fixture(html` <${tag} .modelValue=${'init-string'} .formatter=${formatterSpy} - .validators=${[new Bar()]} + .validators=${[new EqualsBar()]} >${inputSlot} `) ); - expect(formatterSpy.callCount).to.equal(0); - expect(el.formattedValue).to.equal('init-string'); + expect(formatterSpy.callCount).to.equal(1); + expect(el.formattedValue).to.equal('foo: init-string'); el.modelValue = 'bar'; - expect(formatterSpy.callCount).to.equal(1); + expect(formatterSpy.callCount).to.equal(2); expect(el.formattedValue).to.equal('foo: bar'); mimicUserInput(el, 'foo'); - expect(formatterSpy.callCount).to.equal(1); + expect(formatterSpy.callCount).to.equal(2); expect(el.value).to.equal('foo'); }); }); diff --git a/packages/ui/components/input-date/src/LionInputDate.js b/packages/ui/components/input-date/src/LionInputDate.js index 7ef8edb851..18047dd14e 100644 --- a/packages/ui/components/input-date/src/LionInputDate.js +++ b/packages/ui/components/input-date/src/LionInputDate.js @@ -2,6 +2,32 @@ import { IsDate } from '@lion/ui/form-core.js'; import { LionInput } from '@lion/ui/input.js'; import { formatDate, LocalizeMixin, parseDate } from '@lion/ui/localize-no-side-effects.js'; +/** + * + * @param {Date | undefined} newV + * @param {Date | undefined} oldV + */ +function hasDateChanged(newV, oldV) { + return !( + ( + Object.prototype.toString.call(newV) === '[object Date]' && // is Date Object + newV && + newV.getDate() === oldV?.getDate?.() && // day + newV.getMonth() === oldV.getMonth?.() && // month + newV.getFullYear() === oldV.getFullYear?.() + ) // year + ); +} + +/** + * Change function that returns true if `value` is different from `oldValue`. + * This method is used as the default for a property's `hasChanged` function. + */ +export const notEqual = (value, old) => { + // This ensures (old==NaN, value==NaN) always returns false + return old !== value && (old === old || value === value); +}; + /** * @param {Date|number} date */ @@ -22,7 +48,10 @@ export class LionInputDate extends LocalizeMixin(LionInput) { /** @type {any} */ static get properties() { return { - modelValue: Date, + modelValue: { + type: Date, + hasChanged: hasDateChanged, + }, }; } diff --git a/packages/ui/components/input-date/test/lion-input-date.test.js b/packages/ui/components/input-date/test/lion-input-date.test.js index 0a4aeff721..e7260067a7 100644 --- a/packages/ui/components/input-date/test/lion-input-date.test.js +++ b/packages/ui/components/input-date/test/lion-input-date.test.js @@ -67,7 +67,33 @@ describe('', () => { expect(el.validationStates.error).not.to.have.property('MaxDate'); }); - describe('locale', () => { + it('serializes to iso format', async () => { + const el = await fixture(html` + + `); + expect(el.serializedValue).to.equal('2000-12-15'); + }); + + // So new Date('2000/12/30') equals new Date('2000/12/30') + it.only('performs hasChanged check based on "equalsDate" check', async () => { + let callCount = 0; + const el = await fixture(html` + + `); + callCount = 0; + el.modelValue = new Date('2000/12/30'); + + console.log(Array.from(el.constructor.elementProperties)); + + expect(callCount).to.equal(0); + }); + + describe('Localization', () => { it('uses formatOptions.locale', async () => { const el = await fixture(html` ', () => { await el.updateComplete; expect(el.formattedValue).to.equal('15/06/2017'); // should stay british }); + + describe('Timezones', async () => { + const dateAmsterdam = new Date( + new Date('2017/06/15').toLocaleString('en-US', { timeZone: 'Europe/Amsterdam' }), + ); + const dateManila = new Date( + new Date('2017/06/15').toLocaleString('en-US', { timeZone: 'Asia/Manila' }), + ); + const dateNewYork = new Date( + new Date('2017/06/15').toLocaleString('en-US', { timeZone: 'America/New_York' }), + ); + + it('works with different timezones', async () => { + const el = await fixture(html` + + `); + expect(el.formattedValue).to.equal('15/06/2017', 'Europe/Amsterdam'); + + el.modelValue = dateManila; + expect(el.formattedValue).to.equal('15/06/2017', 'Asia/Manila'); + + el.modelValue = dateNewYork; + expect(el.formattedValue).to.equal('14/06/2017', 'America/New_York'); + }); + + it('validators work with different timezones', async () => { + const el = await fixture(html` + + `); + expect(el.formattedValue).to.equal('15/06/2017', 'Europe/Amsterdam'); + expect(el.hasFeedbackFor).not.to.include('error', 'Europe/Amsterdam'); + + el.modelValue = dateManila; + expect(el.formattedValue).to.equal('15/06/2017', 'Asia/Manila'); + expect(el.hasFeedbackFor).not.to.include('error', 'Asia/Manila'); + + el.modelValue = dateNewYork; + expect(el.formattedValue).to.equal('14/06/2017', 'America/New_York'); + expect(el.hasFeedbackFor).not.to.include('error', 'America/New_York'); + }); + }); }); - describe('timezones', async () => { - const dateAmsterdam = new Date( - new Date('2017/06/15').toLocaleString('en-US', { timeZone: 'Europe/Amsterdam' }), - ); - const dateManila = new Date( - new Date('2017/06/15').toLocaleString('en-US', { timeZone: 'Asia/Manila' }), - ); - const dateNewYork = new Date( - new Date('2017/06/15').toLocaleString('en-US', { timeZone: 'America/New_York' }), - ); - - it('works with different timezones', async () => { + describe('Accessibility', () => { + it('is accessible', async () => { const el = await fixture(html` - + `); - expect(el.formattedValue).to.equal('15/06/2017', 'Europe/Amsterdam'); - - el.modelValue = dateManila; - expect(el.formattedValue).to.equal('15/06/2017', 'Asia/Manila'); - - el.modelValue = dateNewYork; - expect(el.formattedValue).to.equal('14/06/2017', 'America/New_York'); + await expect(el).to.be.accessible(); }); - it('validators work with different timezones', async () => { + it('is accessible when readonly', async () => { const el = await fixture(html` - + `); - expect(el.formattedValue).to.equal('15/06/2017', 'Europe/Amsterdam'); - expect(el.hasFeedbackFor).not.to.include('error', 'Europe/Amsterdam'); - - el.modelValue = dateManila; - expect(el.formattedValue).to.equal('15/06/2017', 'Asia/Manila'); - expect(el.hasFeedbackFor).not.to.include('error', 'Asia/Manila'); - - el.modelValue = dateNewYork; - expect(el.formattedValue).to.equal('14/06/2017', 'America/New_York'); - expect(el.hasFeedbackFor).not.to.include('error', 'America/New_York'); + await expect(el).to.be.accessible(); }); - }); - - it('is accessible', async () => { - const el = await fixture(html` - - `); - await expect(el).to.be.accessible(); - }); - - it('is accessible when readonly', async () => { - const el = await fixture(html` - - `); - await expect(el).to.be.accessible(); - }); - - it('is accessible when disabled', async () => { - const el = await fixture(html` - - `); - await expect(el).to.be.accessible(); - }); - it('serializes to iso format', async () => { - const el = await fixture(html` - - `); - expect(el.serializedValue).to.equal('2000-12-15'); + it('is accessible when disabled', async () => { + const el = await fixture(html` + + `); + await expect(el).to.be.accessible(); + }); }); });