Add form element to arguments passed to (private) validate hook #1793
Conversation
There was a problem hiding this comment.
Overall this looks good to me and I don't have concerns adding another argument on validate. It is not a public API. We have already changed it without major version as we have control over the validation addons using it.
I only have one question if we couldn't simplify the implementation.
@jelhan in which way? |
| @@ -400,7 +409,7 @@ export default class Form extends Component { | |||
|
|
|||
| return RSVP.resolve() | |||
| .then(() => { | |||
| return this.hasValidator ? this.validate(model) : null; | |||
| return this.hasValidator ? this.validate(model, this._element) : null; | |||
There was a problem hiding this comment.
Somehow my review comment got lost. 😢
I'm wondering if we could pick up the form element from the event instead of tracking it through {{ref}}. The event listener is registered on the form element. Shouldn't the form element be available as event.target in that case?
There was a problem hiding this comment.
Yes, indeed I had it this way, until I realized some existing test failed. It was one where we submit the form using the yielded submit action, and not a submit (browser) event. In which case we don't have event.target. That's why I also added the second test in this PR, which is replicating this very situation!
simonihmig
changed the title
See #1792