Skip to content

Commit

Permalink
fix: stop Escape keydown propagation if the event was handled (#3039) (
Browse files Browse the repository at this point in the history
…#3046)

Co-authored-by: David Sosa <[email protected]>
  • Loading branch information
vaadin-bot and sosa-vaadin authored Nov 15, 2021
1 parent 1ef32f7 commit a225a07
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 4 deletions.
10 changes: 7 additions & 3 deletions packages/combo-box/src/vaadin-combo-box-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -569,11 +569,14 @@ export const ComboBoxMixin = (subclass) =>
_onEscape(e) {
if (this.autoOpenDisabled) {
// Auto-open is disabled
if (this.value !== this._inputElementValue) {
if (this.opened || (this.value !== this._inputElementValue && this._inputElementValue.length > 0)) {
// The overlay is open or
// The input value has changed but the change hasn't been committed, so cancel it.
e.stopPropagation();
this._focusedIndex = -1;
this.cancel();
} else if (this.clearButtonVisible && !this.opened) {
} else if (this.clearButtonVisible && !this.opened && !!this.value) {
e.stopPropagation();
// The clear button is visible and the overlay is closed, so clear the value.
this._clear();
}
Expand All @@ -591,7 +594,8 @@ export const ComboBoxMixin = (subclass) =>
// No item is focused, cancel the change and close the overlay
this.cancel();
}
} else if (this.clearButtonVisible) {
} else if (this.clearButtonVisible && !!this.value) {
e.stopPropagation();
// The clear button is visible and the overlay is closed, so clear the value.
this._clear();
}
Expand Down
29 changes: 28 additions & 1 deletion packages/combo-box/test/keyboard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
fire,
fixtureSync,
focusout,
keyboardEventFor,
keyDownOn,
nextFrame
} from '@vaadin/testing-helpers';
Expand Down Expand Up @@ -69,6 +70,23 @@ describe('keyboard', () => {

expect(getFocusedIndex()).to.equal(0);
});

it('should propagate escape key event if dropdown is closed', () => {
const event = keyboardEventFor('keydown', 27, [], 'Escape');
const keyDownSpy = sinon.spy(event, 'stopPropagation');
input.dispatchEvent(event);
expect(keyDownSpy.called).to.be.false;
});

it('should not propagate esc keydown event when overlay is closed, clear button is visible and value is not empty', () => {
comboBox.value = 'bar';
comboBox.clearButtonVisible = true;

const event = keyboardEventFor('keydown', 27, [], 'Escape');
const keyDownSpy = sinon.spy(event, 'stopPropagation');
input.dispatchEvent(event);
expect(keyDownSpy.called).to.be.true;
});
});

describe('navigating after overlay opened', () => {
Expand Down Expand Up @@ -231,7 +249,7 @@ describe('keyboard', () => {
expect(input.value).to.equal('foobar');
});

it('should revert to the custom value after keyboar navigation', () => {
it('should revert to the custom value after keyboard navigation', () => {
comboBox.allowCustomValue = true;
comboBox.value = 'foobar';
arrowDownKeyDown(input);
Expand Down Expand Up @@ -557,5 +575,14 @@ describe('keyboard', () => {
escKeyDown(input);
expect(comboBox.value).to.equal('bar');
});

it('should not propagate when input value is not empty', () => {
inputText('foo');

const event = keyboardEventFor('keydown', 27, [], 'Escape');
const keyDownSpy = sinon.spy(event, 'stopPropagation');
input.dispatchEvent(event);
expect(keyDownSpy.called).to.be.true;
});
});
});

0 comments on commit a225a07

Please sign in to comment.