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

[FEATURE] Add onInvalid action to form. #846

Merged

Conversation

Serabe
Copy link
Contributor

@Serabe Serabe commented Oct 17, 2017

Add onInvalid action, invoked when trying to submit an invalid form.

Add `onInvalid` action, invoked when trying to submit an invalid form.
@Serabe
Copy link
Contributor Author

Serabe commented Oct 17, 2017

Not sure why the build failed given the test passed :$

@miguelcobain
Copy link
Collaborator

miguelcobain commented Oct 17, 2017

Very weird. I can merge this because it's clearly not related.

Regarding the PR, I love the idea, but I'm unsure about the naming.
onInvalid sounds like an action that is sent when the form becomes invalid, which is not the case.

What if we kept using onSubmit but we sent the validity of the form in the action?
People could then later treat the submit accordingly in their action.

Using your approach, I think we should rename the action. Perhaps onSubmitInvalid?

Let me know your thoughts. Thanks for the PR.

@Serabe
Copy link
Contributor Author

Serabe commented Oct 17, 2017

I would like to use invalid as the name of the event because that is the standard name according to MDN. If you don't like that option, onSubmitInvalid looks good too.

@miguelcobain
Copy link
Collaborator

I was unaware of that event.
Makes sense to name this action accordingly since they attempt to do the same thing.
Thanks again!

@miguelcobain miguelcobain merged commit a34e306 into adopted-ember-addons:master Oct 18, 2017
@Serabe Serabe deleted the feature/invalid-event branch October 18, 2017 15:32
@Serabe
Copy link
Contributor Author

Serabe commented Oct 18, 2017

Thanks!

@Serabe
Copy link
Contributor Author

Serabe commented Nov 20, 2017

@miguelcobain is there any planned release that includes this PR?

Thanks!

@miguelcobain
Copy link
Collaborator

@Serabe v1.0.0-beta.4 was published with this PR. Sorry for the delay.

@Serabe
Copy link
Contributor Author

Serabe commented Dec 21, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants