Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Added support to multiselect and requireOptionMatch=false at the same time for lion-combobox #2108

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e75764c
fix(lion-radio-group): Resetting a group containing options with form…
GuiAmPm Oct 17, 2023
7cf6257
refactor: update test name
GuiAmPm Oct 17, 2023
dc8689d
Merge branch 'ing-bank:master' into master
GuiAmPm Oct 21, 2023
4189606
feature: Added support to multiselect and require option=false at the…
GuiAmPm Oct 21, 2023
f2cd4fb
Refactor: remove leftover TODO comments
GuiAmPm Oct 21, 2023
d3ca876
Refactor: fix lint issues
GuiAmPm Oct 21, 2023
aa73750
Refactor: Renamed mixin, cleanup
GuiAmPm Oct 24, 2023
961aab5
Refactor: Fixed suite test name
GuiAmPm Oct 24, 2023
7d65c70
Fix: Removed redundant mixin
GuiAmPm Oct 24, 2023
6f46f00
Refactor: Split unit tests, removed escape unexpected behaviour
GuiAmPm Oct 24, 2023
b0e4a11
Refactor: Renamed misleading variable name, updated suite file name
GuiAmPm Oct 24, 2023
7318e38
Refactor: Fix lint issue
GuiAmPm Oct 24, 2023
b3d40a8
Refactor: Include LionCombobox integration tests for CustomChoiceGrou…
GuiAmPm Oct 24, 2023
9344eca
Refactor: fix linting issues I introduced
GuiAmPm Oct 24, 2023
5a5b5f1
Fix: fixes single-choice, requireOptionMatch=false to not clear selec…
GuiAmPm Oct 25, 2023
5175497
Refactor: Added tests to empty inputs
GuiAmPm Oct 25, 2023
40a4859
Merge branch 'ing-bank:master' into master
GuiAmPm Oct 27, 2023
1b3722f
Merge branch 'master' into feature-multiselect-multiselect-and-requir…
GuiAmPm Oct 27, 2023
cb73b81
Refactor: Apply suggestions from code review
GuiAmPm Nov 1, 2023
1560d9c
Refactor: Cleanup based on feedback
GuiAmPm Nov 2, 2023
ae14266
Refactoring: Removed unnecessary update override
GuiAmPm Nov 6, 2023
24ffefd
chore: do not expose CustomChoiceGroupMixin via public entrypoints (i…
tlouisse Nov 8, 2023
3a09ed2
chore: fix lint
tlouisse Nov 8, 2023
ee4ea16
chore: fix lint
tlouisse Nov 8, 2023
55fcb79
chore: fix lint
tlouisse Nov 8, 2023
5cdc944
Update CustomChoiceGroupMixin.test.js
tlouisse Nov 8, 2023
8d64882
Update packages/ui/components/combobox/src/LionCombobox.js
tlouisse Nov 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/dirty-emus-exercise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': minor
---

Fix: fixes single-choice, requireOptionMatch=false to not clear selection
6 changes: 6 additions & 0 deletions .changeset/ten-maps-tap.md
Original file line number Diff line number Diff line change
@@ -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
43 changes: 8 additions & 35 deletions docs/components/combobox/src/demo-selection-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class DemoSelectionDisplay extends LitElement {
* Can be used to visually indicate the next
*/
removeChipOnNextBackspace: Boolean,
selectedElements: Array,
selectedChoices: Array,
};
}

Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

/**
Expand All @@ -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>
`;
}
Expand All @@ -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);
})}
Expand All @@ -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;
}
Expand Down
23 changes: 23 additions & 0 deletions docs/components/combobox/use-cases.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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.
Expand Down
99 changes: 73 additions & 26 deletions packages/ui/components/combobox/src/LionCombobox.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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 {
Expand All @@ -43,6 +44,10 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(LionListbox)) {
requireOptionMatch: {
type: Boolean,
},
allowCustomChoice: {
tlouisse marked this conversation as resolved.
Show resolved Hide resolved
type: Boolean,
attribute: 'allow-custom-choice',
},
__shouldAutocompleteNextUpdate: Boolean,
};
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
/**
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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).",
);
}
}
}

/**
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -1065,25 +1083,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;
}
}
}

Expand All @@ -1110,12 +1155,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?
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
@@ -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',
});
Loading