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

refactor!: align hasInputValue behavior across components #8200

Merged
merged 1 commit into from
Nov 21, 2024
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
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