From 0c711f69628d4a56ef61d95c9bb67278f532eed9 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Wed, 24 Mar 2021 16:06:46 +0100 Subject: [PATCH 1/3] fix(form-core): delay rejectRegistrationComplete 1 microtask Co-authored-by: palash2601 --- .../systems/overlays/assets/umbrella-form.js | 126 ++++++++++++++++++ .../src/choice-group/ChoiceGroupMixin.js | 6 +- .../src/form-group/FormGroupMixin.js | 4 +- .../test/dialog-integrations.js | 20 +++ 4 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 docs/docs/systems/overlays/assets/umbrella-form.js create mode 100644 packages/form-integrations/test/dialog-integrations.js diff --git a/docs/docs/systems/overlays/assets/umbrella-form.js b/docs/docs/systems/overlays/assets/umbrella-form.js new file mode 100644 index 000000000..578575026 --- /dev/null +++ b/docs/docs/systems/overlays/assets/umbrella-form.js @@ -0,0 +1,126 @@ +import { LitElement, html } from '@lion/core'; +import { Required, MinLength } from '@lion/form-core'; +import '@lion/form/define'; +import '@lion/fieldset/define'; +import '@lion/input/define'; +import '@lion/input-date/define'; +import '@lion/input-datepicker/define'; +import '@lion/input-amount/define'; +import '@lion/input-iban/define'; +import '@lion/input-email/define'; +import '@lion/checkbox-group/define'; +import '@lion/radio-group/define'; +import '@lion/select/define'; +import '@lion/select-rich/define'; +import '@lion/input-range/define'; +import '@lion/textarea/define'; +import '@lion/button/define'; + +export class UmbrellaForm extends LitElement { + get _lionFormNode() { + return /** @type {import('@lion/form').LionForm} */ (this.shadowRoot?.querySelector( + 'lion-form', + )); + } + + render() { + return html` + +
+ + + + + + + + + + + + + + + + + + + + + + + Red + Hotpink + Teal + + + + + + + + + + + +
+ Submit + { + const lionForm = this._lionFormNode; + lionForm.resetGroup(); + }} + >Reset +
+
+ + `; + } +} +customElements.define('umbrella-form', UmbrellaForm); diff --git a/packages/form-core/src/choice-group/ChoiceGroupMixin.js b/packages/form-core/src/choice-group/ChoiceGroupMixin.js index 3595f5b68..7e62e44eb 100644 --- a/packages/form-core/src/choice-group/ChoiceGroupMixin.js +++ b/packages/form-core/src/choice-group/ChoiceGroupMixin.js @@ -181,7 +181,11 @@ const ChoiceGroupMixinImplementation = superclass => super.disconnectedCallback(); if (this.registrationComplete.done === false) { - this.__rejectRegistrationComplete(); + Promise.resolve().then(() => { + Promise.resolve().then(() => { + this.__rejectRegistrationComplete(); + }); + }); } } diff --git a/packages/form-core/src/form-group/FormGroupMixin.js b/packages/form-core/src/form-group/FormGroupMixin.js index eed4dea46..2190d6ca8 100644 --- a/packages/form-core/src/form-group/FormGroupMixin.js +++ b/packages/form-core/src/form-group/FormGroupMixin.js @@ -184,7 +184,9 @@ const FormGroupMixinImplementation = superclass => this.__hasActiveOutsideClickHandling = false; } if (this.registrationComplete.done === false) { - this.__rejectRegistrationComplete(); + Promise.resolve().then(() => { + this.__rejectRegistrationComplete(); + }); } } diff --git a/packages/form-integrations/test/dialog-integrations.js b/packages/form-integrations/test/dialog-integrations.js new file mode 100644 index 000000000..e661f544b --- /dev/null +++ b/packages/form-integrations/test/dialog-integrations.js @@ -0,0 +1,20 @@ +import { expect, fixture, html } from '@open-wc/testing'; +import './helpers/umbrella-form.js'; +import '@lion/dialog/lion-dialog.js'; + +/** + * @typedef {import('./helpers/umbrella-form.js').UmbrellaForm} UmbrellaForm + * @typedef {import('@lion/dialog/').LionDialog} LionDialog + */ + +// Test umbrella form inside dialog +describe('Form inside dialog Integrations', () => { + it('"Successfully spawns all form components inside a dialog', async () => { + expect( + await fixture(html` + + + `), + ).to.not.throw(); + }); +}); From 72a6ccc81e5421fbfae903ec465b778758f91434 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Wed, 24 Mar 2021 16:42:37 +0100 Subject: [PATCH 2/3] feat(combobox): allow to configure textbox sync on close --- .changeset/cool-peaches-burn.md | 11 ++++++ docs/components/inputs/combobox/features.md | 5 ++- packages/combobox/src/LionCombobox.js | 14 +++++-- packages/combobox/test/lion-combobox.test.js | 40 ++++++++++++++++++++ 4 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 .changeset/cool-peaches-burn.md diff --git a/.changeset/cool-peaches-burn.md b/.changeset/cool-peaches-burn.md new file mode 100644 index 000000000..6bc06d2aa --- /dev/null +++ b/.changeset/cool-peaches-burn.md @@ -0,0 +1,11 @@ +--- +'@lion/combobox': minor +'@lion/form-core': patch +'@lion/form-integrations': patch +--- + +Allow Subclassers of LionCombobox to set '\_syncToTextboxCondition' in closing phase of overlay + +## Fixes + +Allow an extra microtask in registration phase to make forms inside dialogs compatible. diff --git a/docs/components/inputs/combobox/features.md b/docs/components/inputs/combobox/features.md index 801d06bef..5a8cdd9a5 100644 --- a/docs/components/inputs/combobox/features.md +++ b/docs/components/inputs/combobox/features.md @@ -215,7 +215,10 @@ This will: ```js preview-story export const multipleChoice = () => html` - + ${lazyRender( listboxData.map( (entry, i) => diff --git a/packages/combobox/src/LionCombobox.js b/packages/combobox/src/LionCombobox.js index 0d4b394e7..b377bbf76 100644 --- a/packages/combobox/src/LionCombobox.js +++ b/packages/combobox/src/LionCombobox.js @@ -440,7 +440,12 @@ export class LionCombobox extends OverlayMixin(LionListbox) { __onOverlayClose() { if (!this.multipleChoice) { - if (this.checkedIndex !== -1) { + if ( + this.checkedIndex !== -1 && + this._syncToTextboxCondition(this.modelValue, this.__oldModelValue, { + phase: 'overlay-close', + }) + ) { this._inputNode.value = this.formElements[ /** @type {number} */ (this.checkedIndex) ].choiceValue; @@ -739,12 +744,15 @@ export class LionCombobox extends OverlayMixin(LionListbox) { } /** + * @overridable * @param {string|string[]} modelValue * @param {string|string[]} oldModelValue */ // eslint-disable-next-line no-unused-vars - _syncToTextboxCondition(modelValue, oldModelValue) { - return this.autocomplete === 'inline' || this.autocomplete === 'both'; + _syncToTextboxCondition(modelValue, oldModelValue, { phase } = {}) { + return ( + this.autocomplete === 'inline' || this.autocomplete === 'both' || phase === 'overlay-close' + ); } /** diff --git a/packages/combobox/test/lion-combobox.test.js b/packages/combobox/test/lion-combobox.test.js index f95242c75..4870f59b7 100644 --- a/packages/combobox/test/lion-combobox.test.js +++ b/packages/combobox/test/lion-combobox.test.js @@ -936,6 +936,46 @@ describe('lion-combobox', () => { await performChecks('both', [0, 1], ''); }); + it('is possible to adjust textbox synchronize condition on overlay close', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); + expect(el._inputNode.value).to.equal(''); + + /** + * @param {'none' | 'list' | 'inline' | 'both'} autocomplete + * @param {number|number[]} index + * @param {string} valueOnClose + */ + async function performChecks(autocomplete, index, valueOnClose) { + await el.updateComplete; + el.opened = true; + el.setCheckedIndex(-1); + await el.updateComplete; + el.autocomplete = autocomplete; + el.setCheckedIndex(index); + el.opened = false; + await el.updateComplete; + expect(el._inputNode.value).to.equal(valueOnClose); + } + + await performChecks('none', 0, ''); + await performChecks('list', 0, ''); + await performChecks('inline', 0, ''); + await performChecks('both', 0, ''); + + el.multipleChoice = true; + await performChecks('none', [0, 1], ''); + await performChecks('list', [0, 1], ''); + await performChecks('inline', [0, 1], ''); + await performChecks('both', [0, 1], ''); + }); + it('does inline autocompletion when adding chars', async () => { const el = /** @type {LionCombobox} */ (await fixture(html` From b7743d8f22319686b5c851c5ea268e6ce537e8ce Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Wed, 24 Mar 2021 17:22:24 +0100 Subject: [PATCH 3/3] fix(combobox): open on focused when showAllOnEmpty --- .changeset/cool-peaches-burn.md | 3 ++- packages/combobox/src/LionCombobox.js | 3 +++ packages/combobox/test/lion-combobox.test.js | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/.changeset/cool-peaches-burn.md b/.changeset/cool-peaches-burn.md index 6bc06d2aa..bed822eab 100644 --- a/.changeset/cool-peaches-burn.md +++ b/.changeset/cool-peaches-burn.md @@ -8,4 +8,5 @@ Allow Subclassers of LionCombobox to set '\_syncToTextboxCondition' in closing p ## Fixes -Allow an extra microtask in registration phase to make forms inside dialogs compatible. +- form-core: allow an extra microtask in registration phase to make forms inside dialogs compatible. +- combobox: open on focused when showAllOnEmpty diff --git a/packages/combobox/src/LionCombobox.js b/packages/combobox/src/LionCombobox.js index b377bbf76..3de26b7af 100644 --- a/packages/combobox/src/LionCombobox.js +++ b/packages/combobox/src/LionCombobox.js @@ -390,6 +390,9 @@ export class LionCombobox extends OverlayMixin(LionListbox) { */ // eslint-disable-next-line class-methods-use-this _showOverlayCondition({ lastKey }) { + if (this.showAllOnEmpty && this.focused) { + return true; + } // when no keyboard action involved (on focused change), return current opened state if (!lastKey) { return this.opened; diff --git a/packages/combobox/test/lion-combobox.test.js b/packages/combobox/test/lion-combobox.test.js index 4870f59b7..4c3131c41 100644 --- a/packages/combobox/test/lion-combobox.test.js +++ b/packages/combobox/test/lion-combobox.test.js @@ -177,6 +177,22 @@ describe('lion-combobox', () => { el.autocomplete = 'both'; await performChecks(); }); + + it('shows overlay on focusin', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); + + expect(el.opened).to.be.false; + el._comboboxNode.dispatchEvent(new Event('focusin', { bubbles: true, composed: true })); + await el.updateComplete; + expect(el.opened).to.be.true; + }); }); });