refactor: move input value getters and setters to InputMixin #5586
Conversation
5ffdd62 to
80e421a
Compare
80e421a to
1119d5d
Compare
| @@ -538,8 +526,9 @@ export const DatePickerMixin = (subclass) => | |||
| */ | |||
| checkValidity() { | |||
| const inputValid = | |||
| !this._inputValue || | |||
| (!!this._selectedDate && this._inputValue === this._getFormattedDate(this.i18n.formatDate, this._selectedDate)); | |||
| !this._inputElementValue || | |||
There was a problem hiding this comment.
Consider extracting the input value to a separate line e.g. like this:
const value = this._inputElementValue;Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -118,10 +118,15 @@ class ComboBoxLight extends ComboBoxDataProviderMixin(ComboBoxMixin(ValidateMixi | |||
| } | |||
|
|
|||
| /** | |||
| * @return {string} | |||
| * Override this getter from `InputMixin` to allow using | |||
There was a problem hiding this comment.
note: Although I can see it is quite common in our code base, I would argue that it can be misleading to start a JSDoc description with Override... when intending to explain the reason we have overridden this or that method. Such a description can be interpreted as a command – as if it suggests you override the getter in case you would like to use a custom property name.
There was a problem hiding this comment.
Yes, it can be misleading in some cases indeed.
Maybe it should be changed to Overriding instead.
Somehow I was using it consistently since Vaadin 22 🤷♂️
| @@ -118,10 +118,15 @@ class ComboBoxLight extends ComboBoxDataProviderMixin(ComboBoxMixin(ValidateMixi | |||
| } | |||
|
|
|||
| /** | |||
| * @return {string} | |||
| * Override this getter from `InputMixin` to allow using | |||
There was a problem hiding this comment.
Yes, it can be misleading in some cases indeed.
Maybe it should be changed to Overriding instead.
Somehow I was using it consistently since Vaadin 22 🤷♂️
|
Hi @vursen , this commit cannot be picked to 24.0 by this bot, can you take a look and pick it manually? Otherwise, please use 'git cherry-pick --skip' |
Description
The PR gathers input element value setters and getters from the components and moves them to
InputMixin. Collecting them all in one single place will make it possible to fix updating_hasInputValuefor all the components at once afterward.Part of #5410
Type of change