feat: support (multiselect) choice-groups with allow-custom-choice

ing-bank · Nov 8, 2023 · be4e25a · be4e25a
1 parent c459ded
commit be4e25a
Show file tree

Hide file tree

Showing 21 changed files with 900 additions and 107 deletions.
diff --git a/.changeset/dirty-emus-exercise.md 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
@@ -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
@@ -25,7 +25,7 @@ export class DemoSelectionDisplay extends LitElement {
        * Can be used to visually indicate the next
        */
       removeChipOnNextBackspace: Boolean,
-      selectedElements: Array,
+      selectedChoices: Array,
     };
   }
 
@@ -72,20 +72,14 @@ 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;
   }
 
   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`
       <span class="selection-chip ${highlight ? 'selection-chip--highlighted' : ''}">
-        ${option.value}
+        ${option}
       </span>
     `;
   }
@@ -154,9 +127,9 @@ export class DemoSelectionDisplay extends LitElement {
     }
     return html`
       <div class="combobox__selection">
-        ${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
@@ -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`
+  <lion-combobox name="combo" label="Multiple" .requireOptionMatch=${false} multiple-choice>
+    <demo-selection-display
+      slot="selection-display"
+      style="display: contents;"
+    ></demo-selection-display>
+    ${lazyRender(
+      listboxData.map(
+        (entry, i) =>
+          html` <lion-option .choiceValue="${entry}" ?checked=${i === 0}>${entry}</lion-option> `,
+      ),
+    )}
+  </lion-combobox>
+`;
+```
+
 ## 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
@@ -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 <input>
+   * @override CustomChoiceGroupMixin
+   * @param {string|string[]} value - viewValue: the formatted value inside <input>
    * @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;
     }
   }
@@ -1068,25 +1086,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;
+      }
     }
   }
 
@@ -1113,12 +1158,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
@@ -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
@@ -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',
+});