diff --git a/.changeset/dirty-emus-exercise.md b/.changeset/dirty-emus-exercise.md new file mode 100644 index 0000000000..cebb3371b1 --- /dev/null +++ b/.changeset/dirty-emus-exercise.md @@ -0,0 +1,5 @@ +--- +'@lion/ui': minor +--- + +Fix: fixes single-choice, requireOptionMatch=false to not clear selection diff --git a/.changeset/ten-maps-tap.md b/.changeset/ten-maps-tap.md new file mode 100644 index 0000000000..28684b17dc --- /dev/null +++ b/.changeset/ten-maps-tap.md @@ -0,0 +1,6 @@ +--- +'publish-docs': patch +'@lion/ui': patch +--- + +feature: Added support to multiselect and require option=false at the same time for lion-combobox diff --git a/docs/components/combobox/src/demo-selection-display.js b/docs/components/combobox/src/demo-selection-display.js index 0b16a9feda..e97108c1e6 100644 --- a/docs/components/combobox/src/demo-selection-display.js +++ b/docs/components/combobox/src/demo-selection-display.js @@ -25,7 +25,7 @@ export class DemoSelectionDisplay extends LitElement { * Can be used to visually indicate the next */ removeChipOnNextBackspace: Boolean, - selectedElements: Array, + selectedChoices: Array, }; } @@ -72,12 +72,6 @@ export class DemoSelectionDisplay extends LitElement { return this.comboboxElement._inputNode; } - _computeSelectedElements() { - const { formElements, checkedIndex } = /** @type {LionCombobox} */ (this.comboboxElement); - const checkedIndexes = Array.isArray(checkedIndex) ? checkedIndex : [checkedIndex]; - return formElements.filter((_, i) => checkedIndexes.includes(i)); - } - get multipleChoice() { return this.comboboxElement?.multipleChoice; } @@ -85,7 +79,7 @@ export class DemoSelectionDisplay extends LitElement { constructor() { super(); - this.selectedElements = []; + this.selectedChoices = []; /** @type {EventListener} */ this.__textboxOnKeyup = this.__textboxOnKeyup.bind(this); @@ -110,29 +104,8 @@ export class DemoSelectionDisplay extends LitElement { */ onComboboxElementUpdated(changedProperties) { if (changedProperties.has('modelValue')) { - this.selectedElements = this._computeSelectedElements(); - } - } - - /** - * Whenever selectedElements are updated, makes sure that latest added elements - * are shown latest, and deleted elements respect existing order of chips. - */ - __reorderChips() { - const { selectedElements } = this; - if (this.__prevSelectedEls) { - const addedEls = selectedElements.filter(e => !this.__prevSelectedEls.includes(e)); - const deletedEls = this.__prevSelectedEls.filter(e => !selectedElements.includes(e)); - if (addedEls.length) { - this.selectedElements = [...this.__prevSelectedEls, ...addedEls]; - } else if (deletedEls.length) { - deletedEls.forEach(delEl => { - this.__prevSelectedEls.splice(this.__prevSelectedEls.indexOf(delEl), 1); - }); - this.selectedElements = this.__prevSelectedEls; - } + this.selectedChoices = this.comboboxElement.modelValue; } - this.__prevSelectedEls = this.selectedElements; } /** @@ -143,7 +116,7 @@ export class DemoSelectionDisplay extends LitElement { _selectedElementTemplate(option, highlight) { return html` - ${option.value} + ${option} `; } @@ -154,9 +127,9 @@ export class DemoSelectionDisplay extends LitElement { } return html`
- ${this.selectedElements.map((option, i) => { + ${this.selectedChoices.map((option, i) => { const highlight = Boolean( - this.removeChipOnNextBackspace && i === this.selectedElements.length - 1, + this.removeChipOnNextBackspace && i === this.selectedChoices.length - 1, ); return this._selectedElementTemplate(option, highlight); })} @@ -174,8 +147,8 @@ export class DemoSelectionDisplay extends LitElement { __textboxOnKeyup(ev) { if (ev.key === 'Backspace') { if (!this._inputNode.value) { - if (this.removeChipOnNextBackspace && this.selectedElements.length) { - this.selectedElements[this.selectedElements.length - 1].checked = false; + if (this.removeChipOnNextBackspace && this.selectedChoices.length) { + this.comboboxElement.modelValue = this.selectedChoices.slice(0, -1); } this.removeChipOnNextBackspace = true; } diff --git a/docs/components/combobox/use-cases.md b/docs/components/combobox/use-cases.md index abda92a4ad..6fb52e918d 100644 --- a/docs/components/combobox/use-cases.md +++ b/docs/components/combobox/use-cases.md @@ -231,6 +231,7 @@ This will: > Please note that the lion-combobox-selection-display below is not exposed and only serves > as an example. The selection part of a multiselect combobox is not yet accessible. Please keep > in mind that for now, as a Subclasser, you would have to take care of this part yourself. +> Also keep in mind that the combobox organizes the selected list by its original index in the option list ```js preview-story export const multipleChoice = () => html` @@ -249,6 +250,28 @@ export const multipleChoice = () => html` `; ``` +Alternatively, the multi-choice flag can be combined with .requireMultipleMatch=false to allow users to enter their own options. + +> Note that the non-matching items will be displayed in the end of the list in the order that were entered. Since those have no index +> in the option list, they don't have a representing value in the checkedIndex property. + +```js preview-story +export const multipleCustomizableChoice = () => html` + + + ${lazyRender( + listboxData.map( + (entry, i) => + html` ${entry} `, + ), + )} + +`; +``` + ## Validation The combobox works with a `Required` validator to check if it is empty. diff --git a/packages/ui/components/combobox/src/LionCombobox.js b/packages/ui/components/combobox/src/LionCombobox.js index 056ed48ba9..483a2aab30 100644 --- a/packages/ui/components/combobox/src/LionCombobox.js +++ b/packages/ui/components/combobox/src/LionCombobox.js @@ -6,6 +6,7 @@ import { OverlayMixin, withDropdownConfig } from '@lion/ui/overlays.js'; import { css, html } from 'lit'; import { makeMatchingTextBold, unmakeMatchingTextBold } from './utils/makeMatchingTextBold.js'; import { MatchesOption } from './validators.js'; +import { CustomChoiceGroupMixin } from '../../form-core/src/choice-group/CustomChoiceGroupMixin.js'; const matchA11ySpanReverseFns = new WeakMap(); @@ -27,7 +28,7 @@ const matchA11ySpanReverseFns = new WeakMap(); * LionCombobox: implements the wai-aria combobox design pattern and integrates it as a Lion * FormControl */ -export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) { +export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMixin(LionListbox))) { /** @type {any} */ static get properties() { return { @@ -43,6 +44,10 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) { requireOptionMatch: { type: Boolean, }, + allowCustomChoice: { + type: Boolean, + attribute: 'allow-custom-choice', + }, __shouldAutocompleteNextUpdate: Boolean, }; } @@ -316,7 +321,9 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) { */ get _inputNode() { if (this._ariaVersion === '1.1' && this._comboboxNode) { - return /** @type {HTMLInputElement} */ (this._comboboxNode.querySelector('input')); + return /** @type {HTMLInputElement} */ ( + this._comboboxNode.querySelector('input') || this._comboboxNode + ); } return /** @type {HTMLInputElement} */ (this._comboboxNode); } @@ -364,6 +371,20 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) { return this._inputNode; } + /** + * @returns {boolean} + */ + get requireOptionMatch() { + return !this.allowCustomChoice; + } + + /** + * @param {boolean} value + */ + set requireOptionMatch(value) { + this.allowCustomChoice = !value; + } + constructor() { super(); /** @@ -486,14 +507,20 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) { /** * Converts viewValue to modelValue - * @param {string} value - viewValue: the formatted value inside + * @override CustomChoiceGroupMixin + * @param {string|string[]} value - viewValue: the formatted value inside * @returns {*} modelValue */ parser(value) { - if (this.requireOptionMatch && this.checkedIndex === -1 && value !== '') { + if ( + this.requireOptionMatch && + this.checkedIndex === -1 && + value !== '' && + !Array.isArray(value) + ) { return new Unparseable(value); } - return value; + return super.parser(value); } /** @@ -554,15 +581,6 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) { if (typeof this._selectionDisplayNode?.onComboboxElementUpdated === 'function') { this._selectionDisplayNode.onComboboxElementUpdated(changedProperties); } - - if (changedProperties.has('requireOptionMatch') || changedProperties.has('multipleChoice')) { - if (!this.requireOptionMatch && this.multipleChoice) { - // TODO implement !requireOptionMatch and multipleChoice flow - throw new Error( - "multipleChoice and requireOptionMatch=false can't be used at the same time (yet).", - ); - } - } } /** @@ -697,8 +715,8 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) { * @protected */ _setTextboxValue(v) { - // Make sure that we don't loose inputNode.selectionStart and inputNode.selectionEnd - if (this._inputNode.value !== v) { + // Make sure that we don't lose inputNode.selectionStart and inputNode.selectionEnd + if (this._inputNode && this._inputNode.value !== v) { this._inputNode.value = v; } } @@ -1065,25 +1083,52 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) { * @protected */ _listboxOnKeyDown(ev) { - super._listboxOnKeyDown(ev); const { key } = ev; switch (key) { case 'Escape': this.opened = false; + super._listboxOnKeyDown(ev); this._setTextboxValue(''); break; + case 'Backspace': + case 'Delete': + if (this.requireOptionMatch) { + super._listboxOnKeyDown(ev); + } else { + this.opened = false; + } + break; case 'Enter': if (this.multipleChoice && this.opened) { ev.preventDefault(); } - if (!this.formElements[this.activeIndex]) { - return; + + if ( + !this.requireOptionMatch && + this.multipleChoice && + (!this.formElements[this.activeIndex] || + this.formElements[this.activeIndex].hasAttribute('aria-hidden') || + !this.opened) + ) { + ev.preventDefault(); + + this.modelValue = this.parser([...this.modelValue, this._inputNode.value]); + + this._inputNode.value = ''; + this.opened = false; + } else { + super._listboxOnKeyDown(ev); + // TODO: should we clear the input value here when allowCustomChoice is false? + // For now, we don't... } if (!this.multipleChoice) { this.opened = false; } break; - /* no default */ + default: { + super._listboxOnKeyDown(ev); + break; + } } } @@ -1110,12 +1155,14 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) { */ // eslint-disable-next-line no-unused-vars _syncToTextboxMultiple(modelValue, oldModelValue = []) { - const diff = modelValue.filter(x => !oldModelValue.includes(x)); - const newValue = this.formElements - .filter(option => diff.includes(option.choiceValue)) - .map(option => this._getTextboxValueFromOption(option)) - .join(' '); - this._setTextboxValue(newValue); // or last selected value? + if (this.requireOptionMatch) { + const diff = modelValue.filter(x => !oldModelValue.includes(x)); + const newValue = this.formElements + .filter(option => diff.includes(option.choiceValue)) + .map(option => this._getTextboxValueFromOption(option)) + .join(' '); + this._setTextboxValue(newValue); // or last selected value? + } } /** diff --git a/packages/ui/components/combobox/test-helpers/combobox-helpers.js b/packages/ui/components/combobox/test-helpers/combobox-helpers.js index b4594c5b12..845f758112 100644 --- a/packages/ui/components/combobox/test-helpers/combobox-helpers.js +++ b/packages/ui/components/combobox/test-helpers/combobox-helpers.js @@ -72,15 +72,18 @@ export async function mimicUserTypingAdvanced(el, values) { const selectionEnd = _inputNode.selectionEnd || 0; const hasSelection = selectionStart !== selectionEnd; - if (key === 'Backspace') { + if (key === 'Backspace' || key === 'Delete') { if (hasSelection) { _inputNode.value = _inputNode.value.slice(0, selectionStart) + _inputNode.value.slice(selectionEnd); cursorPosition = selectionStart; - } else if (cursorPosition > 0) { + } else if (cursorPosition > 0 && key === 'Backspace') { _inputNode.value = _inputNode.value.slice(0, cursorPosition - 1) + _inputNode.value.slice(cursorPosition); cursorPosition -= 1; + } else if (cursorPosition < _inputNode.value.length && key === 'Delete') { + _inputNode.value = + _inputNode.value.slice(0, cursorPosition) + _inputNode.value.slice(cursorPosition + 1); } } else if (hasSelection) { _inputNode.value = diff --git a/packages/ui/components/combobox/test/lion-combobox-integrations.test.js b/packages/ui/components/combobox/test/lion-combobox-integrations.test.js index 19b850076b..0189a33c5e 100644 --- a/packages/ui/components/combobox/test/lion-combobox-integrations.test.js +++ b/packages/ui/components/combobox/test/lion-combobox-integrations.test.js @@ -1,4 +1,9 @@ import { runListboxMixinSuite } from '@lion/ui/listbox-test-suites.js'; import '@lion/ui/define/lion-combobox.js'; +import { runCustomChoiceGroupMixinSuite } from '../../form-core/test-suites/choice-group/CustomChoiceGroupMixin.suite.js'; runListboxMixinSuite({ tagString: 'lion-combobox' }); +runCustomChoiceGroupMixinSuite({ + parentTagString: 'lion-combobox', + childTagString: 'lion-option', +}); diff --git a/packages/ui/components/combobox/test/lion-combobox.test.js b/packages/ui/components/combobox/test/lion-combobox.test.js index 82ce91c40d..fa3786e2cf 100644 --- a/packages/ui/components/combobox/test/lion-combobox.test.js +++ b/packages/ui/components/combobox/test/lion-combobox.test.js @@ -395,29 +395,6 @@ describe('lion-combobox', () => { expect(el.formElements[0].checked).to.be.false; }); - it('multiple choice and requireOptionMatch is false do not work together yet', async () => { - const errorMessage = `multipleChoice and requireOptionMatch=false can't be used at the same time (yet).`; - let error; - const el = /** @type {LionCombobox} */ ( - await fixture(html` - - Artichoke - Chard - Chicory - Victoria Plum - - `) - ); - try { - el.requireOptionMatch = false; - await el.updateComplete; - } catch (err) { - error = err; - } - expect(error).to.be.instanceOf(Error); - expect(/** @type {Error} */ (error).message).to.equal(errorMessage); - }); - it('clears modelValue and textbox value on clear()', async () => { const el = /** @type {LionCombobox} */ ( await fixture(html` @@ -591,6 +568,33 @@ describe('lion-combobox', () => { expect(el.showsFeedbackFor).to.include('error', 'showsFeedbackFor'); }); + it('ignores empty string modelValue inputs', async () => { + const el = /** @type {LionCombobox} */ ( + await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `) + ); + + el.requireOptionMatch = false; + await el.updateComplete; + const { _inputNode } = getComboboxMembers(el); + + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + expect(el.modelValue).to.eql([]); + + mimicUserTyping(el, ' '); + await el.updateComplete; + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + expect(el.modelValue).to.eql([]); + }); + it('allows a value outside of the option list when requireOptionMatch is false', async () => { const el = /** @type {LionCombobox} */ ( await fixture(html` @@ -615,6 +619,35 @@ describe('lion-combobox', () => { expect(_inputNode.value).to.equal('Foo'); }); + it("doesn't select any similar options after using delete when requireOptionMatch is false", async () => { + const el = /** @type {LionCombobox} */ ( + await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `) + ); + el.requireOptionMatch = false; + const { _inputNode } = getComboboxMembers(el); + + mimicUserTyping(el, 'Art'); + await el.updateComplete; + + await mimicUserTypingAdvanced(el, ['Delete']); + await el.updateComplete; + await el.updateComplete; + + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + + expect(el.checkedIndex).to.equal(-1); + expect(el.modelValue).to.equal('Art'); + expect(_inputNode.value).to.equal('Art'); + }); + it("when removing a letter it won't select the option", async () => { // We don't autocomplete when characters are removed const el = /** @type {LionCombobox} */ ( @@ -667,6 +700,173 @@ describe('lion-combobox', () => { expect(el.modelValue).to.equal('Foo'); expect(el.formElements[0].checked).to.be.false; }); + + it('allows custom selections when multi-choice when requireOptionMatch is false', async () => { + const el = /** @type {LionCombobox} */ ( + await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `) + ); + + el.requireOptionMatch = false; + await el.updateComplete; + + const { _inputNode } = getComboboxMembers(el); + expect(el.modelValue).to.eql(['Chard']); + expect(el.checkedIndex).to.eql([1]); + + mimicUserTyping(el, 'Foo'); + await el.updateComplete; + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + + expect(el.modelValue).to.eql(['Chard', 'Foo']); + expect(el.checkedIndex).to.eql([1]); + + mimicUserTyping(el, 'Bar'); + await el.updateComplete; + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + }); + + it('allows manyu custom selections when multi-choice when requireOptionMatch is false', async () => { + const el = /** @type {LionCombobox} */ ( + await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `) + ); + + el.requireOptionMatch = false; + await el.updateComplete; + + const { _inputNode } = getComboboxMembers(el); + + mimicUserTyping(el, 'Foo'); + await el.updateComplete; + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + + expect(el.modelValue).to.eql(['Foo']); + expect(el.checkedIndex).to.eql([]); + + mimicUserTyping(el, 'Bar'); + await el.updateComplete; + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + + expect(el.modelValue).to.eql(['Foo', 'Bar']); + expect(el.checkedIndex).to.eql([]); + }); + + it('allows new options when multi-choice when requireOptionMatch=false and autocomplete="both", without selecting similar values', async () => { + const el = /** @type {LionCombobox} */ ( + await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `) + ); + + await el.updateComplete; + + const { _inputNode } = getComboboxMembers(el); + + mimicUserTyping(el, 'Artist'); + await el.updateComplete; + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + + expect(el.modelValue).to.eql(['Artist']); + }); + + it('allows new options when multi-choice when requireOptionMatch=false and autocomplete="both", when deleting autocomplete values using Backspace', async () => { + const el = /** @type {LionCombobox} */ ( + await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `) + ); + + await el.updateComplete; + + const { _inputNode } = getComboboxMembers(el); + mimicUserTyping(el, 'Art'); + await el.updateComplete; + await mimicUserTypingAdvanced(el, ['Backspace']); + await el.updateComplete; + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + + expect(el.modelValue).to.eql(['Art']); + }); + + it('allows new custom options when multi-choice when requireOptionMatch=false and autocomplete="both", when deleting autocompleted values using Delete', async () => { + const el = /** @type {LionCombobox} */ ( + await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `) + ); + + await el.updateComplete; + + const { _inputNode } = getComboboxMembers(el); + el.modelValue = []; + + mimicUserTyping(el, 'Art'); + await el.updateComplete; + await mimicUserTypingAdvanced(el, ['Delete']); + await el.updateComplete; + mimicKeyPress(_inputNode, 'Enter'); + await el.updateComplete; + expect(el.modelValue).to.eql(['Art']); + }); }); describe('Overlay visibility', () => { diff --git a/packages/ui/components/form-core/src/choice-group/ChoiceGroupMixin.js b/packages/ui/components/form-core/src/choice-group/ChoiceGroupMixin.js index 23e1060248..d9416b93d7 100644 --- a/packages/ui/components/form-core/src/choice-group/ChoiceGroupMixin.js +++ b/packages/ui/components/form-core/src/choice-group/ChoiceGroupMixin.js @@ -169,6 +169,7 @@ const ChoiceGroupMixinImplementation = superclass => /** @param {import('lit').PropertyValues} changedProperties */ updated(changedProperties) { super.updated(changedProperties); + if (changedProperties.has('name') && this.name !== changedProperties.get('name')) { this.formElements.forEach(child => { // eslint-disable-next-line no-param-reassign diff --git a/packages/ui/components/form-core/src/choice-group/CustomChoiceGroupMixin.js b/packages/ui/components/form-core/src/choice-group/CustomChoiceGroupMixin.js new file mode 100644 index 0000000000..6ab8ac4f51 --- /dev/null +++ b/packages/ui/components/form-core/src/choice-group/CustomChoiceGroupMixin.js @@ -0,0 +1,173 @@ +import { dedupeMixin } from '@open-wc/dedupe-mixin'; +import { ChoiceGroupMixin } from './ChoiceGroupMixin.js'; + +/** + * @typedef {import('../../types/choice-group/CustomChoiceGroupMixinTypes.js').CustomChoiceGroupMixin} CustomChoiceGroupMixin + * @typedef {import('../../types/choice-group/CustomChoiceGroupMixinTypes.js').CustomChoiceGroupHost} CustomChoiceGroupHost + */ + +/** + * @param {any|any[]} value + * @returns {any[]} + */ +function ensureArray(value) { + return Array.isArray(value) ? value : [value]; +} + +/** + * Extends the ChoiceGroupMixin to add optional support for custom user choices without altering the initial choice list. + * + * @type {CustomChoiceGroupMixin} + * @param {import('@open-wc/dedupe-mixin').Constructor} superclass + */ +const CustomChoiceGroupMixinImplementation = superclass => + // @ts-ignore https://github.com/microsoft/TypeScript/issues/36821#issuecomment-588375051 + class CustomChoiceGroupMixin extends ChoiceGroupMixin(superclass) { + static get properties() { + return { + allowCustomChoice: { + type: Boolean, + attribute: 'allow-custom-choice', + }, + modelValue: { type: Object }, + }; + } + + // @ts-ignore + get modelValue() { + return this.__getChoicesFrom(super.modelValue); + } + + set modelValue(value) { + super.modelValue = value; + + if (value === null || value === undefined || value === '') { + // @ts-ignore + this._customChoices = new Set(); + } else if (this.allowCustomChoice) { + const old = this.modelValue; + // @ts-ignore + this._customChoices = new Set(ensureArray(value)); + this.requestUpdate('modelValue', old); + } + } + + // @ts-ignore + get formattedValue() { + return this.__getChoicesFrom(super.formattedValue); + } + + set formattedValue(value) { + super.formattedValue = value; + + if (value === null || value === undefined) { + this._customChoices = new Set(); + } else if (this.allowCustomChoice) { + const old = this.modelValue; + // Convert formattedValue to modelValue to store as custom choices, or fall back to the input value + this._customChoices = new Set( + ensureArray(value).map( + val => this.formElements.find(el => el.formattedValue === val)?.modelValue || val, + ), + ); + this.requestUpdate('modelValue', old); + } + } + + // @ts-ignore + get serializedValue() { + return this.__getChoicesFrom(super.serializedValue); + } + + set serializedValue(value) { + super.serializedValue = value; + + if (value === null || value === undefined) { + this._customChoices = new Set(); + } else if (this.allowCustomChoice) { + const old = this.modelValue; + // Convert serializedValue to modelValue to store as custom choices, or fall back to the input value + this._customChoices = new Set( + ensureArray(value).map( + val => this.formElements.find(el => el.serializedValue === val)?.modelValue || val, + ), + ); + this.requestUpdate('modelValue', old); + } + } + + /** + * Custom elements are all missing elements that have no corresponding element, independent if enabled or not. + */ + // @ts-ignore + get customChoices() { + if (!this.allowCustomChoice) { + return []; + } + + const elems = this._getCheckedElements(); + + return Array.from(this._customChoices).filter( + choice => !elems.some(elem => elem.choiceValue === choice), + ); + } + + constructor() { + super(); + + this.allowCustomChoice = false; + + /** + * @type {Set} + * @protected + */ + this._customChoices = new Set(); + } + + /** + * @private + */ + // @ts-ignore + __getChoicesFrom(input) { + const values = input; + if (!this.allowCustomChoice) { + return values; + } + + if (this.multipleChoice) { + return [...ensureArray(values), ...this.customChoices]; + } + + if (values === '') { + return this._customChoices.values().next().value || ''; + } + + return values; + } + + /** + * @protected + */ + _isEmpty() { + return super._isEmpty() && this._customChoices.size === 0; + } + + clear() { + this._customChoices = new Set(); + super.clear(); + } + + /** + * @param {string|string[]} value + * @returns {*} + */ + parser(value) { + if (this.allowCustomChoice && Array.isArray(value)) { + return value.filter(v => v.trim() !== ''); + } + + return value; + } + }; + +export const CustomChoiceGroupMixin = dedupeMixin(CustomChoiceGroupMixinImplementation); diff --git a/packages/ui/components/form-core/src/validate/validators/Required.js b/packages/ui/components/form-core/src/validate/validators/Required.js index 2bbe142bfb..416117ba37 100644 --- a/packages/ui/components/form-core/src/validate/validators/Required.js +++ b/packages/ui/components/form-core/src/validate/validators/Required.js @@ -45,7 +45,7 @@ export class Required extends Validator { /** * @param {FormControlHost & HTMLElement} formControl */ - // @ts-ignore [allow-protected] we are allowed to know FormControl protcected props in form-core + // @ts-ignore [allow-protected] we are allowed to know FormControl protected props in form-core // eslint-disable-next-line class-methods-use-this onFormControlConnect({ _inputNode: inputNode }) { if (inputNode) { @@ -61,7 +61,7 @@ export class Required extends Validator { /** * @param {FormControlHost & HTMLElement} formControl */ - // @ts-ignore [allow-protected] we are allowed to know FormControl protcected props in form-core + // @ts-ignore [allow-protected] we are allowed to know FormControl protected props in form-core // eslint-disable-next-line class-methods-use-this onFormControlDisconnect({ _inputNode: inputNode }) { if (inputNode) { diff --git a/packages/ui/components/form-core/test-suites/choice-group/ChoiceGroupMixin.suite.js b/packages/ui/components/form-core/test-suites/choice-group/ChoiceGroupMixin.suite.js index dbf320e248..8857729e68 100644 --- a/packages/ui/components/form-core/test-suites/choice-group/ChoiceGroupMixin.suite.js +++ b/packages/ui/components/form-core/test-suites/choice-group/ChoiceGroupMixin.suite.js @@ -55,6 +55,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi `) ); + expect(el.modelValue).to.equal('female'); el.formElements[0].checked = true; expect(el.modelValue).to.equal('male'); diff --git a/packages/ui/components/form-core/test-suites/choice-group/CustomChoiceGroupMixin.suite.js b/packages/ui/components/form-core/test-suites/choice-group/CustomChoiceGroupMixin.suite.js new file mode 100644 index 0000000000..0cd519cf0b --- /dev/null +++ b/packages/ui/components/form-core/test-suites/choice-group/CustomChoiceGroupMixin.suite.js @@ -0,0 +1,265 @@ +import '@lion/ui/define/lion-fieldset.js'; +import '@lion/ui/define/lion-checkbox-group.js'; +import '@lion/ui/define/lion-checkbox.js'; +import { expect, fixture, fixtureSync, html, unsafeStatic } from '@open-wc/testing'; + +/** + * + * @typedef {import('../../test/choice-group/CustomChoiceGroupMixin.test.js').CustomChoiceGroup} CustomChoiceGroup + */ + +/** + * @param {{ parentTagString?:string, childTagString?: string, choiceType?: string}} config + */ +export function runCustomChoiceGroupMixinSuite({ + parentTagString, + childTagString, + choiceType, +} = {}) { + const cfg = { + parentTagString: parentTagString || 'custom-choice-input-group', + childTagString: childTagString || 'custom-choice-input', + choiceType: choiceType || 'single', + }; + + const parentTag = unsafeStatic(cfg.parentTagString); + const childTag = unsafeStatic(cfg.childTagString); + + describe(`CustomChoiceGroupMixin: ${cfg.parentTagString}`, () => { + if (cfg.choiceType === 'single') { + it('has a single modelValue representing a custom value', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice name="gender[]"> + <${childTag} .choiceValue=${'male'}> + <${childTag} checked .choiceValue=${'female'}> + + `) + ); + + await el.registrationComplete; + + expect(el.modelValue).to.equal('female'); + el.modelValue = 'male'; + expect(el.modelValue).to.equal('male'); + + el.modelValue = 'other'; + expect(el.modelValue).to.equal('other'); + + expect(el.formElements[0].checked).to.be.false; + expect(el.formElements[1].checked).to.be.false; + }); + + it('has a single formattedValue representing a custom value', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice name="gender"> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'} checked> + + `) + ); + + el.modelValue = 'other'; + expect(el.formattedValue).to.equal('other'); + }); + } + + it('can set initial custom modelValue on creation', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice name="gender[]" .modelValue=${'other'}> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'}> + + `) + ); + + if (cfg.choiceType === 'single') { + expect(el.modelValue).to.equal('other'); + } else { + expect(el.modelValue).to.deep.equal(['other']); + } + expect(el.formElements[0].checked).to.be.false; + expect(el.formElements[1].checked).to.be.false; + }); + + it('can set initial custom serializedValue on creation', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice name="gender[]" .serializedValue=${'other'}> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'}> + + `) + ); + + if (cfg.choiceType === 'single') { + expect(el.serializedValue).to.equal('other'); + } else { + expect(el.serializedValue).to.deep.equal(['other']); + } + expect(el.formElements[0].checked).to.be.false; + expect(el.formElements[1].checked).to.be.false; + }); + + it('can set initial custom formattedValue on creation', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice name="gender[]" .formattedValue=${'other'}> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'}> + + `) + ); + + if (cfg.choiceType === 'single') { + expect(el.formattedValue).to.equal('other'); + } else { + expect(el.formattedValue).to.deep.equal(['other']); + } + expect(el.formElements[0].checked).to.be.false; + expect(el.formElements[1].checked).to.be.false; + }); + + it('correctly handles custom modelValue being set before registrationComplete', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + fixtureSync(html` + <${parentTag} allow-custom-choice name="gender[]" .modelValue=${null}> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'}> + + `) + ); + + if (cfg.choiceType === 'single') { + el.modelValue = 'other'; + await el.registrationComplete; + expect(el.modelValue).to.equal('other'); + } else { + el.modelValue = ['other']; + await el.registrationComplete; + expect(el.modelValue).to.deep.equal(['other']); + } + }); + + it('correctly handles custom serializedValue being set before registrationComplete', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + fixtureSync(html` + <${parentTag} allow-custom-choice name="gender[]" .serializedValue=${null}> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'}> + + `) + ); + + if (cfg.choiceType === 'single') { + // @ts-expect-error + el.serializedValue = 'other'; + await el.registrationComplete; + expect(el.serializedValue).to.equal('other'); + } else { + // @ts-expect-error + el.serializedValue = ['other']; + await el.registrationComplete; + expect(el.serializedValue).to.deep.equal(['other']); + } + }); + + it('can be cleared, even when a custom value is selected', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice name="gender[]"> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'}> + + `) + ); + if (cfg.choiceType === 'single') { + el.modelValue = 'other'; + } else { + el.modelValue = ['other']; + } + + el.clear(); + + if (cfg.choiceType === 'single') { + expect(el.serializedValue).to.deep.equal(''); + } else { + expect(el.serializedValue).to.deep.equal([]); + } + }); + + describe('multipleChoice', () => { + it('has a single modelValue representing all currently checked values, including custom values', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice multiple-choice name="gender[]"> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'} checked> + + `) + ); + + expect(el.modelValue).to.eql(['female']); + + el.modelValue = ['female', 'male']; + expect(el.modelValue).to.eql(['male', 'female']); + + el.modelValue = ['female', 'male', 'other']; + expect(el.modelValue).to.eql(['male', 'female', 'other']); + }); + + it('has a single serializedValue representing all currently checked values, including custom values', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice multiple-choice name="gender[]"> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'} checked> + + `) + ); + + expect(el.serializedValue).to.eql(['female']); + + el.modelValue = ['female', 'male', 'other']; + expect(el.serializedValue).to.eql(['male', 'female', 'other']); + }); + + it('has a single formattedValue representing all currently checked values', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice multiple-choice name="gender[]"> + <${childTag} .choiceValue=${'male'}> + <${childTag} .choiceValue=${'female'} checked> + + `) + ); + + expect(el.formattedValue).to.eql(['female']); + + el.modelValue = ['female', 'male', 'other']; + expect(el.formattedValue).to.eql(['male', 'female', 'other']); + }); + + it('unchecks non-matching checkboxes when setting the modelValue', async () => { + const el = /** @type {CustomChoiceGroup} */ ( + await fixture(html` + <${parentTag} allow-custom-choice multiple-choice name="gender[]"> + <${childTag} .choiceValue=${'male'} checked> + <${childTag} .choiceValue=${'female'} checked> + + `) + ); + + expect(el.modelValue).to.eql(['male', 'female']); + expect(el.formElements[0].checked).to.be.true; + expect(el.formElements[1].checked).to.be.true; + + el.modelValue = ['other']; + expect(el.formElements[0].checked).to.be.false; + expect(el.formElements[1].checked).to.be.false; + }); + }); + }); +} diff --git a/packages/ui/components/form-core/test/choice-group/CustomChoiceGroupMixin.integrations.test.js b/packages/ui/components/form-core/test/choice-group/CustomChoiceGroupMixin.integrations.test.js new file mode 100644 index 0000000000..10b09628b2 --- /dev/null +++ b/packages/ui/components/form-core/test/choice-group/CustomChoiceGroupMixin.integrations.test.js @@ -0,0 +1,55 @@ +import { runChoiceGroupMixinSuite } from '@lion/ui/form-core-test-suites.js'; +import { LitElement } from 'lit'; +import '@lion/ui/define/lion-fieldset.js'; +import '@lion/ui/define/lion-checkbox-group.js'; +import '@lion/ui/define/lion-checkbox.js'; +import { FormGroupMixin, ChoiceInputMixin } from '@lion/ui/form-core.js'; +import { LionInput } from '@lion/ui/input.js'; +import { CustomChoiceGroupMixin } from '../../src/choice-group/CustomChoiceGroupMixin.js'; + +class CustomChoiceGroup extends CustomChoiceGroupMixin(FormGroupMixin(LitElement)) {} +customElements.define('custom-choice-input-group', CustomChoiceGroup); + +class ChoiceInput extends ChoiceInputMixin(LionInput) {} +customElements.define('custom-choice-input', ChoiceInput); + +class CustomChoiceGroupAllowCustom extends CustomChoiceGroupMixin(FormGroupMixin(LitElement)) { + constructor() { + super(); + this.allowCustomChoice = true; + } +} +customElements.define('allow-custom-choice-input-group', CustomChoiceGroupAllowCustom); + +class MultipleCustomChoiceGroup extends CustomChoiceGroupMixin(FormGroupMixin(LitElement)) { + constructor() { + super(); + this.multipleChoice = true; + } +} +customElements.define('multiple-custom-choice-input-group', MultipleCustomChoiceGroup); + +class MultipleCustomChoiceGroupAllowCustom extends CustomChoiceGroupMixin( + FormGroupMixin(LitElement), +) { + constructor() { + super(); + this.multipleChoice = true; + this.allowCustomChoice = true; + } +} +customElements.define( + 'multiple-allow-custom-choice-input-group', + MultipleCustomChoiceGroupAllowCustom, +); + +runChoiceGroupMixinSuite({ parentTagString: 'custom-choice-input-group' }); +runChoiceGroupMixinSuite({ + parentTagString: 'multiple-custom-choice-input-group', + choiceType: 'multiple', +}); +runChoiceGroupMixinSuite({ parentTagString: 'allow-custom-choice-input-group' }); +runChoiceGroupMixinSuite({ + parentTagString: 'multiple-allow-custom-choice-input-group', + choiceType: 'multiple', +}); diff --git a/packages/ui/components/form-core/test/choice-group/CustomChoiceGroupMixin.test.js b/packages/ui/components/form-core/test/choice-group/CustomChoiceGroupMixin.test.js new file mode 100644 index 0000000000..9c650769ad --- /dev/null +++ b/packages/ui/components/form-core/test/choice-group/CustomChoiceGroupMixin.test.js @@ -0,0 +1,16 @@ +import { ChoiceInputMixin, FormGroupMixin } from '@lion/ui/form-core.js'; +import { LionInput } from '@lion/ui/input.js'; +import { LitElement } from 'lit'; +import '@lion/ui/define/lion-fieldset.js'; +import '@lion/ui/define/lion-checkbox-group.js'; +import '@lion/ui/define/lion-checkbox.js'; +import { CustomChoiceGroupMixin } from '../../src/choice-group/CustomChoiceGroupMixin.js'; +import { runCustomChoiceGroupMixinSuite } from '../../test-suites/choice-group/CustomChoiceGroupMixin.suite.js'; + +export class CustomChoiceGroup extends CustomChoiceGroupMixin(FormGroupMixin(LitElement)) {} +customElements.define('custom-choice-input-group', CustomChoiceGroup); + +class ChoiceInput extends ChoiceInputMixin(LionInput) {} +customElements.define('custom-choice-input', ChoiceInput); + +runCustomChoiceGroupMixinSuite(); diff --git a/packages/ui/components/form-core/types/choice-group/ChoiceGroupMixinTypes.ts b/packages/ui/components/form-core/types/choice-group/ChoiceGroupMixinTypes.ts index ad203dca21..72e63cd5f9 100644 --- a/packages/ui/components/form-core/types/choice-group/ChoiceGroupMixinTypes.ts +++ b/packages/ui/components/form-core/types/choice-group/ChoiceGroupMixinTypes.ts @@ -25,7 +25,7 @@ export declare class ChoiceGroupHost { filterFn?: (el: FormControl, property?: string) => boolean, ): void; protected _throwWhenInvalidChildModelValue(child: FormControlHost): void; - protected _isEmpty(): void; + protected _isEmpty(): boolean; protected _checkSingleChoiceElements(ev: Event): void; protected _getCheckedElements(): ChoiceInputHost[]; protected _setCheckedElements(value: any, check: boolean): void; diff --git a/packages/ui/components/form-core/types/choice-group/CustomChoiceGroupMixinTypes.ts b/packages/ui/components/form-core/types/choice-group/CustomChoiceGroupMixinTypes.ts new file mode 100644 index 0000000000..ad971d0d78 --- /dev/null +++ b/packages/ui/components/form-core/types/choice-group/CustomChoiceGroupMixinTypes.ts @@ -0,0 +1,30 @@ +import { Constructor } from '@open-wc/dedupe-mixin'; +import { LitElement } from 'lit'; + +import { ChoiceGroupHost } from './ChoiceGroupMixinTypes.js'; + +export declare class CustomChoiceGroupHost { + allowCustomChoice: boolean; + get modelValue(): any; + set modelValue(value: any); + get serializedValue(): string; + set serializedValue(value: string); + get formattedValue(): string; + set formattedValue(value: string); + + clear(): void; + parser(value: string | string[]): string | string[]; + + protected _isEmpty(): boolean; +} + +export declare function CustomChoiceGroupImplementation>( + superclass: T, +): T & + Constructor & + Pick & + Constructor & + Pick & + Pick; + +export type CustomChoiceGroupMixin = typeof CustomChoiceGroupImplementation; diff --git a/packages/ui/components/form-core/types/choice-group/index.ts b/packages/ui/components/form-core/types/choice-group/index.ts index 5508ee4a8c..755d06e5aa 100644 --- a/packages/ui/components/form-core/types/choice-group/index.ts +++ b/packages/ui/components/form-core/types/choice-group/index.ts @@ -1,2 +1,3 @@ export * from './ChoiceInputMixinTypes.js'; export * from './ChoiceGroupMixinTypes.js'; +export * from './CustomChoiceGroupMixinTypes.js'; diff --git a/packages/ui/components/listbox/src/ListboxMixin.js b/packages/ui/components/listbox/src/ListboxMixin.js index 7dbae3a834..436e31b39d 100644 --- a/packages/ui/components/listbox/src/ListboxMixin.js +++ b/packages/ui/components/listbox/src/ListboxMixin.js @@ -196,22 +196,6 @@ const ListboxMixinImplementation = superclass => return this._listboxNode; } - /** - * @override ChoiceGroupMixin - */ - get serializedValue() { - return this.modelValue; - } - - // Duplicating from FormGroupMixin, because you cannot independently inherit/override getter + setter. - // If you override one, gotta override the other, they go in pairs. - /** - * @override ChoiceGroupMixin - */ - set serializedValue(value) { - super.serializedValue = value; - } - get activeIndex() { return this.formElements.findIndex(el => el.active === true); } @@ -473,6 +457,7 @@ const ListboxMixinImplementation = superclass => } clear() { + super.clear(); this.setCheckedIndex(-1); this.resetInteractionState(); } diff --git a/packages/ui/components/listbox/test-suites/ListboxMixin.suite.js b/packages/ui/components/listbox/test-suites/ListboxMixin.suite.js index 58d68b2a81..359e80ca48 100644 --- a/packages/ui/components/listbox/test-suites/ListboxMixin.suite.js +++ b/packages/ui/components/listbox/test-suites/ListboxMixin.suite.js @@ -163,7 +163,7 @@ export function runListboxMixinSuite(customConfig = {}) { }); it('requests update for modelValue when checkedIndex changes', async () => { - const el = await fixture(html` + const el = /** @type {LionListbox} */ await fixture(html` <${tag} name="gender" .modelValue=${'other'}> <${optionTag} .choiceValue=${'male'}> <${optionTag} .choiceValue=${'female'}> diff --git a/packages/ui/exports/types/form-core.ts b/packages/ui/exports/types/form-core.ts index b76955ce59..1a4571f74f 100644 --- a/packages/ui/exports/types/form-core.ts +++ b/packages/ui/exports/types/form-core.ts @@ -14,4 +14,8 @@ export { FormRegistrarHost } from '../../components/form-core/types/registration export { ElementWithParentFormGroup } from '../../components/form-core/types/registration/FormRegistrarMixinTypes.js'; export { FormRegistrarPortalHost } from '../../components/form-core/types/registration/FormRegistrarPortalMixinTypes.js'; export { SyncUpdatableHost } from '../../components/form-core/types/utils/SyncUpdatableMixinTypes.js'; -export { ValidateHost, ValidationType, FeedbackMessage } from '../../components/form-core/types/validate/ValidateMixinTypes.js'; +export { + ValidateHost, + ValidationType, + FeedbackMessage, +} from '../../components/form-core/types/validate/ValidateMixinTypes.js';