ayo/lion
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(combobox): do not reopen on focusin edge cases

Browse source
This commit is contained in:
Thijs Louisse 2021-04-17 00:00:32 +02:00
parent 6aa7fc29c8
commit 351e9598fe
3 changed files with 165 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/cold-papayas-judge.md Normal file
View file

@ -0,0 +1,5 @@
---
'@lion/combobox': patch
---
do not reopen listbox on focusin edge cases

58
packages/combobox/src/LionCombobox.js
View file

@ -307,7 +307,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
} }
/** /**
* @param {'disabled'|'modelValue'|'readOnly'} name * @param {'disabled'|'modelValue'|'readOnly'|'focused'} name
* @param {unknown} oldValue * @param {unknown} oldValue
*/ */
requestUpdateInternal(name, oldValue) { requestUpdateInternal(name, oldValue) {
@ -324,6 +324,9 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
} }
} }
} }
if (name === 'focused' && this.focused) {
this.__requestShowOverlay();
}
} }
/** /**
@ -331,11 +334,6 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
*/ */
updated(changedProperties) { updated(changedProperties) {
super.updated(changedProperties); super.updated(changedProperties);
if (changedProperties.has('focused')) {
if (this.focused) {
this.__requestShowOverlay();
}
}
if (changedProperties.has('opened')) { if (changedProperties.has('opened')) {
if (this.opened) { if (this.opened) {
@ -413,12 +411,18 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
* return options.currentValue.length > 4 && super._showOverlayCondition(options); * return options.currentValue.length > 4 && super._showOverlayCondition(options);
* } * }
* *
* @param {{ currentValue: string, lastKey:string|undefined }} options * @param {{ currentValue?: string, lastKey?: string }} options
* @protected * @protected
* @returns {boolean} * @returns {boolean}
*/ */
// TODO: batch all pending condition triggers in __pendingShowTriggers, reducing race conditions
// eslint-disable-next-line class-methods-use-this // eslint-disable-next-line class-methods-use-this
_showOverlayCondition({ lastKey }) { _showOverlayCondition({ lastKey }) {
const hideOn = ['Tab', 'Escape', 'Enter'];
if (lastKey && hideOn.includes(lastKey)) {
return false;
}
if (this.showAllOnEmpty && this.focused) { if (this.showAllOnEmpty && this.focused) {
return true; return true;
} }
@ -426,8 +430,8 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
if (!lastKey) { if (!lastKey) {
return /** @type {boolean} */ (this.opened); return /** @type {boolean} */ (this.opened);
} }
const doNotShowOn = ['Tab', 'Esc', 'Enter'];
return !doNotShowOn.includes(lastKey); return true;
} }
/** /**
@ -449,11 +453,8 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
* @param {KeyboardEvent} ev * @param {KeyboardEvent} ev
* @protected * @protected
*/ */
_textboxOnKeydown(ev) { // eslint-disable-next-line class-methods-use-this, no-unused-vars
if (ev.key === 'Tab') { _textboxOnKeydown(ev) {}
this.opened = false;
}
}
/** /**
* @param {MouseEvent} ev * @param {MouseEvent} ev
@ -461,11 +462,12 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
*/ */
_listboxOnClick(ev) { _listboxOnClick(ev) {
super._listboxOnClick(ev); super._listboxOnClick(ev);
this._inputNode.focus();
if (!this.multipleChoice) { if (!this.multipleChoice) {
this.activeIndex = -1; this.activeIndex = -1;
this.opened = false; this._setOpenedWithoutPropertyEffects(false);
} }
this._inputNode.focus();
} }
/** /**
@ -756,6 +758,15 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
this.__setupCombobox(); this.__setupCombobox();
} }
/**
* @enhance OverlayMixin
* @protected
*/
_teardownOverlayCtrl() {
super._teardownOverlayCtrl();
this.__teardownCombobox();
}
/** /**
* @enhance OverlayMixin * @enhance OverlayMixin
* @protected * @protected
@ -784,7 +795,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
const { key } = ev; const { key } = ev;
switch (key) { switch (key) {
case 'Escape': case 'Escape':
this.opened = false; this._setOpenedWithoutPropertyEffects(false);
this._setTextboxValue(''); this._setTextboxValue('');
break; break;
case 'Enter': case 'Enter':
@ -792,7 +803,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
return; return;
} }
if (!this.multipleChoice) { if (!this.multipleChoice) {
this.opened = false; this._setOpenedWithoutPropertyEffects(false);
} }
break; break;
/* no default */ /* no default */
@ -891,10 +902,13 @@ export class LionCombobox extends OverlayMixin(LionListbox) {
* @private * @private
*/ */
__requestShowOverlay(ev) { __requestShowOverlay(ev) {
this.opened = this._showOverlayCondition({ const lastKey = ev && ev.key;
lastKey: ev && ev.key, this._setOpenedWithoutPropertyEffects(
currentValue: this._inputNode.value, this._showOverlayCondition({
}); lastKey,
currentValue: this._inputNode.value,
}),
);
} }
clear() { clear() {

161
packages/combobox/test/lion-combobox.test.js
View file

@ -57,7 +57,7 @@ function mimicUserTyping(el, value) {
} }
/** /**
* @param {HTMLInputElement} el * @param {HTMLElement} el
* @param {string} key * @param {string} key
*/ */
function mimicKeyPress(el, key) { function mimicKeyPress(el, key) {
@ -157,41 +157,73 @@ async function fruitFixture({ autocomplete, matchMode } = {}) {
} }
describe('lion-combobox', () => { describe('lion-combobox', () => {
describe('Options', () => { describe('Options visibility', () => {
describe('showAllOnEmpty', () => { it('hides options when text in input node is cleared after typing something by default', async () => {
it('hides options when text in input node is cleared after typing something by default', async () => { const el = /** @type {LionCombobox} */ (await fixture(html`
const el = /** @type {LionCombobox} */ (await fixture(html` <lion-combobox name="foo">
<lion-combobox name="foo"> <lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option> <lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option> <lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option> <lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option>
<lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option> </lion-combobox>
</lion-combobox> `));
`));
const options = el.formElements; const options = el.formElements;
const visibleOptions = () => options.filter(o => o.getAttribute('aria-hidden') !== 'true'); const visibleOptions = () => options.filter(o => o.getAttribute('aria-hidden') !== 'true');
async function performChecks() { async function performChecks() {
mimicUserTyping(el, 'c'); mimicUserTyping(el, 'c');
await el.updateComplete; await el.updateComplete;
expect(visibleOptions().length).to.equal(4); expect(visibleOptions().length).to.equal(4);
mimicUserTyping(el, ''); mimicUserTyping(el, '');
await el.updateComplete; await el.updateComplete;
expect(visibleOptions().length).to.equal(0); expect(visibleOptions().length).to.equal(0);
} }
// FIXME: autocomplete 'none' should have this behavior as well // FIXME: autocomplete 'none' should have this behavior as well
// el.autocomplete = 'none'; // el.autocomplete = 'none';
// await performChecks(); // await performChecks();
el.autocomplete = 'list'; el.autocomplete = 'list';
await performChecks(); await performChecks();
el.autocomplete = 'inline'; el.autocomplete = 'inline';
await performChecks(); await performChecks();
el.autocomplete = 'both'; el.autocomplete = 'both';
await performChecks(); await performChecks();
}); });
it('hides listbox on click/enter (when multiple-choice is false)', async () => {
const el = /** @type {LionCombobox} */ (await fixture(html`
<lion-combobox name="foo">
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
<lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option>
</lion-combobox>
`));
const { _comboboxNode, _listboxNode } = getComboboxMembers(el);
async function open() {
// activate opened listbox
_comboboxNode.dispatchEvent(new Event('focusin', { bubbles: true, composed: true }));
mimicUserTyping(el, 'ch');
return el.updateComplete;
}
await open();
expect(el.opened).to.be.true;
const visibleOptions = el.formElements.filter(o => o.style.display !== 'none');
visibleOptions[0].click();
expect(el.opened).to.be.false;
await open();
expect(el.opened).to.be.true;
el.activeIndex = el.formElements.indexOf(visibleOptions[0]);
mimicKeyPress(_listboxNode, 'Enter');
await el.updateComplete;
expect(el.opened).to.be.false;
});
describe('With ".showAllOnEmpty"', () => {
it('keeps showing options when text in input node is cleared after typing something', async () => { it('keeps showing options when text in input node is cleared after typing something', async () => {
const el = /** @type {LionCombobox} */ (await fixture(html` const el = /** @type {LionCombobox} */ (await fixture(html`
<lion-combobox name="foo" autocomplete="list" show-all-on-empty> <lion-combobox name="foo" autocomplete="list" show-all-on-empty>
@ -240,6 +272,63 @@ describe('lion-combobox', () => {
await el.updateComplete; await el.updateComplete;
expect(el.opened).to.be.true; expect(el.opened).to.be.true;
}); });
it('hides overlay on focusin after [Escape]', async () => {
const el = /** @type {LionCombobox} */ (await fixture(html`
<lion-combobox name="foo" .showAllOnEmpty="${true}">
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
<lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option>
</lion-combobox>
`));
const { _comboboxNode, _inputNode } = getComboboxMembers(el);
expect(el.opened).to.be.false;
_comboboxNode.dispatchEvent(new Event('focusin', { bubbles: true, composed: true }));
await el.updateComplete;
expect(el.opened).to.be.true;
_inputNode.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' }));
await el.updateComplete;
expect(el.opened).to.be.false;
});
it('hides listbox on click/enter (when multiple-choice is false)', async () => {
const el = /** @type {LionCombobox} */ (await fixture(html`
<lion-combobox name="foo" .showAllOnEmpty="${true}">
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
<lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option>
</lion-combobox>
`));
const { _comboboxNode, _listboxNode, _inputNode } = getComboboxMembers(el);
async function open() {
// activate opened listbox
_comboboxNode.dispatchEvent(new Event('focusin', { bubbles: true, composed: true }));
mimicUserTyping(el, 'ch');
return el.updateComplete;
}
await open();
expect(el.opened).to.be.true;
const visibleOptions = el.formElements.filter(o => o.style.display !== 'none');
visibleOptions[0].click();
await el.updateComplete;
expect(el.opened).to.be.false;
_inputNode.value = '';
_inputNode.blur();
await open();
await el.updateComplete;
expect(el.opened).to.be.true;
el.activeIndex = el.formElements.indexOf(visibleOptions[0]);
mimicKeyPress(_listboxNode, 'Enter');
expect(el.opened).to.be.false;
});
}); });
}); });
@ -1766,7 +1855,7 @@ describe('lion-combobox', () => {
describe('Multiple Choice', () => { describe('Multiple Choice', () => {
// TODO: possibly later share test with select-rich if it officially supports multipleChoice // TODO: possibly later share test with select-rich if it officially supports multipleChoice
it('does not close listbox on click/enter/space', async () => { it('does not close listbox on click/enter', async () => {
const el = /** @type {LionCombobox} */ (await fixture(html` const el = /** @type {LionCombobox} */ (await fixture(html`
<lion-combobox name="foo" multiple-choice> <lion-combobox name="foo" multiple-choice>
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option> <lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
@ -1787,10 +1876,8 @@ describe('lion-combobox', () => {
const visibleOptions = el.formElements.filter(o => o.style.display !== 'none'); const visibleOptions = el.formElements.filter(o => o.style.display !== 'none');
visibleOptions[0].click(); visibleOptions[0].click();
expect(el.opened).to.equal(true); expect(el.opened).to.equal(true);
// visibleOptions[1].dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); mimicKeyPress(visibleOptions[1], 'Enter');
// expect(el.opened).to.equal(true); expect(el.opened).to.equal(true);
// visibleOptions[2].dispatchEvent(new KeyboardEvent('keyup', { key: ' ' }));
// expect(el.opened).to.equal(true);
}); });
}); });