Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[draft] incorporate hasChanged in sync updates #1914

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 39 additions & 16 deletions packages/ui/components/form-core/src/FormatMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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' });
}
}
Expand All @@ -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 */
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -416,13 +437,15 @@ 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 }));
}

/** @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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}</${tag}>`)
);
Expand Down
26 changes: 9 additions & 17 deletions packages/ui/components/form-core/test/lion-field.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -357,40 +357,32 @@ describe('<lion-field>', () => {
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}</${tag}>
`)
);

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');
});
});
Expand Down
31 changes: 30 additions & 1 deletion packages/ui/components/input-date/src/LionInputDate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// This ensures (old==NaN, value==NaN) always returns false
return old !== value && (old === old || value === value);
};

/**
* @param {Date|number} date
*/
Expand All @@ -22,7 +48,10 @@ export class LionInputDate extends LocalizeMixin(LionInput) {
/** @type {any} */
static get properties() {
return {
modelValue: Date,
modelValue: {
type: Date,
hasChanged: hasDateChanged,
},
};
}

Expand Down
151 changes: 86 additions & 65 deletions packages/ui/components/input-date/test/lion-input-date.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,33 @@ describe('<lion-input-date>', () => {
expect(el.validationStates.error).not.to.have.property('MaxDate');
});

describe('locale', () => {
it('serializes to iso format', async () => {
const el = await fixture(html`
<lion-input-date .modelValue="${new Date('2000/12/15')}"></lion-input-date>
`);
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`
<lion-input-date
.modelValue="${new Date('2000/12/30')}"
@model-value-changed="${() => {
callCount += 1;
}}"
></lion-input-date>
`);
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`
<lion-input-date
Expand Down Expand Up @@ -112,79 +138,74 @@ describe('<lion-input-date>', () => {
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`
<lion-input-date .modelValue=${dateAmsterdam}></lion-input-date>
`);
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`
<lion-input-date
.modelValue=${new Date('2017/06/15')}
.validators=${[new MinDate(new Date('2017/06/14'))]}
></lion-input-date>
`);
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`
<lion-input-date .modelValue=${dateAmsterdam}></lion-input-date>
<lion-input-date><label slot="label">Label</label></lion-input-date>
`);
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`
<lion-input-date
.modelValue=${new Date('2017/06/15')}
.validators=${[new MinDate(new Date('2017/06/14'))]}
></lion-input-date>
<lion-input-date readonly .modelValue=${new Date('2017/06/15')}
><label slot="label">Label</label></lion-input-date
>
`);
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`
<lion-input-date><label slot="label">Label</label></lion-input-date>
`);
await expect(el).to.be.accessible();
});

it('is accessible when readonly', async () => {
const el = await fixture(html`
<lion-input-date readonly .modelValue=${new Date('2017/06/15')}
><label slot="label">Label</label></lion-input-date
>
`);
await expect(el).to.be.accessible();
});

it('is accessible when disabled', async () => {
const el = await fixture(html`
<lion-input-date disabled><label slot="label">Label</label></lion-input-date>
`);
await expect(el).to.be.accessible();
});

it('serializes to iso format', async () => {
const el = await fixture(html`
<lion-input-date .modelValue="${new Date('2000/12/15')}"></lion-input-date>
`);
expect(el.serializedValue).to.equal('2000-12-15');
it('is accessible when disabled', async () => {
const el = await fixture(html`
<lion-input-date disabled><label slot="label">Label</label></lion-input-date>
`);
await expect(el).to.be.accessible();
});
});
});