Skip to content

Commit

Permalink
fix!: align how clear button works with combo-box behavior (#6100)
Browse files Browse the repository at this point in the history
  • Loading branch information
DiegoCardoso committed Jul 7, 2023
1 parent 1dacbff commit 5878ce1
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 5 deletions.
14 changes: 13 additions & 1 deletion packages/field-base/src/clear-button-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Copyright (c) 2021 - 2023 Vaadin Ltd.
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { isTouch } from '@vaadin/component-base/src/browser-utils.js';
import { KeyboardMixin } from '@vaadin/component-base/src/keyboard-mixin.js';
import { InputMixin } from './input-mixin.js';

Expand Down Expand Up @@ -51,6 +52,7 @@ export const ClearButtonMixin = (superclass) =>
super.ready();

if (this.clearElement) {
this.clearElement.addEventListener('mousedown', (event) => this._onClearButtonMouseDown(event));
this.clearElement.addEventListener('click', (event) => this._onClearButtonClick(event));
}
}
Expand All @@ -61,10 +63,20 @@ export const ClearButtonMixin = (superclass) =>
*/
_onClearButtonClick(event) {
event.preventDefault();
this.inputElement.focus();
this._onClearAction();
}

/**
* @param {MouseEvent} event
* @protected
*/
_onClearButtonMouseDown(event) {
event.preventDefault();
if (!isTouch) {
this.inputElement.focus();
}
}

/**
* Override an event listener inherited from `KeydownMixin` to clear on Esc.
* Components that extend this mixin can prevent this behavior by overriding
Expand Down
24 changes: 22 additions & 2 deletions packages/field-base/test/clear-button-mixin.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import {
fire,
fixtureSync,
keyboardEventFor,
mousedown,
nextFrame,
nextRender,
} from '@vaadin/testing-helpers';
import { sendKeys } from '@web/test-runner-commands';
import sinon from 'sinon';
import { isTouch } from '@vaadin/component-base/src/browser-utils.js';
import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
import { ClearButtonMixin } from '../src/clear-button-mixin.js';
Expand Down Expand Up @@ -63,12 +65,30 @@ const runTests = (defineHelper, baseMixin) => {
expect(input.value).to.equal('');
});

it('should focus the input on clear button click', () => {
(!isTouch ? it : it.skip)('should focus the input on clear button mousedown', () => {
const spy = sinon.spy(input, 'focus');
clearButton.click();
mousedown(clearButton);
expect(spy.calledOnce).to.be.true;
});

it('should prevent default on clear button mousedown', () => {
const event = new CustomEvent('mousedown', { cancelable: true });
clearButton.dispatchEvent(event);
expect(event.defaultPrevented).to.be.true;
});

(isTouch ? it : it.skip)('should not focus the input on clear button touch', () => {
const spy = sinon.spy(input, 'focus');
mousedown(clearButton);
expect(spy.called).to.be.false;
});

(isTouch ? it : it.skip)('should keep focus at the input on clear button touch', () => {
input.focus();
mousedown(clearButton);
expect(document.activeElement).to.be.equal(input);
});

it('should dispatch input event on clear button click', () => {
const spy = sinon.spy();
input.addEventListener('input', spy);
Expand Down
6 changes: 4 additions & 2 deletions packages/field-base/test/input-control-mixin.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import {
fixtureSync,
keyboardEventFor,
keyDownOn,
mousedown,
nextFrame,
nextRender,
} from '@vaadin/testing-helpers';
import sinon from 'sinon';
import { isTouch } from '@vaadin/component-base/src/browser-utils.js';
import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
import { InputControlMixin } from '../src/input-control-mixin.js';
Expand Down Expand Up @@ -74,9 +76,9 @@ const runTests = (defineHelper, baseMixin) => {
expect(input.value).to.equal('');
});

it('should focus the input on clear button click', () => {
(!isTouch ? it : it.skip)('should focus the input on clear clearButton mousedown', () => {
const spy = sinon.spy(input, 'focus');
button.click();
mousedown(button);
expect(spy.calledOnce).to.be.true;
});

Expand Down

0 comments on commit 5878ce1

Please sign in to comment.