Skip to content

Commit

Permalink
fix: only close combo-box overlay on blur from keyboard (#7554)
Browse files Browse the repository at this point in the history
  • Loading branch information
web-padawan authored Jul 17, 2024
1 parent 01919cf commit fab9459
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 8 deletions.
21 changes: 18 additions & 3 deletions integration/tests/dialog-combo-box.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import { expect } from '@esm-bundle/chai';
import { fixtureSync, mousedown, nextFrame, nextUpdate, oneEvent, touchstart } from '@vaadin/testing-helpers';
import {
fixtureSync,
mousedown,
nextFrame,
nextUpdate,
oneEvent,
outsideClick,
touchstart,
} from '@vaadin/testing-helpers';
import { sendKeys } from '@web/test-runner-commands';
import './not-animated-styles.js';
import '@vaadin/combo-box';
Expand Down Expand Up @@ -27,20 +35,27 @@ describe('combo-box in dialog', () => {
await nextUpdate(comboBox);
});

it('should not close the dialog when closing time-picker on input element Escape', async () => {
it('should not close the dialog when closing combo-box on input element Escape', async () => {
await sendKeys({ press: 'Escape' });

expect(comboBox.opened).to.be.false;
expect(dialog.opened).to.be.true;
});

it('should close the dialog on subsequent Escape after the time-picker is closed', async () => {
it('should close the dialog on subsequent Escape after the combo-box is closed', async () => {
await sendKeys({ press: 'Escape' });

await sendKeys({ press: 'Escape' });

expect(dialog.opened).to.be.false;
});

it('should not close the dialog when closing combo-box on outside click', () => {
outsideClick();

expect(comboBox.opened).to.be.false;
expect(dialog.opened).to.be.true;
});
});

describe('clear button visible', () => {
Expand Down
16 changes: 14 additions & 2 deletions packages/combo-box/src/vaadin-combo-box-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
import { DisabledMixin } from '@vaadin/a11y-base/src/disabled-mixin.js';
import { FocusMixin } from '@vaadin/a11y-base/src/focus-mixin.js';
import { isElementFocused } from '@vaadin/a11y-base/src/focus-utils.js';
import { isElementFocused, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
import { KeyboardMixin } from '@vaadin/a11y-base/src/keyboard-mixin.js';
import { isTouch } from '@vaadin/component-base/src/browser-utils.js';
import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
Expand Down Expand Up @@ -1358,7 +1358,19 @@ export const ComboBoxMixin = (subclass) =>
return;
}

this._closeOrCommit();
if (isKeyboardActive()) {
// Close on Tab key causing blur. With mouse, close on outside click instead.
this._closeOrCommit();
return;
}

if (!this.opened) {
this._commitValue();
} else if (!this._overlayOpened) {
// Combo-box is opened, but overlay is not visible -> custom value was entered.
// Make sure we close here as there won't be an "outside click" in this case.
this.close();
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion packages/combo-box/test/interactions.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
nextRender,
nextUpdate,
outsideClick,
tabKeyDown,
tap,
touchstart,
} from '@vaadin/testing-helpers';
Expand Down Expand Up @@ -58,7 +59,8 @@ describe('interactions', () => {
expect(comboBox.opened).to.be.false;
});

it('should close the when focus is lost', () => {
it('should close when focus is lost from keyboard', () => {
tabKeyDown(input);
focusout(input);

expect(comboBox.opened).to.be.false;
Expand Down
2 changes: 0 additions & 2 deletions packages/time-picker/src/vaadin-time-picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,6 @@ class TimePicker extends PatternMixin(InputControlMixin(ThemableMixin(ElementMix
super._setFocused(focused);

if (!focused) {
this.__commitValueChange();

// Do not validate when focusout is caused by document
// losing focus, which happens on browser tab switch.
if (document.hasFocus()) {
Expand Down

0 comments on commit fab9459

Please sign in to comment.