feat: Forms: Add reset validation functionality to form and form element [GAUD-6979] #4983
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
| this._errors = []; | ||
|
|
||
| this.childErrors.forEach((_, ele) => { | ||
| if (!isCustomFormElement(ele)) return; |
There was a problem hiding this comment.
A case (?) that wouldn't be covered is if the child error is a native element. I don't know if that's actually a thing we deal with.
margaree
changed the title
margaree
changed the title
…ests and documentation.
components/form/form.js
Outdated
| firstUpdated(changedProperties) { | ||
| super.firstUpdated(changedProperties); | ||
|
|
||
| this.addEventListener('d2l-dialog-close', () => { |
There was a problem hiding this comment.
Probably worth a unit test that covers this case?
components/form/form.js
Outdated
| this.addEventListener('d2l-dialog-close', () => { | ||
| const dialogAncestor = findComposedAncestor( | ||
| this, | ||
| (node) => { return (node?.tagName?.includes('D2L-DIALOG')); } |
There was a problem hiding this comment.
I think we'd want to do this for all dialogs, yeah? Like including d2l-dialog-fullscreen or other dialog components that might extend the DialogMixin.
There was a problem hiding this comment.
The .includes( should cover that?
There was a problem hiding this comment.
Oh good point. I glossed over that. So it would just be anyone (like LCS) that might have extended the mixin for their own component. Maybe they're on their own to reset their forms.
There was a problem hiding this comment.
Wonder if something like node.prototype instanceof DialogMixin would work?
There was a problem hiding this comment.
Still looking into it but so far haven't been able to get that to work
There was a problem hiding this comment.
I know in a couple other places we've set a property that can be checked, which seems kinda hacky. What Dave proposed seems much nicer.
There was a problem hiding this comment.
I haven't been able to get anything along the lines of the node.prototype instanceof DialogMixin check working. I can get setting a property on dialog mixin and checking for that working. Do we want to go through with the property option? Any other ideas that I maybe haven't tried?
There was a problem hiding this comment.
Hmm, I feel like a _isDialogMixin = true "private" variable would be slightly more resilient than the tag name check, but am good with either.
Co-authored-by: Dave Lockhart <[email protected]>
|
🎉 This PR is included in version 3.43.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Jira ticket
This adds reset functionality to both a
d2l-formandFormElementMixin. I'd originally looked at adding it to theFormMixinbut that got complicated because of potential for nesting of forms and how some functionality lives in Form vs others inFormMixin. I can re-look at that, but at the moment there's nothing just usingFormMixin.This can be tested on the form demo page.
To do: