fix(combobox): make the first occurrence of a string highlighted, instead of the last

.changeset/fast-taxis-kneel.md

@@ -0,0 +1,5 @@
+---
+'@lion/ui': patch
+---
+
+[combobox] make the first occurrence of a string highlighted, instead of the last.

docs/components/combobox/use-cases.md

@@ -14,7 +14,7 @@ availability of the popup.
 
 ```js script
 import { LitElement, html, repeat } from '@mdjs/mdjs-preview';
-import { listboxData } from '../listbox/src/listboxData.js';
+import { listboxData, listboxComplexData } from '../listbox/src/listboxData.js';
 import { LionCombobox } from '@lion/ui/combobox.js';
 import '@lion/ui/define/lion-combobox.js';
 import '@lion/ui/define/lion-option.js';
@@ -328,20 +328,45 @@ customElements.define('demo-server-side', DemoServerSide);
 export const serverSideCompletion = () => html`<demo-server-side></demo-server-side>`;
 ```
 
-## Set complex object in choiceValue
+## Complex options
 
-A common use case is to set a complex object in the `choiceValue` property. By default, `LionCombobox` will display in the text input the `modelValue`. By overriding the `_getTextboxValueFromOption` method, you can choose which value will be used for the input value.
+For performance reasons a complex object in the choiceValue property is unwanted. But it is possible to create more complex options.
+
+To highlight the correct elements of the option, each element should be tagged with a `data-key` attribute. Which will be used in the `_onFilterMatch` and `_onFilterUnmatch` functions.
 
 ```js preview-story
 class ComplexObjectCombobox extends LionCombobox {
   /**
-   * Override how the input text is filled
+   * @overridable
-   * @param {LionOption} option
+   * @param {LionOption & {__originalInnerHTML?:string}} option
-   * @returns {string}
+   * @param {string} matchingString
+   * @protected
    */
-  // eslint-disable-next-line class-methods-use-this
+  _onFilterMatch(option, matchingString) {
-  _getTextboxValueFromOption(option) {
+    Array.from(option.children).forEach(child => {
-    return option.label;
+      if (child.hasAttribute('data-key')) {
+        this._highlightMatchedOption(child, matchingString);
+      }
+    });
+    // Alternatively, an extension can add an animation here
+    option.style.display = '';
+  }
+
+  /**
+   * @overridable
+   * @param {LionOption & {__originalInnerHTML?:string}} option
+   * @param {string} [curValue]
+   * @param {string} [prevValue]
+   * @protected
+   */
+  _onFilterUnmatch(option, curValue, prevValue) {
+    Array.from(option.children).forEach(child => {
+      if (child.hasAttribute('data-key')) {
+        this._unhighlightMatchedOption(child);
+      }
+    });
+    // Alternatively, an extension can add an animation here
+    option.style.display = 'none';
   }
 }
 
@@ -351,20 +376,19 @@ const onModelValueChanged = event => {
   console.log(`event.target.modelValue: ${JSON.stringify(event.target.modelValue)}`);
 };
 
-const listboxDataObject = listboxData.map(e => {
-  return { label: e, name: e };
-});
-
 export const complexObjectChoiceValue = () => html` <complex-object-combobox
   name="combo"
   label="Display only the label once selected"
   @model-value-changed="${onModelValueChanged}"
 >
   ${lazyRender(
-    listboxDataObject.map(
+    listboxComplexData.map(
       entry =>
         html`
-          <lion-option .choiceValue="${entry}" .label="${entry.label}">${entry.label}</lion-option>
+          <lion-option .choiceValue="${entry.label}">
+            <div data-key>${entry.label}</div>
+            <small>${entry.description}</small>
+          </lion-option>
         `,
     ),
   )}

docs/components/listbox/src/listboxData.js

@@ -63,3 +63,69 @@ export const listboxData = [
   'Yam',
   'Zucchini',
 ];
+
+export const listboxComplexData = [
+  { label: 'Apple', description: 'Rosaceae' },
+  { label: 'Artichoke', description: 'Cardoon' },
+  { label: 'Asparagus', description: 'Liliopsida' },
+  { label: 'Banana', description: 'Bananas' },
+  { label: 'Beets', description: 'Beet' },
+  { label: 'Bell pepper', description: 'Solanaceae' },
+  { label: 'Broccoli', description: 'Wild cabbage' },
+  { label: 'Brussels sprout', description: 'Wild cabbage' },
+  { label: 'Cabbage', description: 'Wild cabbage' },
+  { label: 'Carrot', description: 'Apiales' },
+  { label: 'Cauliflower', description: 'Wild cabbage' },
+  { label: 'Celery', description: 'Apium' },
+  { label: 'Chard', description: 'Beet' },
+  { label: 'Chicory', description: 'Chicory' },
+  { label: 'Corn', description: 'Corn' },
+  { label: 'Cucumber', description: 'Cucumis' },
+  { label: 'Daikon', description: 'Radishes' },
+  { label: 'Date', description: 'Arecaceae' },
+  { label: 'Edamame', description: 'Wild bean' },
+  { label: 'Eggplant', description: 'Nightshade' },
+  { label: 'Elderberry', description: 'Moschatel' },
+  { label: 'Fennel', description: 'Fennels' },
+  { label: 'Fig', description: 'Fig trees' },
+  { label: 'Garlic', description: 'Allium' },
+  { label: 'Grape', description: 'Vitis vinifera' },
+  { label: 'Honeydew melon', description: 'Citrullus' },
+  { label: 'Iceberg lettuce', description: 'Lactuca' },
+  { label: 'Jerusalem artichoke', description: 'Sunflowers' },
+  { label: 'Kale', description: 'Brassicaceae' },
+  { label: 'Kiwi', description: 'Ratites' },
+  { label: 'Leek', description: 'Onion' },
+  { label: 'Lemon', description: 'Citrus' },
+  { label: 'Mango', description: 'Mangifera' },
+  { label: 'Mangosteen', description: 'Saptrees' },
+  { label: 'Melon', description: 'Citrullus' },
+  { label: 'Mushroom', description: 'Eumycota' },
+  { label: 'Nectarine', description: 'Citrus' },
+  { label: 'Okra', description: 'Mallows' },
+  { label: 'Olive', description: 'Olives' },
+  { label: 'Onion', description: 'Onion' },
+  { label: 'Orange', description: 'Citrus' },
+  { label: 'Parship', description: 'Umbellifers' },
+  { label: 'Pea', description: 'Peas' },
+  { label: 'Pear', description: 'Malinae' },
+  { label: 'Pineapple', description: 'Pineapples' },
+  { label: 'Potato', description: 'Nightshade' },
+  { label: 'Pumpkin', description: 'Cucurbitaceae' },
+  { label: 'Quince', description: 'Cydonia' },
+  { label: 'Radish', description: 'Radishes' },
+  { label: 'Rhubarb', description: 'Rhubarb' },
+  { label: 'Shallot', description: 'Onion' },
+  { label: 'Spinach', description: 'Spinacia' },
+  { label: 'Squash', description: 'Cucurbiteae' },
+  { label: 'Strawberry', description: 'Fragaria' },
+  { label: 'Sweet potato', description: 'Ipomoea' },
+  { label: 'Tomato', description: 'Nightshade' },
+  { label: 'Turnip', description: 'Field mustard' },
+  { label: 'Ugli fruit', description: 'Citrus reticulata' },
+  { label: 'Victoria plum', description: 'Prunus domestica' },
+  { label: 'Watercress', description: 'Nasturtium' },
+  { label: 'Watermelon', description: 'Citrullus' },
+  { label: 'Yam', description: 'Dioscorea' },
+  { label: 'Zucchini', description: 'Cucurbitaceae' },
+];

packages/ui/components/combobox/src/LionCombobox.js

@@ -1,62 +1,11 @@
-import { html, css } from 'lit';
 import { browserDetection } from '@lion/ui/core.js';
-import { OverlayMixin, withDropdownConfig } from '@lion/ui/overlays.js';
 import { LionListbox } from '@lion/ui/listbox.js';
+import { OverlayMixin, withDropdownConfig } from '@lion/ui/overlays.js';
+import { css, html } from 'lit';
+import { makeMatchingTextBold, unmakeMatchingTextBold } from './utils/makeMatchingTextBold.js';
 
-const matchReverseFns = new WeakMap();
 const matchA11ySpanReverseFns = new WeakMap();
 
-/**
- * @param {Node} root
- * @param {string} matchingString
- * @param {Node} option
- */
-function makeMatchingTextBold(root, matchingString, option) {
-  Array.from(root.childNodes).forEach(childNode => {
-    if (childNode.nodeName === '#text') {
-      // check for match based on nodeValue
-
-      const re = new RegExp(`^(.*)(${matchingString})(.*)$`, 'i');
-      // @ts-ignore
-      const match = childNode.nodeValue.match(re);
-
-      if (match) {
-        // 1. textContent before match
-
-        const textBefore = document.createTextNode(match[1]);
-        root.appendChild(textBefore);
-
-        // 2. matched part
-        const boldElement = document.createElement('b');
-        // eslint-disable-next-line prefer-destructuring
-        boldElement.textContent = match[2];
-        root.appendChild(boldElement);
-
-        // 3. textContent after match
-
-        const textAfter = document.createTextNode(match[3]);
-        root.appendChild(textAfter);
-        root.removeChild(childNode);
-
-        matchReverseFns.set(option, () => {
-          root.appendChild(childNode);
-          if (root.contains(textBefore) && textBefore.parentNode !== null) {
-            textBefore.parentNode.removeChild(textBefore);
-          }
-          if (root.contains(boldElement) && boldElement.parentNode !== null) {
-            boldElement.parentNode.removeChild(boldElement);
-          }
-          if (root.contains(textAfter) && textAfter.parentNode !== null) {
-            textAfter.parentNode.removeChild(textAfter);
-          }
-        });
-      }
-    } else {
-      makeMatchingTextBold(childNode, matchingString, option);
-    }
-  });
-}
-
 // TODO: make ListboxOverlayMixin that is shared between SelectRich and Combobox
 // TODO: extract option matching based on 'typed character cache' and share that logic
 // on Listbox or ListNavigationWithActiveDescendantMixin
@@ -304,7 +253,6 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
      * @type {'begin'|'all'}
      */
     this.matchMode = 'all';
-
     /**
      * When true, the listbox is open and textbox goes from a value to empty, all options are shown.
      * By default, the listbox closes on empty, similar to wai-aria example and <datalist>
@@ -642,14 +590,27 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
    * @param {string} matchingString
    * @protected
    */
-  // eslint-disable-next-line class-methods-use-this
   _onFilterMatch(option, matchingString) {
-    makeMatchingTextBold(option, matchingString, option);
+    this._highlightMatchedOption(option, matchingString);
+
+    // Alternatively, an extension can add an animation here
+    option.style.display = '';
+  }
+
+  /**
+   * @overridable
+   * @param {Element} option
+   * @param {string} matchingString
+   * @protected
+   */
+  // eslint-disable-next-line class-methods-use-this
+  _highlightMatchedOption(option, matchingString) {
+    makeMatchingTextBold(option, matchingString);
 
     // For Safari, we need to add a label to the element
     if (option.textContent) {
       const a11ySpan = document.createElement('span');
-      a11ySpan.setAttribute('aria-label', option.textContent);
+      a11ySpan.setAttribute('aria-label', option.textContent.replace(/\s+/g, ' '));
       Array.from(option.childNodes).forEach(childNode => {
         a11ySpan.appendChild(childNode);
       });
@@ -664,8 +625,6 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
         }
       });
     }
-    // Alternatively, an extension can add an animation here
-    option.style.display = '';
   }
 
   /**
@@ -677,14 +636,24 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
    */
   // eslint-disable-next-line no-unused-vars, class-methods-use-this
   _onFilterUnmatch(option, curValue, prevValue) {
-    if (matchReverseFns.has(option)) {
+    this._unhighlightMatchedOption(option);
-      matchReverseFns.get(option)();
+
-    }
+    // Alternatively, an extension can add an animation here
+    option.style.display = 'none';
+  }
+
+  /**
+   * @overridable
+   * @param {Element} option
+   * @protected
+   */
+  // eslint-disable-next-line class-methods-use-this
+  _unhighlightMatchedOption(option) {
+    unmakeMatchingTextBold(option);
+
     if (matchA11ySpanReverseFns.has(option)) {
       matchA11ySpanReverseFns.get(option)();
     }
-    // Alternatively, an extension can add an animation here
-    option.style.display = 'none';
   }
   /* eslint-enable no-param-reassign */
 

packages/ui/components/combobox/src/utils/makeMatchingTextBold.js

@@ -0,0 +1,68 @@
+const matchReverseFns = new WeakMap();
+
+/**
+ * @param {Node} root
+ * @param {string} matchingString
+ */
+export function makeMatchingTextBold(root, matchingString) {
+  Array.from(root.childNodes).forEach(childNode => {
+    if (childNode.nodeName === '#text') {
+      // check for match based on nodeValue
+
+      const re = new RegExp(`^(.*?)(${matchingString})(.*)$`, 'i');
+      // @ts-ignore
+      const match = childNode.nodeValue.match(re);
+
+      if (match) {
+        // 1. textContent before match
+        const textBefore = document.createTextNode(match[1]);
+        root.appendChild(textBefore);
+
+        // 2. matched part
+        const boldElement = document.createElement('b');
+        // eslint-disable-next-line prefer-destructuring
+        boldElement.textContent = match[2];
+        root.appendChild(boldElement);
+
+        // 3. textContent after match
+        const textAfter = document.createTextNode(match[3]);
+        root.appendChild(textAfter);
+
+        root.removeChild(childNode);
+
+        matchReverseFns.set(root, () => {
+          root.appendChild(childNode);
+          if (root.contains(textBefore) && textBefore.parentNode !== null) {
+            textBefore.parentNode.removeChild(textBefore);
+          }
+          if (root.contains(boldElement) && boldElement.parentNode !== null) {
+            boldElement.parentNode.removeChild(boldElement);
+          }
+          if (root.contains(textAfter) && textAfter.parentNode !== null) {
+            textAfter.parentNode.removeChild(textAfter);
+          }
+        });
+      }
+    } else {
+      makeMatchingTextBold(childNode, matchingString);
+    }
+  });
+}
+
+/**
+ * @param {Node} root
+ */
+export function unmakeMatchingTextBold(root) {
+  if (matchReverseFns.has(root)) {
+    matchReverseFns.get(root)();
+  }
+  Array.from(root.childNodes).forEach(childNode => {
+    if (childNode.nodeName === '#text') {
+      if (matchReverseFns.has(childNode)) {
+        matchReverseFns.get(childNode)();
+      }
+    } else {
+      unmakeMatchingTextBold(childNode);
+    }
+  });
+}

packages/ui/components/combobox/test/lion-combobox.test.js

@@ -13,6 +13,7 @@ import { LionCombobox } from '@lion/ui/combobox.js';
  * @typedef {import('../types/SelectionDisplay.js').SelectionDisplay} SelectionDisplay
  * @typedef {import('../../listbox/types/ListboxMixinTypes.js').ListboxHost} ListboxHost
  * @typedef {import('../../form-core/types/FormControlMixinTypes.js').FormControlHost} FormControlHost
+ * @typedef {import('@lion/ui/listbox.js').LionOption} LionOption
  */
 
 /**
@@ -1632,7 +1633,7 @@ describe('lion-combobox', () => {
       });
     });
 
-    it('highlights matching options', async () => {
+    it('highlights first occcurence of matching option', async () => {
       const el = /** @type {LionCombobox} */ (
         await fixture(html`
           <lion-combobox name="foo" match-mode="all">
@@ -1645,6 +1646,16 @@ describe('lion-combobox', () => {
       );
       const options = el.formElements;
 
+      mimicUserTyping(/** @type {LionCombobox} */ (el), 'c');
+
+      await el.updateComplete;
+      expect(options[0]).lightDom.to.equal(`<span aria-label="Artichoke">Arti<b>c</b>hoke</span>`);
+      expect(options[1]).lightDom.to.equal(`<span aria-label="Chard"><b>C</b>hard</span>`);
+      expect(options[2]).lightDom.to.equal(`<span aria-label="Chicory"><b>C</b>hicory</span>`);
+      expect(options[3]).lightDom.to.equal(
+        `<span aria-label="Victoria Plum">Vi<b>c</b>toria Plum</span>`,
+      );
+
       mimicUserTyping(/** @type {LionCombobox} */ (el), 'ch');
 
       await el.updateComplete;
@@ -1662,6 +1673,48 @@ describe('lion-combobox', () => {
       expect(options[3]).lightDom.to.equal(`Victoria Plum`);
     });
 
+    it('highlights matching complex options', async () => {
+      const el = /** @type {LionCombobox} */ (
+        await fixture(html`
+          <lion-combobox name="foo" match-mode="all">
+            <lion-option .choiceValue="${'Artichoke'}">
+              <div>Artichoke</div>
+              <small>Cardoon</small>
+            </lion-option>
+            <lion-option .choiceValue="${'Chard'}">
+              <div>Chard</div>
+              <small>Beet</small>
+            </lion-option>
+            <lion-option .choiceValue="${'Chicory'}">
+              <div>Chicory</div>
+              <small>Chicory</small>
+            </lion-option>
+            <lion-option .choiceValue="${'Victoria Plum'}">
+              <div>Victoria Plum</div>
+              <small>Prunus domestica</small>
+            </lion-option>
+          </lion-combobox>
+        `)
+      );
+      const options = el.formElements;
+
+      mimicUserTyping(/** @type {LionCombobox} */ (el), 'ch');
+
+      await el.updateComplete;
+      expect(options[0]).lightDom.to.equal(
+        `<span aria-label=" Artichoke Cardoon "><div>Arti<b>ch</b>oke</div><small>Cardoon</small>`,
+      );
+      expect(options[1]).lightDom.to.equal(
+        `<span aria-label=" Chard Beet "><div><b>Ch</b>ard</div><small>Beet</small>`,
+      );
+      expect(options[2]).lightDom.to.equal(
+        `<span aria-label=" Chicory Chicory "><div><b>Ch</b>icory</div><small><b>Ch</b>icory</small>`,
+      );
+      expect(options[3]).lightDom.to.equal(
+        `<div>Victoria Plum</div><small>Prunus domestica</small>`,
+      );
+    });
+
     it('synchronizes textbox when autocomplete is "inline" or "both"', async () => {
       const el = /** @type {LionCombobox} */ (
         await fixture(html`
@@ -1999,6 +2052,98 @@ describe('lion-combobox', () => {
           await performChecks();
         });
       });
+
+      it('allows to override "_onFilterMatch" and "_onFilterUmatch"', async () => {
+        class X extends LionCombobox {
+          /**
+           * @overridable
+           * @param {LionOption & {__originalInnerHTML?:string}} option
+           * @param {string} matchingString
+           * @protected
+           */
+          _onFilterMatch(option, matchingString) {
+            Array.from(option.children).forEach(child => {
+              if (child.hasAttribute('data-key')) {
+                this._highlightMatchedOption(child, matchingString);
+              }
+            });
+            // Alternatively, an extension can add an animation here
+            // option.style.display = '';
+          }
+
+          /**
+           * @overridable
+           * @param {LionOption & {__originalInnerHTML?:string}} option
+           * @param {string} [curValue]
+           * @param {string} [prevValue]
+           * @protected
+           */
+          // eslint-disable-next-line no-unused-vars
+          _onFilterUnmatch(option, curValue, prevValue) {
+            Array.from(option.children).forEach(child => {
+              if (child.hasAttribute('data-key')) {
+                this._unhighlightMatchedOption(child);
+              }
+            });
+            // Alternatively, an extension can add an animation here
+            // option.style.display = 'none';
+          }
+        }
+        const tagName = defineCE(X);
+        const tag = unsafeStatic(tagName);
+
+        const el = /** @type {LionCombobox} */ (
+          await fixture(html`
+            <${tag} name="foo" autocomplete="both">
+              <lion-option .choiceValue="${'Artichoke'}">
+                <div data-key>Artichoke</div>
+                <small>Cardoon</small>
+              </lion-option>
+              <lion-option .choiceValue="${'Chard'}">
+                <div data-key>Chard</div>
+                <small>Beet</small>
+              </lion-option>
+              <lion-option .choiceValue="${'Chicory'}">
+                <div data-key>Chicory</div>
+                <small>Chicory</small>
+              </lion-option>
+              <lion-option .choiceValue="${'Victoria Plum'}">
+                <div data-key>Victoria Plum</div>
+                <small>Prunus domestica</small>
+              </lion-option>
+            </${tag}>
+          `)
+        );
+        const options = el.formElements;
+
+        mimicUserTyping(/** @type {LionCombobox} */ (el), 'ch');
+
+        await el.updateComplete;
+        expect(options[0]).lightDom.to.equal(
+          `<div data-key><span aria-label="Artichoke">Arti<b>ch</b>oke</span></div><small>Cardoon</small>`,
+        );
+        expect(options[1]).lightDom.to.equal(
+          `<div data-key><span aria-label="Chard"><b>Ch</b>ard</div></span><small>Beet</small>`,
+        );
+        expect(options[2]).lightDom.to.equal(
+          `<div data-key><span aria-label="Chicory"><b>Ch</b>icory</span></div><small>Chicory</small>`,
+        );
+        expect(options[3]).lightDom.to.equal(
+          `<div data-key>Victoria Plum</div><small>Prunus domestica</small>`,
+        );
+
+        mimicUserTyping(/** @type {LionCombobox} */ (el), 'D');
+
+        await el.updateComplete;
+        expect(options[0]).lightDom.to.equal(`<div data-key>Artichoke</div><small>Cardoon</small>`);
+        expect(options[1]).lightDom.to.equal(
+          `<div data-key><span aria-label="Chard">Char<b>d</b></div></span><small>Beet</small>`,
+        );
+        expect(options[2]).lightDom.to.equal(`<div data-key>Chicory</div><small>Chicory</small>`);
+        expect(options[3]).lightDom.to.equal(
+          `<div data-key>Victoria Plum</div><small>Prunus domestica</small>`,
+        );
+      });
     });
 
     describe('Active index behavior', () => {
@@ -2278,6 +2423,26 @@ describe('lion-combobox', () => {
         expect(labelledElement).to.not.be.null;
         expect(labelledElement.innerText).to.equal('Artichoke');
       });
+
+      it('adds aria-label to highlighted complex options', async () => {
+        const el = /** @type {LionCombobox} */ (
+          await fixture(html`
+            <lion-combobox name="foo" match-mode="all">
+              <lion-option .choiceValue="${'Artichoke'}">
+                <div>Artichoke</div>
+                <small>Cardoon</small>
+              </lion-option>
+            </lion-combobox>
+          `)
+        );
+        const options = el.formElements;
+
+        mimicUserTyping(/** @type {LionCombobox} */ (el), 'choke');
+        await el.updateComplete;
+        const labelledElement = options[0].querySelector('span[aria-label=" Artichoke Cardoon "]');
+        expect(labelledElement).to.not.be.null;
+        expect(labelledElement.innerText).to.equal('Artichoke\nCardoon');
+      });
     });
   });
 

packages/ui/exports/combobox.js

@@ -1 +1,5 @@
 export { LionCombobox } from '../components/combobox/src/LionCombobox.js';
+export {
+  makeMatchingTextBold,
+  unmakeMatchingTextBold,
+} from '../components/combobox/src/utils/makeMatchingTextBold.js';