diff --git a/.changeset/heavy-ghosts-sell.md b/.changeset/heavy-ghosts-sell.md new file mode 100644 index 000000000..e38a9b708 --- /dev/null +++ b/.changeset/heavy-ghosts-sell.md @@ -0,0 +1,11 @@ +--- +'@lion/combobox': patch +--- + +Combobox api, demo and ux improvements + +- renamed `filterOptionCondition ` (similarity to `match-mode`, since this is basically an override) +- demos for `matchCondition` +- inline autocompletion edge cases solved (that would be inconsistent ux otherwise) +- demos took a long time render: introduced a lazyRender directive that only adds (expensive) lionOptions after first meaningful paint has happened +- made clearer from the code that selectionDisplay component is for demo purposes only at this moment diff --git a/packages/combobox/README.md b/packages/combobox/README.md index 5a9ea2cdf..273510ae9 100644 --- a/packages/combobox/README.md +++ b/packages/combobox/README.md @@ -19,7 +19,9 @@ import { loadDefaultFeedbackMessages } from '@lion/validate-messages'; import { listboxData } from '@lion/listbox/docs/listboxData.js'; import '@lion/listbox/lion-option.js'; import './lion-combobox.js'; -import './docs/lion-combobox-selection-display.js'; +import './docs/demo-selection-display.js'; +import { lazyRender } from './docs/lazyRender.js'; +import levenshtein from './docs/levenshtein.js'; export default { title: 'Forms/Combobox', @@ -29,7 +31,9 @@ export default { ```js preview-story export const main = () => html` - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` @@ -47,7 +51,7 @@ to the configurable values `none`, `list`, `inline` and `both`. | both | ✓ | ✓ | ✓ | ✓ | ✓ | - **list** shows a list on keydown character press -- **filter** filters list of potential matches according to `matchmode` or provided `filterOptionCondition` +- **filter** filters list of potential matches according to `matchmode` or provided `matchCondition` - **focus** automatically focuses closest match (makes it the activedescendant) - **check** automatically checks/selects closest match when `selection-follows-focus` is enabled (this is the default configuration) - **complete** completes the textbox value inline (the 'missing characters' will be added as selected text) @@ -59,7 +63,9 @@ Selection will happen manually by the user. ```js preview-story export const autocompleteNone = () => html` - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` @@ -69,7 +75,9 @@ When `autocomplete="list"` is configured, it will filter listbox suggestions bas ```js preview-story export const autocompleteList = () => html` - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` @@ -80,7 +88,9 @@ It does NOT filter list of potential matches. ```js preview-story export const autocompleteInline = () => html` - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` @@ -91,7 +101,9 @@ This is the default value for `autocomplete`. ```js preview-story export const autocompleteBoth = () => html` - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` @@ -107,7 +119,9 @@ So 'ch' will both match 'Chard' and 'Artichoke'. ```js preview-story export const matchModeBegin = () => html` - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` @@ -115,7 +129,33 @@ export const matchModeBegin = () => html` ```js preview-story export const matchModeAll = () => html` - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} + +`; +``` + +When the preconfigurable `match-mode` conditions are not sufficient, +one can define a custom matching function. +The example below matches when the Levenshtein distance is below 3 (including some other conditions). + +```js preview-story +export const customMatchCondition = () => html` + + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` @@ -133,7 +173,9 @@ will be kept track of independently. ```js preview-story export const noSelectionFollowsFocus = () => html` - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` @@ -149,7 +191,9 @@ export const noRotateKeyboardNavigation = () => html` label="No Rotate Keyboard Navigation" .rotateKeyboardNavigation="${false}" > - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` @@ -170,10 +214,12 @@ This will: ```js preview-story export const multipleChoice = () => html` - - ${listboxData.map( - (entry, i) => - html` ${entry} `, + + ${lazyRender( + listboxData.map( + (entry, i) => + html` ${entry} `, + ), )} `; @@ -193,7 +239,9 @@ export const invokerButton = () => html` }}" > - ${listboxData.map(entry => html` ${entry} `)} + ${lazyRender( + listboxData.map(entry => html` ${entry} `), + )} `; ``` diff --git a/packages/combobox/docs/demo-selection-display.js b/packages/combobox/docs/demo-selection-display.js new file mode 100644 index 000000000..1ab2c09e8 --- /dev/null +++ b/packages/combobox/docs/demo-selection-display.js @@ -0,0 +1,191 @@ +// eslint-disable-next-line max-classes-per-file +import { LitElement, html, css, nothing } from '@lion/core'; + +/** + * Disclaimer: this is just an example component demoing the selection display of LionCombobox + * It needs an 'a11y plan' and tests before it could be released + */ + +/** + * @typedef {import('../src/LionCombobox.js').LionCombobox} LionCombobox + */ + +/** + * Renders the wrapper containing the textbox that triggers the listbox with filtered options. + * Optionally, shows 'chips' that indicate the selection. + * Should be considered an internal/protected web component to be used in conjunction with + * LionCombobox + * + */ +export class DemoSelectionDisplay extends LitElement { + static get properties() { + return { + comboboxElement: Object, + /** + * Can be used to visually indicate the next + */ + removeChipOnNextBackspace: Boolean, + selectedElements: Array, + }; + } + + static get styles() { + return css` + :host { + display: flex; + } + + .combobox__selection { + flex: none; + } + + .combobox__input { + display: block; + } + + .selection-chip { + border-radius: 4px; + background-color: #eee; + padding: 4px; + font-size: 10px; + } + + .selection-chip--highlighted { + background-color: #ccc; + } + + * > ::slotted([slot='_textbox']) { + outline: none; + width: 100%; + height: 100%; + box-sizing: border-box; + border: none; + border-bottom: 1px solid; + } + `; + } + + /** + * @configure FocusMixin + */ + get _inputNode() { + 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 = []; + + /** @type {EventListener} */ + this.__textboxOnKeyup = this.__textboxOnKeyup.bind(this); + /** @type {EventListener} */ + this.__restoreBackspace = this.__restoreBackspace.bind(this); + } + + /** + * @param {import('lit-element').PropertyValues } changedProperties + */ + firstUpdated(changedProperties) { + super.firstUpdated(changedProperties); + + if (this.multipleChoice) { + this._inputNode.addEventListener('keyup', this.__textboxOnKeyup); + this._inputNode.addEventListener('focusout', this.__restoreBackspace); + } + } + + /** + * @param {import('lit-element').PropertyValues } changedProperties + */ + 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.__prevSelectedEls = this.selectedElements; + } + + /** + * @param {import("@lion/listbox").LionOption} option + * @param {boolean} highlight + */ + // eslint-disable-next-line class-methods-use-this + _selectedElementTemplate(option, highlight) { + return html` + + ${option.value} + + `; + } + + _selectedElementsTemplate() { + if (!this.multipleChoice) { + return nothing; + } + return html` +
+ ${this.selectedElements.map((option, i) => { + const highlight = Boolean( + this.removeChipOnNextBackspace && i === this.selectedElements.length - 1, + ); + return this._selectedElementTemplate(option, highlight); + })} +
+ `; + } + + render() { + return html` ${this._selectedElementsTemplate()} `; + } + + /** + * @param {{ key: string; }} ev + */ + __textboxOnKeyup(ev) { + if (ev.key === 'Backspace') { + if (!this._inputNode.value) { + if (this.removeChipOnNextBackspace && this.selectedElements.length) { + this.selectedElements[this.selectedElements.length - 1].checked = false; + } + this.removeChipOnNextBackspace = true; + } + } else { + this.removeChipOnNextBackspace = false; + } + } + + __restoreBackspace() { + this.removeChipOnNextBackspace = false; + } +} +customElements.define('demo-selection-display', DemoSelectionDisplay); diff --git a/packages/combobox/docs/lazyRender.js b/packages/combobox/docs/lazyRender.js new file mode 100644 index 000000000..12e6631e4 --- /dev/null +++ b/packages/combobox/docs/lazyRender.js @@ -0,0 +1,23 @@ +import { directive } from '@lion/core'; + +/** + * In order to speed up the first meaningful paint, use this directive + * on content that is: + * - (visually) hidden + * - out of the page flow (having position: 'absolute|fixed') + * + * A good practice would be to use it in overlays, + * For hidden tab panels, collapsible content etc. it's also useful + * @example + * + * ${lazyRender( + * largeListOfData.map(entry => html` ${entry} `), + * )} + * + */ +export const lazyRender = directive(tplResult => part => { + setTimeout(() => { + part.setValue(tplResult); + part.commit(); + }); +}); diff --git a/packages/combobox/docs/levenshtein.js b/packages/combobox/docs/levenshtein.js new file mode 100644 index 000000000..8c4226b9f --- /dev/null +++ b/packages/combobox/docs/levenshtein.js @@ -0,0 +1,95 @@ +/* eslint-disable*/ +// https://github.com/gustf/js-levenshtein/blob/master/index.js + +function _min(d0, d1, d2, bx, ay) { + return d0 < d1 || d2 < d1 ? (d0 > d2 ? d2 + 1 : d0 + 1) : bx === ay ? d1 : d1 + 1; +} + +export default function (a, b) { + if (a === b) { + return 0; + } + + if (a.length > b.length) { + var tmp = a; + a = b; + b = tmp; + } + + var la = a.length; + var lb = b.length; + + while (la > 0 && a.charCodeAt(la - 1) === b.charCodeAt(lb - 1)) { + la--; + lb--; + } + + var offset = 0; + + while (offset < la && a.charCodeAt(offset) === b.charCodeAt(offset)) { + offset++; + } + + la -= offset; + lb -= offset; + + if (la === 0 || lb < 3) { + return lb; + } + + var x = 0; + var y; + var d0; + var d1; + var d2; + var d3; + var dd; + var dy; + var ay; + var bx0; + var bx1; + var bx2; + var bx3; + + var vector = []; + + for (y = 0; y < la; y++) { + vector.push(y + 1); + vector.push(a.charCodeAt(offset + y)); + } + + var len = vector.length - 1; + + for (; x < lb - 3; ) { + bx0 = b.charCodeAt(offset + (d0 = x)); + bx1 = b.charCodeAt(offset + (d1 = x + 1)); + bx2 = b.charCodeAt(offset + (d2 = x + 2)); + bx3 = b.charCodeAt(offset + (d3 = x + 3)); + dd = x += 4; + for (y = 0; y < len; y += 2) { + dy = vector[y]; + ay = vector[y + 1]; + d0 = _min(dy, d0, d1, bx0, ay); + d1 = _min(d0, d1, d2, bx1, ay); + d2 = _min(d1, d2, d3, bx2, ay); + dd = _min(d2, d3, dd, bx3, ay); + vector[y] = dd; + d3 = d2; + d2 = d1; + d1 = d0; + d0 = dy; + } + } + + for (; x < lb; ) { + bx0 = b.charCodeAt(offset + (d0 = x)); + dd = ++x; + for (y = 0; y < len; y += 2) { + dy = vector[y]; + vector[y] = dd = _min(dy, d0, dd, bx0, vector[y + 1]); + d0 = dy; + } + } + + return dd; +} diff --git a/packages/combobox/src/LionCombobox.js b/packages/combobox/src/LionCombobox.js index e2c5f3bd5..4faff9625 100644 --- a/packages/combobox/src/LionCombobox.js +++ b/packages/combobox/src/LionCombobox.js @@ -246,6 +246,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { this._listboxReceivesNoFocus = true; this.__prevCboxValueNonSelected = ''; + this.__prevCboxValue = ''; /** @type {EventListener} */ this.__showOverlay = this.__showOverlay.bind(this); @@ -307,27 +308,27 @@ export class LionCombobox extends OverlayMixin(LionListbox) { this.__shouldAutocompleteNextUpdate ) { // Only update list in render cycle - this._handleAutocompletion({ - curValue: this._inputNode.value, - prevValue: this.__prevCboxValueNonSelected, - }); + this._handleAutocompletion(); this.__shouldAutocompleteNextUpdate = false; } - if (this._selectionDisplayNode) { + if (typeof this._selectionDisplayNode?.onComboboxElementUpdated === 'function') { this._selectionDisplayNode.onComboboxElementUpdated(changedProperties); } } /** + * When the preconfigurable `match-mode` conditions are not sufficient, + * one can define a custom matching function. + * * @overridable * @param {LionOption} option - * @param {string} curValue current ._inputNode value + * @param {string} textboxValue current ._inputNode value */ - filterOptionCondition(option, curValue) { + matchCondition(option, textboxValue) { let idx = -1; - if (typeof option.choiceValue === 'string' && typeof curValue === 'string') { - idx = option.choiceValue.toLowerCase().indexOf(curValue.toLowerCase()); + if (typeof option.choiceValue === 'string' && typeof textboxValue === 'string') { + idx = option.choiceValue.toLowerCase().indexOf(textboxValue.toLowerCase()); } if (this.matchMode === 'all') { @@ -353,6 +354,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { if (ev.key === 'Tab') { this.opened = false; } + this.__hasSelection = this._inputNode.value.length !== this._inputNode.selectionStart; } /** @@ -424,9 +426,16 @@ export class LionCombobox extends OverlayMixin(LionListbox) { option.style.display = 'none'; } + /** + * + * @overridable whether a user int + */ _computeUserIntendsAutoFill({ prevValue, curValue }) { const userIsAddingChars = prevValue.length < curValue.length; - const userStartsNewWord = prevValue.length && curValue.length && prevValue[0] !== curValue[0]; + const userStartsNewWord = + prevValue.length && + curValue.length && + prevValue[0].toLowerCase() !== curValue[0].toLowerCase(); return userIsAddingChars || userStartsNewWord; } @@ -434,15 +443,15 @@ export class LionCombobox extends OverlayMixin(LionListbox) { /** * Matches visibility of listbox options against current ._inputNode contents - * @param {object} config - * @param {string} config.curValue current ._inputNode value - * @param {string} config.prevValue previous ._inputNode value */ - _handleAutocompletion({ curValue, prevValue }) { + _handleAutocompletion() { if (this.autocomplete === 'none') { return; } + const curValue = this._inputNode.value; + const prevValue = this.__hasSelection ? this.__prevCboxValueNonSelected : this.__prevCboxValue; + /** * The filtered list of options that will match in this autocompletion cycle * @type {LionOption[]} @@ -454,8 +463,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { /** @typedef {LionOption & { onFilterUnmatch?:function, onFilterMatch?:function }} OptionWithFilterFn */ this.formElements.forEach((/** @type {OptionWithFilterFn} */ option, i) => { - const show = - this.autocomplete === 'inline' ? true : this.filterOptionCondition(option, curValue); + const show = this.autocomplete === 'inline' ? true : this.matchCondition(option, curValue); // [1]. Synchronize ._inputNode value and active descendant with closest match if (isAutoFillCandidate) { @@ -515,8 +523,11 @@ export class LionCombobox extends OverlayMixin(LionListbox) { option.removeAttribute('aria-hidden'); }); /** @type {number} */ - const { selectionStart } = this._inputNode; - this.__prevCboxValueNonSelected = curValue.slice(0, selectionStart); + + this.__prevCboxValueNonSelected = curValue; + // See test "computation of "user intends autofill" works correctly afer autofill" + this.__prevCboxValue = this._inputNode.value; + this.__hasSelection = hasAutoFilled; if (this._overlayCtrl && this._overlayCtrl._popper) { this._overlayCtrl._popper.update(); @@ -600,10 +611,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { } __initFilterListbox() { - this._handleAutocompletion({ - curValue: this._inputNode.value, - prevValue: this.__prevCboxValueNonSelected, - }); + this._handleAutocompletion(); } __setComboboxDisabledAndReadOnly() { diff --git a/packages/combobox/test/lion-combobox.test.js b/packages/combobox/test/lion-combobox.test.js index 53070edad..e46bf7f02 100644 --- a/packages/combobox/test/lion-combobox.test.js +++ b/packages/combobox/test/lion-combobox.test.js @@ -21,6 +21,45 @@ function mimicUserTyping(el, value) { el._overlayInvokerNode.dispatchEvent(new Event('keydown')); } +/** + * @param {LionCombobox} el + * @param {string[]} value + */ +async function mimicUserTypingAdvanced(el, values) { + const inputNode = el._inputNode; + inputNode.dispatchEvent(new Event('focusin', { bubbles: true })); + + let hasSelection = inputNode.selectionStart !== inputNode.selectionEnd; + + for (const key of values) { + // eslint-disable-next-line no-await-in-loop, no-loop-func + await new Promise(resolve => { + setTimeout(() => { + if (key === 'Backspace') { + if (hasSelection) { + inputNode.value = + inputNode.value.slice(0, inputNode.selectionStart) + + inputNode.value.slice(inputNode.selectionEnd, inputNode.value.length); + } else { + inputNode.value = inputNode.value.slice(0, -1); + } + } else if (hasSelection) { + inputNode.value = + inputNode.value.slice(0, inputNode.selectionStart) + + key + + inputNode.value.slice(inputNode.selectionEnd, inputNode.value.length); + } else { + inputNode.value += key; + } + hasSelection = false; + inputNode.dispatchEvent(new KeyboardEvent('keydown', { key })); + el._inputNode.dispatchEvent(new Event('input', { bubbles: true, composed: true })); + resolve(); + }); + }); + } +} + /** * @param {LionCombobox} el */ @@ -589,7 +628,7 @@ describe('lion-combobox', () => { expect(el._inputNode.selectionStart).to.equal('ch'.length); expect(el._inputNode.selectionEnd).to.equal('Chard'.length); - mimicUserTyping(el, 'chic'); + await mimicUserTypingAdvanced(el, ['i', 'c']); await el.updateComplete; expect(el._inputNode.value).to.equal('Chicory'); expect(el._inputNode.selectionStart).to.equal('chic'.length); @@ -599,8 +638,8 @@ describe('lion-combobox', () => { mimicUserTyping(el, 'ch'); await el.updateComplete; expect(el._inputNode.value).to.equal('ch'); - expect(el._inputNode.selectionStart).to.equal(2); - expect(el._inputNode.selectionEnd).to.equal(2); + expect(el._inputNode.selectionStart).to.equal('ch'.length); + expect(el._inputNode.selectionEnd).to.equal('ch'.length); }); it('does autocompletion when adding chars', async () => { @@ -613,20 +652,20 @@ describe('lion-combobox', () => { `)); - mimicUserTyping(el, 'ch'); - await el.updateComplete; + mimicUserTyping(el, 'ch'); // ch + await el.updateComplete; // Ch[ard] expect(el.activeIndex).to.equal(1); expect(el.checkedIndex).to.equal(1); - mimicUserTyping(el, 'chic'); - await el.updateComplete; + await mimicUserTypingAdvanced(el, ['i', 'c']); // Chic + await el.updateComplete; // Chic[ory] expect(el.activeIndex).to.equal(2); expect(el.checkedIndex).to.equal(2); - mimicUserTyping(el, 'ch'); - await el.updateComplete; - expect(el.activeIndex).to.equal(2); - expect(el.checkedIndex).to.equal(-1); + await mimicUserTypingAdvanced(el, ['Backspace', 'Backspace', 'Backspace', 'Backspace', 'h']); // Ch + await el.updateComplete; // Ch[ard] + expect(el.activeIndex).to.equal(1); + expect(el.checkedIndex).to.equal(1); }); it('does autocompletion when changing the word', async () => { @@ -644,7 +683,7 @@ describe('lion-combobox', () => { expect(el.activeIndex).to.equal(1); expect(el.checkedIndex).to.equal(1); - mimicUserTyping(el, 'chic'); + await mimicUserTypingAdvanced(el, 'ic'.split('')); await el.updateComplete; expect(el.activeIndex).to.equal(2); expect(el.checkedIndex).to.equal(2); @@ -656,6 +695,31 @@ describe('lion-combobox', () => { expect(el.checkedIndex).to.equal(0); }); + it('computation of "user intends autofill" works correctly afer autofill', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); + + mimicUserTyping(el, 'ch'); + await el.updateComplete; + expect(el._inputNode.value).to.equal('Chard'); + expect(el._inputNode.selectionStart).to.equal('ch'.length); + expect(el._inputNode.selectionEnd).to.equal('Chard'.length); + + // Autocompletion happened. When we go backwards ('Char'), we should not + // autocomplete to 'Chard' anymore. + mimicUserTyping(el, 'Char'); + await el.updateComplete; + expect(el._inputNode.value).to.equal('Char'); // so not 'Chard' + expect(el._inputNode.selectionStart).to.equal('Char'.length); + expect(el._inputNode.selectionEnd).to.equal('Char'.length); + }); + it('highlights matching options', async () => { const el = /** @type {LionCombobox} */ (await fixture(html` @@ -1008,7 +1072,7 @@ describe('lion-combobox', () => { function onlyExactMatches(option, curValue) { return option.value === curValue; } - el.filterOptionCondition = onlyExactMatches; + el.matchCondition = onlyExactMatches; mimicUserTyping(/** @type {LionCombobox} */ (el), 'Chicory'); await el.updateComplete; expect(getFilteredOptionValues(/** @type {LionCombobox} */ (el))).to.eql(['Chicory']);