Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not clear on Esc with clear button visible when readonly (#8455) (CP: 24.6) #8458

Merged
merged 1 commit into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -877,19 +877,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 @@ -903,7 +900,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 @@ -1160,7 +1160,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.common.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