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

feat: add manual validation mode #8097

Merged
merged 22 commits into from
Nov 7, 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
15 changes: 13 additions & 2 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,21 @@
},
"overrides": [
{
// Prevent using optional-chaining in source files, as it is not supported by Polymer analyzer
"files": ["packages/*/src/**/*.js"],
"rules": {
"es/no-optional-chaining": "error"
// Prevent using optional-chaining in source files, as it is not supported by Polymer analyzer
"es/no-optional-chaining": "error",
"no-restricted-syntax": [
"error",
{
"selector": "ForInStatement",
"message": "for..in loops are slower than Object.{keys,values,entries} and have their caveats."
},
{
"selector": "CallExpression[callee.property.name='validate']",
"message": "Don't call validate() directly - it bypasses manual validation mode. Use _requestValidation() instead"
}
]
}
},
{
Expand Down
4 changes: 2 additions & 2 deletions packages/checkbox-group/src/vaadin-checkbox-group-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export const CheckboxGroupMixin = (superclass) =>
});

if (oldValue !== undefined) {
this.validate();
this._requestValidation();
}
}

Expand Down Expand Up @@ -304,7 +304,7 @@ export const CheckboxGroupMixin = (superclass) =>
// Do not validate when focusout is caused by document
// losing focus, which happens on browser tab switch.
if (!focused && document.hasFocus()) {
this.validate();
this._requestValidation();
}
}
};
6 changes: 3 additions & 3 deletions packages/checkbox/src/vaadin-checkbox-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,14 @@ export const CheckboxMixin = (superclass) =>
// Do not validate when focusout is caused by document
// losing focus, which happens on browser tab switch.
if (!focused && document.hasFocus()) {
this.validate();
this._requestValidation();
}
}

/** @private */
_checkedChanged(checked) {
if (checked || this.__oldChecked) {
this.validate();
this._requestValidation();
}

this.__oldChecked = checked;
Expand All @@ -246,7 +246,7 @@ export const CheckboxMixin = (superclass) =>
super._requiredChanged(required);

if (required === false) {
this.validate();
this._requestValidation();
}
}

Expand Down
11 changes: 0 additions & 11 deletions packages/combo-box/src/vaadin-combo-box-light-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,17 +99,6 @@ export const ComboBoxLightMixin = (superClass) =>
});
}

/**
* Returns true if the current input value satisfies all constraints (if any).
* @return {boolean}
*/
checkValidity() {
if (this.inputElement && this.inputElement.validate) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: We don't support iron form elements anymore.

return this.inputElement.validate();
}
return super.checkValidity();
}

/**
* @protected
* @override
Expand Down
2 changes: 1 addition & 1 deletion packages/combo-box/src/vaadin-combo-box-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ export const ComboBoxMixin = (subclass) =>
// Do not validate when focusout is caused by document
// losing focus, which happens on browser tab switch.
if (document.hasFocus()) {
this.validate();
this._requestValidation();
}

if (this.value !== this._lastCommittedValue) {
Expand Down
8 changes: 4 additions & 4 deletions packages/custom-field/src/vaadin-custom-field-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export const CustomFieldMixin = (superClass) =>
super._setFocused(focused);

if (!focused) {
this.validate();
this._requestValidation();
}
}

Expand Down Expand Up @@ -203,7 +203,7 @@ export const CustomFieldMixin = (superClass) =>
super._requiredChanged(required);

if (required === false) {
this.validate();
this._requestValidation();
}
}

Expand Down Expand Up @@ -233,7 +233,7 @@ export const CustomFieldMixin = (superClass) =>
event.stopPropagation();

this.__setValue();
this.validate();
this._requestValidation();
this.dispatchEvent(
new CustomEvent('change', {
bubbles: true,
Expand Down Expand Up @@ -299,7 +299,7 @@ export const CustomFieldMixin = (superClass) =>
this.__applyInputsValue(value || '\t');

if (oldValue !== undefined) {
this.validate();
this._requestValidation();
}
}

Expand Down
19 changes: 7 additions & 12 deletions packages/date-picker/src/vaadin-date-picker-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ export const DatePickerMixin = (subclass) =>
// Do not validate when focusout is caused by document
// losing focus, which happens on browser tab switch.
if (document.hasFocus()) {
this.validate();
this._requestValidation();
}
}
}
Expand Down Expand Up @@ -624,13 +624,8 @@ export const DatePickerMixin = (subclass) =>
!this._selectedDate || dateAllowed(this._selectedDate, this._minDate, this._maxDate, this.isDateDisabled);

let inputValidity = true;
if (this.inputElement) {
if (this.inputElement.checkValidity) {
inputValidity = this.inputElement.checkValidity();
} else if (this.inputElement.validate) {
// Iron-form-elements have the validate API
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: We don't support iron form elements anymore.

inputValidity = this.inputElement.validate();
}
if (this.inputElement && this.inputElement.checkValidity) {
inputValidity = this.inputElement.checkValidity();
}

return inputValid && isDateValid && inputValidity;
Expand Down Expand Up @@ -717,10 +712,10 @@ export const DatePickerMixin = (subclass) =>
const unparsableValue = this.__unparsableValue;

if (this.__committedValue !== this.value) {
this.validate();
this._requestValidation();
this.dispatchEvent(new CustomEvent('change', { bubbles: true }));
} else if (this.__committedUnparsableValue !== unparsableValue) {
this.validate();
this._requestValidation();
this.dispatchEvent(new CustomEvent('unparsable-change'));
}

Expand Down Expand Up @@ -849,7 +844,7 @@ export const DatePickerMixin = (subclass) =>

if (oldValue !== undefined) {
// Validate only if `value` changes after initialization.
this.validate();
this._requestValidation();
}
}
} else {
Expand Down Expand Up @@ -1015,7 +1010,7 @@ export const DatePickerMixin = (subclass) =>
// Needed in case the value was not changed: open and close dropdown,
// especially on outside click. On Esc key press, do not validate.
if (!this.value && !this._keyboardActive) {
this.validate();
this._requestValidation();
}
}

Expand Down
16 changes: 8 additions & 8 deletions packages/date-time-picker/src/vaadin-date-time-picker-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ export const DateTimePickerMixin = (superClass) =>
// Do not validate when focusout is caused by document
// losing focus, which happens on browser tab switch.
if (!focused && document.hasFocus()) {
this.validate();
this._requestValidation();
}
}

Expand Down Expand Up @@ -414,7 +414,7 @@ export const DateTimePickerMixin = (superClass) =>
event.stopPropagation();

if (this.__dispatchChangeForValue === this.value) {
this.validate();
this._requestValidation();
this.__dispatchChange();
}
this.__dispatchChangeForValue = undefined;
Expand Down Expand Up @@ -473,7 +473,7 @@ export const DateTimePickerMixin = (superClass) =>
newDatePicker.max = this.__formatDateISO(this.__maxDateTime, this.__defaultDateMinMaxValue);

// Disable default internal validation for the component
newDatePicker.validate = () => {};
newDatePicker.manualValidation = true;
}

/** @private */
Expand Down Expand Up @@ -501,7 +501,7 @@ export const DateTimePickerMixin = (superClass) =>
this.__updateTimePickerMinMax();

// Disable default internal validation for the component
newTimePicker.validate = () => {};
newTimePicker.manualValidation = true;
}

/** @private */
Expand Down Expand Up @@ -603,7 +603,7 @@ export const DateTimePickerMixin = (superClass) =>
}

if (this.__oldRequired && !required) {
this.validate();
this._requestValidation();
}

this.__oldRequired = required;
Expand Down Expand Up @@ -796,7 +796,7 @@ export const DateTimePickerMixin = (superClass) =>
this.__updateTimePickerMinMax();

if (this.__datePicker && this.__timePicker && this.value) {
this.validate();
this._requestValidation();
}
}

Expand All @@ -809,7 +809,7 @@ export const DateTimePickerMixin = (superClass) =>
this.__updateTimePickerMinMax();

if (this.__datePicker && this.__timePicker && this.value) {
this.validate();
this._requestValidation();
}
}

Expand Down Expand Up @@ -910,7 +910,7 @@ export const DateTimePickerMixin = (superClass) =>
// run initial validation here. Lit version runs observers differently and
// this observer is executed first - ignore it to prevent validating twice.
if ((this.min && this.__minDateTime) || (this.max && this.__maxDateTime)) {
this.validate();
this._requestValidation();
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/field-base/src/input-constraints-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ export const InputConstraintsMixin = dedupingMixin(
const isLastConstraintRemoved = this.__previousHasConstraints && !hasConstraints;

if ((this._hasValue || this.invalid) && hasConstraints) {
this.validate();
} else if (isLastConstraintRemoved) {
this._requestValidation();
} else if (isLastConstraintRemoved && !this.manualValidation) {
this._setInvalid(false);
}

Expand All @@ -109,7 +109,7 @@ export const InputConstraintsMixin = dedupingMixin(
_onChange(event) {
event.stopPropagation();

this.validate();
this._requestValidation();

this.dispatchEvent(
new CustomEvent('change', {
Expand Down
6 changes: 3 additions & 3 deletions packages/field-base/src/input-field-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export const InputFieldMixin = (superclass) =>
// Do not validate when focusout is caused by document
// losing focus, which happens on browser tab switch.
if (!focused && document.hasFocus()) {
this.validate();
this._requestValidation();
}
}

Expand All @@ -112,7 +112,7 @@ export const InputFieldMixin = (superclass) =>
super._onInput(event);

if (this.invalid) {
this.validate();
this._requestValidation();
}
}

Expand All @@ -133,7 +133,7 @@ export const InputFieldMixin = (superclass) =>
}

if (this.invalid) {
this.validate();
this._requestValidation();
}
}
};
15 changes: 15 additions & 0 deletions packages/field-base/src/validate-mixin.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,19 @@ export declare class ValidateMixinClass {
*/
invalid: boolean;

/**
* Set to true to enable manual validation mode. This mode disables automatic
* constraint validation, allowing you to control the validation process yourself.
* You can still trigger constraint validation manually with the `validate()` method
* or use `checkValidity()` to assess the component's validity without affecting
* the invalid state. In manual validation mode, you can also manipulate
* the `invalid` property directly through your application logic without conflicts
* with the component's internal validation.
*
* @attr {boolean} manual-validation
*/
manualValidation: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add @attr {boolean} manual-validation annotation also to this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


/**
* Specifies that the user must fill in a value.
*/
Expand All @@ -30,4 +43,6 @@ export declare class ValidateMixinClass {
* Returns true if the field value satisfies all constraints (if any).
*/
checkValidity(): boolean;

protected _requestValidation(): void;
}
24 changes: 24 additions & 0 deletions packages/field-base/src/validate-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,22 @@ export const ValidateMixin = dedupingMixin(
value: false,
},

/**
* Set to true to enable manual validation mode. This mode disables automatic
* constraint validation, allowing you to control the validation process yourself.
* You can still trigger constraint validation manually with the `validate()` method
* or use `checkValidity()` to assess the component's validity without affecting
* the invalid state. In manual validation mode, you can also manipulate
* the `invalid` property directly through your application logic without conflicts
* with the component's internal validation.
*
* @attr {boolean} manual-validation
*/
manualValidation: {
type: Boolean,
value: false,
},

/**
* Specifies that the user must fill in a value.
*/
Expand Down Expand Up @@ -79,6 +95,14 @@ export const ValidateMixin = dedupingMixin(
return true;
}

/** @protected */
_requestValidation() {
if (!this.manualValidation) {
// eslint-disable-next-line no-restricted-syntax
this.validate();
}
}

/**
* Fired whenever the field is validated.
*
Expand Down
Loading