Skip to content

Commit

Permalink
fix: do not clear on Esc with clear button visible when readonly (#8455
Browse files Browse the repository at this point in the history
…) (#8460)

Co-authored-by: Serhii Kulykov <[email protected]>
  • Loading branch information
vaadin-bot and web-padawan authored Jan 9, 2025
1 parent 52c9ea3 commit 3839b05
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 15 deletions.
23 changes: 10 additions & 13 deletions packages/combo-box/src/vaadin-combo-box-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -858,19 +858,16 @@ export const ComboBoxMixin = (subclass) =>
* @override
*/
_onEscape(e) {
if (this.autoOpenDisabled) {
if (
this.autoOpenDisabled &&
(this.opened || (this.value !== this._inputElementValue && this._inputElementValue.length > 0))
) {
// Auto-open is disabled
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 && !!this.value) {
e.stopPropagation();
// The clear button is visible and the overlay is closed, so clear the value.
this._onClearAction();
}
// 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.opened) {
// Auto-open is enabled
// The overlay is open
Expand All @@ -884,7 +881,7 @@ export const ComboBoxMixin = (subclass) =>
// No item is focused, cancel the change and close the overlay
this.cancel();
}
} else if (this.clearButtonVisible && !!this.value) {
} else if (this.clearButtonVisible && !!this.value && !this.readonly) {
e.stopPropagation();
// The clear button is visible and the overlay is closed, so clear the value.
this._onClearAction();
Expand Down
8 changes: 8 additions & 0 deletions packages/combo-box/test/keyboard.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,14 @@ describe('keyboard', () => {
expect(comboBox.value).to.equal('bar');
});

it('should not clear the value on esc when readonly', () => {
comboBox.value = 'bar';
comboBox.clearButtonVisible = true;
comboBox.readonly = true;
escKeyDown(input);
expect(comboBox.value).to.equal('bar');
});

it('should not propagate when input value is not empty', async () => {
await sendKeys({ type: 'foo' });

Expand Down
2 changes: 1 addition & 1 deletion packages/date-picker/src/vaadin-date-picker-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,7 @@ export const DatePickerMixin = (subclass) =>
return;
}

if (this.clearButtonVisible && !!this.value) {
if (this.clearButtonVisible && !!this.value && !this.readonly) {
// Stop event from propagating to the host element
// to avoid closing dialog when clearing on Esc
event.stopPropagation();
Expand Down
17 changes: 17 additions & 0 deletions packages/date-picker/test/basic.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,23 @@ describe('clear button', () => {
expect(spy.called).to.be.false;
});

it('should not clear value on Esc when readonly is true', () => {
datePicker.value = '2000-02-01';
datePicker.readonly = true;
const event = keyboardEventFor('keydown', 27, [], 'Escape');
const spy = sinon.spy(event, 'stopPropagation');
datePicker.inputElement.dispatchEvent(event);
expect(spy.called).to.be.false;
});

it('should not stop propagation on Esc when readonly is true', () => {
datePicker.value = '2000-02-01';
datePicker.readonly = true;
const event = keyboardEventFor('keydown', 27, [], 'Escape');
datePicker.inputElement.dispatchEvent(event);
expect(datePicker.value).to.equal('2000-02-01');
});

it('should not close on clear button click when opened', async () => {
await open(datePicker);
datePicker.value = '2001-01-01';
Expand Down
2 changes: 1 addition & 1 deletion packages/field-base/src/clear-button-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const ClearButtonMixin = (superclass) =>
_onEscape(event) {
super._onEscape(event);

if (this.clearButtonVisible && !!this.value) {
if (this.clearButtonVisible && !!this.value && !this.readonly) {
event.stopPropagation();
this._onClearAction();
}
Expand Down
7 changes: 7 additions & 0 deletions packages/field-base/test/clear-button-mixin.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,13 @@ const runTests = (defineHelper, baseMixin) => {
expect(input.value).to.equal('foo');
});

it('should not clear value on Esc when readonly is true', () => {
element.clearButtonVisible = true;
element.readonly = true;
escKeyDown(clearButton);
expect(input.value).to.equal('foo');
});

it('should dispatch input event when clearing value on Esc', () => {
const spy = sinon.spy();
input.addEventListener('input', spy);
Expand Down
8 changes: 8 additions & 0 deletions packages/multi-select-combo-box/test/basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,14 @@ describe('basic', () => {
expect(comboBox.selectedItems).to.deep.equal(['apple', 'orange']);
});

it('should not clear selected items on Esc when readonly', async () => {
comboBox.selectedItems = ['apple', 'orange'];
comboBox.readonly = true;
inputElement.focus();
await sendKeys({ press: 'Escape' });
expect(comboBox.selectedItems).to.deep.equal(['apple', 'orange']);
});

it('should prevent default for touchend event on clear button', () => {
comboBox.selectedItems = ['apple', 'orange'];
const event = new CustomEvent('touchend', { cancelable: true });
Expand Down

0 comments on commit 3839b05

Please sign in to comment.