Skip to content

Commit

Permalink
refactor!: align hasInputValue behavior across components (#8200)
Browse files Browse the repository at this point in the history
  • Loading branch information
vursen authored Nov 21, 2024
1 parent 80486ca commit 3e1f22b
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 105 deletions.
38 changes: 0 additions & 38 deletions packages/date-picker/src/vaadin-date-picker-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -357,20 +357,6 @@ export const DatePickerMixin = (subclass) =>
type: Object,
sync: true,
},

/**
* In date-picker, unlike other components extending `InputMixin`,
* the property indicates true only if the input has been entered by the user.
* In the case of programmatic changes, the property is reset to false.
* Read more about why this workaround is needed:
* https://github.com/vaadin/web-components/issues/5639
*
* @protected
* @override
*/
_hasInputValue: {
type: Boolean,
},
};
}

Expand All @@ -396,30 +382,6 @@ export const DatePickerMixin = (subclass) =>
this._boundOverlayRenderer = this._overlayRenderer.bind(this);
}

/**
* @override
* @protected
*/
get _inputElementValue() {
return super._inputElementValue;
}

/**
* The setter is overridden to reset the `_hasInputValue` property
* to false when the input element's value is updated programmatically.
* In date-picker, `_hasInputValue` is supposed to indicate true only
* if the input has been entered by the user.
* Read more about why this workaround is needed:
* https://github.com/vaadin/web-components/issues/5639
*
* @override
* @protected
*/
set _inputElementValue(value) {
super._inputElementValue = value;
this._hasInputValue = false;
}

/**
* Override a getter from `InputControlMixin` to make it optional
* and to prevent warning when a clear button is missing,
Expand Down
25 changes: 0 additions & 25 deletions packages/date-picker/test/events.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,25 +61,6 @@ describe('events', () => {
expect(datePicker.inputElement.value).to.be.empty;
expect(hasInputValueChangedSpy.calledOnce).to.be.true;
});

it('should be fired when comitting the user input with Enter', async () => {
await sendKeys({ press: 'Enter' });
expect(valueChangedSpy.calledOnce).to.be.true;
expect(hasInputValueChangedSpy.calledOnce).to.be.true;
expect(hasInputValueChangedSpy.calledBefore(valueChangedSpy)).to.be.true;
});

// FIXME: flaky test often failing in CI
it.skip('should be fired when selecting a date with Space', async () => {
// Move focus inside the dropdown to the typed date.
await sendKeys({ press: 'ArrowDown' });
await waitForScrollToFinish(datePicker._overlayContent);
// Select that date.
await sendKeys({ press: 'Space' });
expect(valueChangedSpy.calledOnce).to.be.true;
expect(hasInputValueChangedSpy.calledOnce).to.be.true;
expect(hasInputValueChangedSpy.calledBefore(valueChangedSpy)).to.be.true;
});
});
});

Expand All @@ -106,12 +87,6 @@ describe('events', () => {
expect(hasInputValueChangedSpy.calledOnce).to.be.true;
expect(hasInputValueChangedSpy.calledBefore(valueChangedSpy)).to.be.true;
});

it('should be fired when reverting the user input to the value with Esc', async () => {
await sendKeys({ press: 'Escape' });
expect(datePicker.inputElement.value).to.equal('1/1/2022');
expect(hasInputValueChangedSpy.calledOnce).to.be.true;
});
});
});
});
Expand Down
2 changes: 2 additions & 0 deletions packages/field-base/src/input-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ export const InputMixin = dedupingMixin(
if (this.inputElement) {
this.inputElement[this._inputElementValueProperty] = value;
}

this._hasInputValue = value && value.length > 0;
}

/**
Expand Down
21 changes: 0 additions & 21 deletions packages/time-picker/src/vaadin-lit-time-picker-combo-box.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,27 +61,6 @@ class TimePickerComboBox extends ComboBoxMixin(ThemableMixin(PolylitMixin(LitEle
return this.querySelector('[part="clear-button"]');
}

/**
* @override
* @protected
*/
get _inputElementValue() {
return super._inputElementValue;
}

/**
* The setter is overridden to ensure the `_hasInputValue` property
* doesn't wrongly indicate true after the input element's value
* is reverted or cleared programmatically.
*
* @override
* @protected
*/
set _inputElementValue(value) {
super._inputElementValue = value;
this._hasInputValue = value && value.length > 0;
}

/** @protected */
render() {
return html`
Expand Down
21 changes: 0 additions & 21 deletions packages/time-picker/src/vaadin-time-picker-combo-box.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,27 +73,6 @@ class TimePickerComboBox extends ComboBoxMixin(ThemableMixin(PolymerElement)) {
return this.querySelector('[part="clear-button"]');
}

/**
* @override
* @protected
*/
get _inputElementValue() {
return super._inputElementValue;
}

/**
* The setter is overridden to ensure the `_hasInputValue` property
* doesn't wrongly indicate true after the input element's value
* is reverted or cleared programmatically.
*
* @override
* @protected
*/
set _inputElementValue(value) {
super._inputElementValue = value;
this._hasInputValue = value.length > 0;
}

/** @protected */
ready() {
super.ready();
Expand Down

0 comments on commit 3e1f22b

Please sign in to comment.