diff --git a/.changeset/silent-waves-teach.md b/.changeset/silent-waves-teach.md new file mode 100644 index 000000000..7c0f7953a --- /dev/null +++ b/.changeset/silent-waves-teach.md @@ -0,0 +1,8 @@ +--- +'@lion/combobox': patch +'@lion/form-core': patch +'@lion/form-integrations': patch +'@lion/listbox': patch +--- + +Allow flexibility for extending the repropagation prevention conditions, which is needed for combobox, so that a model-value-changed event is propagated when no option matches after an input change. This allows validation to work properly e.g. for Required. diff --git a/packages/combobox/README.md b/packages/combobox/README.md index b5210901b..1094a3b3c 100644 --- a/packages/combobox/README.md +++ b/packages/combobox/README.md @@ -263,6 +263,35 @@ export const invokerButton = () => html` `; ``` +## Validation + +Validation can be used as normal, below is an example of a combobox with a `Required` validator. + +```js preview-story +export const validation = () => { + loadDefaultFeedbackMessages(); + Required.getMessage = () => 'Please enter a value'; + return html` + +
+ + Rocky + Rocky II + Rocky III + Rocky IV + Rocky V + Rocky Balboa + +
+ + `; +}; +``` + ## Listbox compatibility All configurations that can be applied to `lion-listbox`, can be applied to `lion-combobox` as well. diff --git a/packages/combobox/package.json b/packages/combobox/package.json index 96f1dd0e6..dbcf07604 100644 --- a/packages/combobox/package.json +++ b/packages/combobox/package.json @@ -46,6 +46,9 @@ "@lion/listbox": "0.2.0", "@lion/overlays": "0.21.0" }, + "devDependencies": { + "@lion/validate-messages": "0.3.1" + }, "keywords": [ "combobox", "form", diff --git a/packages/combobox/src/LionCombobox.js b/packages/combobox/src/LionCombobox.js index d66e33750..a08cc6bdc 100644 --- a/packages/combobox/src/LionCombobox.js +++ b/packages/combobox/src/LionCombobox.js @@ -420,13 +420,30 @@ export class LionCombobox extends OverlayMixin(LionListbox) { } } - /* eslint-disable no-param-reassign, class-methods-use-this */ + /** + * We need to extend the repropagation prevention conditions here. + * Usually form groups with single choice will not repropagate model-value-changed of an option upwards + * if this option itself is not the checked one. We want to prevent duplicates. However, for combobox + * it is reasonable that an option can become unchecked without another one becoming checked, because + * users can enter any text they want, whether it matches an option or not. + * + * Therefore, extend the condition to fail by checking if there is any elements checked. If so, then we + * should indeed not repropagate as normally. If there is no elements checked, this will be the only + * model-value-changed event that gets received, and we should repropagate it. + * + * @param {EventTarget & import('../types/choice-group/ChoiceInputMixinTypes').ChoiceInputHost} target + */ + _repropagationCondition(target) { + return super._repropagationCondition(target) || this.formElements.every(el => !el.checked); + } + /* eslint-disable no-param-reassign */ /** * @overridable * @param {LionOption & {__originalInnerHTML?:string}} option * @param {string} matchingString */ + // eslint-disable-next-line class-methods-use-this _onFilterMatch(option, matchingString) { const { innerHTML } = option; option.__originalInnerHTML = innerHTML; @@ -443,7 +460,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { * @param {string} [curValue] * @param {string} [prevValue] */ - // eslint-disable-next-line no-unused-vars + // eslint-disable-next-line no-unused-vars, class-methods-use-this _onFilterUnmatch(option, curValue, prevValue) { if (option.__originalInnerHTML) { option.innerHTML = option.__originalInnerHTML; @@ -451,18 +468,21 @@ export class LionCombobox extends OverlayMixin(LionListbox) { // Alternatively, an extension can add an animation here option.style.display = 'none'; } + /* eslint-enable no-param-reassign */ /** * Computes whether a user intends to autofill (inline autocomplete textbox) * @overridable * @param {{ prevValue:string, curValue:string }} config */ + // eslint-disable-next-line class-methods-use-this _computeUserIntendsAutoFill({ prevValue, curValue }) { const userIsAddingChars = prevValue.length < curValue.length; const userStartsNewWord = prevValue.length && curValue.length && prevValue[0].toLowerCase() !== curValue[0].toLowerCase(); + return userIsAddingChars || userStartsNewWord; } @@ -629,7 +649,6 @@ export class LionCombobox extends OverlayMixin(LionListbox) { this.opened = false; this.__shouldAutocompleteNextUpdate = true; this._setTextboxValue(''); - // this.checkedIndex = -1; break; case 'Enter': if (!this.formElements[this.activeIndex]) { diff --git a/packages/combobox/test/lion-combobox.test.js b/packages/combobox/test/lion-combobox.test.js index ca065073e..2df02778f 100644 --- a/packages/combobox/test/lion-combobox.test.js +++ b/packages/combobox/test/lion-combobox.test.js @@ -4,6 +4,7 @@ import sinon from 'sinon'; import '../lion-combobox.js'; import { LionOptions } from '@lion/listbox/src/LionOptions.js'; import { browserDetection, LitElement } from '@lion/core'; +import { Required } from '@lion/form-core'; /** * @typedef {import('../src/LionCombobox.js').LionCombobox} LionCombobox @@ -207,6 +208,42 @@ describe('lion-combobox', () => { await el.updateComplete; expect(el._inputNode.value).to.equal('20'); }); + + it('sets modelValue to empty string if no option is selected', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); + + expect(el.modelValue).to.equal('Artichoke'); + expect(el.formElements[0].checked).to.be.true; + el.checkedIndex = -1; + await el.updateComplete; + expect(el.modelValue).to.equal(''); + expect(el.formElements[0].checked).to.be.false; + }); + + it('sets modelValue to empty array if no option is selected for multiple choice', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); + + expect(el.modelValue).to.eql(['Artichoke']); + expect(el.formElements[0].checked).to.be.true; + el.checkedIndex = []; + await el.updateComplete; + expect(el.modelValue).to.eql([]); + expect(el.formElements[0].checked).to.be.false; + }); }); describe('Listbox visibility', () => { @@ -409,6 +446,32 @@ describe('lion-combobox', () => { expect(o.getAttribute('aria-hidden')).to.equal('true'); }); }); + + it('works with validation', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); + expect(el.checkedIndex).to.equal(0); + + // Simulate backspace deleting the char at the end of the string + el._inputNode.dispatchEvent(new KeyboardEvent('keydown', { key: 'Backspace' })); + el._inputNode.dispatchEvent(new Event('input')); + const arr = el._inputNode.value.split(''); + arr.splice(el._inputNode.value.length - 1, 1); + el._inputNode.value = arr.join(''); + await el.updateComplete; + el.dispatchEvent(new Event('blur')); + + expect(el.checkedIndex).to.equal(-1); + await el.feedbackComplete; + expect(el.hasFeedbackFor).to.include('error'); + expect(el.showsFeedbackFor).to.include('error'); + }); }); }); @@ -483,7 +546,6 @@ describe('lion-combobox', () => { Item 1 `)); - // @ts-expect-error sinon not typed correctly? const spy = sinon.spy(el._selectionDisplayNode, 'onComboboxElementUpdated'); el.requestUpdate('modelValue'); await el.updateComplete; diff --git a/packages/form-core/src/FocusMixin.js b/packages/form-core/src/FocusMixin.js index f9871ddca..cc016f947 100644 --- a/packages/form-core/src/FocusMixin.js +++ b/packages/form-core/src/FocusMixin.js @@ -6,6 +6,7 @@ import { FormControlMixin } from './FormControlMixin.js'; * @param {import('@open-wc/dedupe-mixin').Constructor} superclass */ const FocusMixinImplementation = superclass => + // @ts-expect-error false positive for incompatible static get properties. Lit-element merges super properties already for you. // eslint-disable-next-line no-unused-vars, max-len, no-shadow class FocusMixin extends FormControlMixin(superclass) { static get properties() { diff --git a/packages/form-core/src/FormControlMixin.js b/packages/form-core/src/FormControlMixin.js index 03470e943..e3504c5e5 100644 --- a/packages/form-core/src/FormControlMixin.js +++ b/packages/form-core/src/FormControlMixin.js @@ -794,13 +794,14 @@ const FormControlMixinImplementation = superclass => return; } - // B2. Are we a single choice choice-group? If so, halt when unchecked + // B2. Are we a single choice choice-group? If so, halt when target unchecked + // and something else is checked, meaning we will get + // another model-value-changed dispatch for the checked target // // We only send the checked changed up (not the unchecked). In this way a choice group // (radio-group, checkbox-group, select/listbox) acts as an 'endpoint' (a single Field) // just like the native