From 1c18057ceeb38e1c0f6933b89afcbfee971aaa69 Mon Sep 17 00:00:00 2001 From: gvangeest Date: Mon, 13 Feb 2023 16:42:47 +0100 Subject: [PATCH] fix(combobox): make the first occurrence of a string highlighted, instead of the last --- .changeset/fast-taxis-kneel.md | 5 + docs/components/combobox/use-cases.md | 54 ++++-- docs/components/listbox/src/listboxData.js | 66 +++++++ .../components/combobox/src/LionCombobox.js | 99 ++++------- .../src/utils/makeMatchingTextBold.js | 68 +++++++ .../combobox/test/lion-combobox.test.js | 167 +++++++++++++++++- packages/ui/exports/combobox.js | 4 + 7 files changed, 382 insertions(+), 81 deletions(-) create mode 100644 .changeset/fast-taxis-kneel.md create mode 100644 packages/ui/components/combobox/src/utils/makeMatchingTextBold.js diff --git a/.changeset/fast-taxis-kneel.md b/.changeset/fast-taxis-kneel.md new file mode 100644 index 000000000..be652d700 --- /dev/null +++ b/.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. diff --git a/docs/components/combobox/use-cases.md b/docs/components/combobox/use-cases.md index 870eb507f..9dbc6a548 100644 --- a/docs/components/combobox/use-cases.md +++ b/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``; ``` -## 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 - * @param {LionOption} option - * @returns {string} + * @overridable + * @param {LionOption & {__originalInnerHTML?:string}} option + * @param {string} matchingString + * @protected */ - // eslint-disable-next-line class-methods-use-this - _getTextboxValueFromOption(option) { - return option.label; + _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 + */ + _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` ${lazyRender( - listboxDataObject.map( + listboxComplexData.map( entry => html` - ${entry.label} + +
${entry.label}
+ ${entry.description} + `, ), )} diff --git a/docs/components/listbox/src/listboxData.js b/docs/components/listbox/src/listboxData.js index 60f89ff59..af3d42b10 100644 --- a/docs/components/listbox/src/listboxData.js +++ b/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' }, +]; diff --git a/packages/ui/components/combobox/src/LionCombobox.js b/packages/ui/components/combobox/src/LionCombobox.js index 8f8f436fa..6d7af9267 100644 --- a/packages/ui/components/combobox/src/LionCombobox.js +++ b/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 @@ -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)) { - matchReverseFns.get(option)(); - } + this._unhighlightMatchedOption(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 */ diff --git a/packages/ui/components/combobox/src/utils/makeMatchingTextBold.js b/packages/ui/components/combobox/src/utils/makeMatchingTextBold.js new file mode 100644 index 000000000..11258598b --- /dev/null +++ b/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); + } + }); +} diff --git a/packages/ui/components/combobox/test/lion-combobox.test.js b/packages/ui/components/combobox/test/lion-combobox.test.js index fe3b3dbae..40414ae1b 100644 --- a/packages/ui/components/combobox/test/lion-combobox.test.js +++ b/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` @@ -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(`Artichoke`); + expect(options[1]).lightDom.to.equal(`Chard`); + expect(options[2]).lightDom.to.equal(`Chicory`); + expect(options[3]).lightDom.to.equal( + `Victoria Plum`, + ); + 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` + + +
Artichoke
+ Cardoon + + +
Chard
+ Beet + + +
Chicory
+ Chicory + + +
Victoria Plum
+ Prunus domestica + + + `) + ); + const options = el.formElements; + + mimicUserTyping(/** @type {LionCombobox} */ (el), 'ch'); + + await el.updateComplete; + expect(options[0]).lightDom.to.equal( + `
Artichoke
Cardoon`, + ); + expect(options[1]).lightDom.to.equal( + `
Chard
Beet`, + ); + expect(options[2]).lightDom.to.equal( + `
Chicory
Chicory`, + ); + expect(options[3]).lightDom.to.equal( + `
Victoria Plum
Prunus domestica`, + ); + }); + 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"> + +
Artichoke
+ Cardoon + + +
Chard
+ Beet + + +
Chicory
+ Chicory + + +
Victoria Plum
+ Prunus domestica + + + `) + ); + const options = el.formElements; + + mimicUserTyping(/** @type {LionCombobox} */ (el), 'ch'); + + await el.updateComplete; + expect(options[0]).lightDom.to.equal( + `
Artichoke
Cardoon`, + ); + expect(options[1]).lightDom.to.equal( + `
Chard
Beet`, + ); + expect(options[2]).lightDom.to.equal( + `
Chicory
Chicory`, + ); + expect(options[3]).lightDom.to.equal( + `
Victoria Plum
Prunus domestica`, + ); + + mimicUserTyping(/** @type {LionCombobox} */ (el), 'D'); + + await el.updateComplete; + expect(options[0]).lightDom.to.equal(`
Artichoke
Cardoon`); + expect(options[1]).lightDom.to.equal( + `
Chard
Beet`, + ); + expect(options[2]).lightDom.to.equal(`
Chicory
Chicory`); + expect(options[3]).lightDom.to.equal( + `
Victoria Plum
Prunus domestica`, + ); + }); }); 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` + + +
Artichoke
+ Cardoon + + + `) + ); + 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'); + }); }); }); diff --git a/packages/ui/exports/combobox.js b/packages/ui/exports/combobox.js index 05107b926..8309c9357 100644 --- a/packages/ui/exports/combobox.js +++ b/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';