-
Notifications
You must be signed in to change notification settings - Fork 896
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
fix(labs): allow validators to report customError #5310
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,8 @@ import { | |
import {mixinElementInternals} from './element-internals.js'; | ||
import {getFormValue, mixinFormAssociated} from './form-associated.js'; | ||
import {CheckboxValidator} from './validators/checkbox-validator.js'; | ||
import {Validator} from './validators/validator.js'; | ||
import {SelectState} from './validators/select-validator.js'; | ||
|
||
describe('mixinConstraintValidation()', () => { | ||
const baseClass = mixinConstraintValidation( | ||
|
@@ -45,6 +47,52 @@ describe('mixinConstraintValidation()', () => { | |
} | ||
} | ||
|
||
/** | ||
* A validator that set customError flag to true | ||
*/ | ||
class CustomErrorValidator extends Validator<SelectState> { | ||
private control?: HTMLInputElement; | ||
|
||
protected override computeValidity(state: SelectState) { | ||
if (!this.control) { | ||
this.control = document.createElement('input'); | ||
} | ||
this.control.setCustomValidity('validator custom error'); | ||
return { | ||
validity: this.control.validity, | ||
validationMessage: this.control.validationMessage, | ||
}; | ||
} | ||
|
||
protected override equals(prev: SelectState, next: SelectState) { | ||
return prev.value === next.value | ||
} | ||
|
||
protected override copy({ value, required }: SelectState) { | ||
return { value, required }; | ||
} | ||
} | ||
|
||
@customElement('test-custom-error-constraint-validation') | ||
class TestCustomErrorConstraintValidation extends baseClass { | ||
@property() value = ''; | ||
@property({ type: Boolean }) required = false; | ||
override render() { | ||
return html`<div id="root"></div>`; | ||
} | ||
[createValidator]() { | ||
return new CustomErrorValidator(() => this); | ||
} | ||
|
||
[getValidityAnchor]() { | ||
return this.shadowRoot?.querySelector<HTMLElement>('#root') ?? null; | ||
} | ||
|
||
[getFormValue]() { | ||
return String(this.value); | ||
} | ||
} | ||
|
||
describe('validity', () => { | ||
it('should return a ValidityState value', () => { | ||
const control = new TestConstraintValidation(); | ||
|
@@ -174,4 +222,26 @@ describe('mixinConstraintValidation()', () => { | |
.toBe('Error'); | ||
}); | ||
}); | ||
|
||
describe('customError', () => { | ||
it('should set customError to true when validator has customError', () => { | ||
const control = new TestCustomErrorConstraintValidation(); | ||
expect(control.validity.customError) | ||
.withContext('validity.customError') | ||
.toBeTrue(); | ||
}); | ||
it('should dispatch invalid event when validator has customError', () => { | ||
const control = new TestCustomErrorConstraintValidation(); | ||
const invalidListener = jasmine.createSpy('invalidListener'); | ||
control.addEventListener('invalid', invalidListener); | ||
control.reportValidity(); | ||
expect(invalidListener).toHaveBeenCalledWith(jasmine.any(Event)); | ||
}); | ||
it('should report custom validation message over other validation messages', () => { | ||
const control = new TestCustomErrorConstraintValidation(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this test needs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @asyncLiz I think this is covered in other tests above in the same file. Here we are just making sure that a Those 3 new tests fail without the fix in labs/behaviors/constraint-validation.ts. They pass after changing it to: const customError = !!this[privateCustomValidationMessage] || validity.customError; |
||
expect(control.validationMessage) | ||
.withContext('validationMessage') | ||
.toBe('validator custom error'); | ||
}); | ||
}) | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs
this.control.required = state.required
to support the multi-validation testThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above: we only care about customError as this is not working properly with the current codebase.