Skip to content

Commit

Permalink
fix(combobox): update the code to when show and hide the overlay, to …
Browse files Browse the repository at this point in the history
…be more robust (#2301)
  • Loading branch information
gerjanvangeest committed Jun 10, 2024
1 parent 57597bb commit b50b960
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 88 deletions.
5 changes: 5 additions & 0 deletions .changeset/wet-eggs-reply.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

[combobox] update the code to when show and hide the overlay, to be more robust
57 changes: 35 additions & 22 deletions packages/ui/components/combobox/src/LionCombobox.js
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,14 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
*/
this.__listboxContentChanged = false;

/** @type {EventListener}
* @protected
*/
this._onKeyUp = this._onKeyUp.bind(this);
/** @type {EventListener}
* @private
*/
this.__requestShowOverlay = this.__requestShowOverlay.bind(this);
this._textboxOnClick = this._textboxOnClick.bind(this);
/** @type {EventListener}
* @protected
*/
Expand Down Expand Up @@ -500,9 +504,6 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
}
}
}
if (name === 'focused' && this.focused) {
this.__requestShowOverlay();
}
}

/**
Expand Down Expand Up @@ -614,11 +615,6 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
* that wraps the listbox with options
*
* @example
* _showOverlayCondition(options) {
* return this.focused || super._showOverlayCondition(options);
* }
*
* @example
* _showOverlayCondition({ lastKey }) {
* return lastKey === 'ArrowDown';
* }
Expand All @@ -635,19 +631,20 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
// TODO: batch all pending condition triggers in __pendingShowTriggers, reducing race conditions
// eslint-disable-next-line class-methods-use-this
_showOverlayCondition({ lastKey }) {
const hideOn = ['Tab', 'Escape', 'Enter'];
if (lastKey && hideOn.includes(lastKey)) {
const alwaysHideOn = ['Tab', 'Escape'];
const notMultipleChoiceHideOn = ['Enter'];
if (
lastKey &&
(alwaysHideOn.includes(lastKey) ||
(!this.multipleChoice && notMultipleChoiceHideOn.includes(lastKey)))
) {
return false;
}

if (this.showAllOnEmpty && this.focused) {
if (this.filled || this.showAllOnEmpty) {
return true;
}
// when no keyboard action involved (on focused change), return current opened state
if (!lastKey) {
return /** @type {boolean} */ (this.opened);
}
return true;
return /** @type {boolean} */ (this.opened);
}

/**
Expand Down Expand Up @@ -678,9 +675,14 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
this.__listboxContentChanged = true;
}

/**
* @param {Event} ev
* @protected
*/
// eslint-disable-next-line no-unused-vars
_textboxOnInput() {
_textboxOnInput(ev) {
this.__shouldAutocompleteNextUpdate = true;
this.opened = true;
}

/**
Expand Down Expand Up @@ -1068,7 +1070,8 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
*/
_setupOpenCloseListeners() {
super._setupOpenCloseListeners();
this._inputNode.addEventListener('keyup', this.__requestShowOverlay);
this._inputNode.addEventListener('keyup', this._onKeyUp);
this._inputNode.addEventListener('click', this._textboxOnClick);
}

/**
Expand All @@ -1077,7 +1080,8 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
*/
_teardownOpenCloseListeners() {
super._teardownOpenCloseListeners();
this._inputNode.removeEventListener('keyup', this.__requestShowOverlay);
this._inputNode.removeEventListener('keyup', this._onKeyUp);
this._inputNode.removeEventListener('click', this._textboxOnClick);
}

/**
Expand Down Expand Up @@ -1225,16 +1229,25 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi

/**
* @param {KeyboardEvent} [ev]
* @private
* @protected
*/
__requestShowOverlay(ev) {
_onKeyUp(ev) {
const lastKey = ev && ev.key;
this.opened = this._showOverlayCondition({
lastKey,
currentValue: this._inputNode.value,
});
}

/**
* @param {FocusEvent} [ev]
* @protected
*/
// eslint-disable-next-line no-unused-vars
_textboxOnClick(ev) {
this.opened = this._showOverlayCondition({});
}

clear() {
this.value = '';
super.clear();
Expand Down
Loading

0 comments on commit b50b960

Please sign in to comment.