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

Remove incorrect validations #230

Conversation

deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented May 28, 2018

Hi!

As mentioned in #176 & #209, the validation done by foundation-datepicker is incorrect since there's no guarantee that the length of the format string will match the actual length of a date. Moreover, this validation has been biting us in combination with the https://github.com/sgruhier/foundation_rails_helper gem. If a submitted form has errors in the date field, this gem will generate form input with "foundation friendly errors". However, this piece of JS will actually remove part of them making them look weird.

Also, in my opinion, users wanting to perform client side validations should use foundation-abide which is more powerful anyways.

So because of all of the above reasons, I think we should remove these incorrect validations from the library.

Fixes #176 & #209.

The length of the format string does not necessarily match the length of
the actual date, so this validation is incorrect.

I'm not sure when this validation is useful anyways (if some malformed
date comes in the field from the server I guess).

In my opinion, the easiest way to fix this is to remove the validation.
Users wanting client side validations in foundation should use
`foundation-abide`.
@deivid-rodriguez
Copy link
Contributor Author

deivid-rodriguez commented May 28, 2018

I committed the package-lock.json file since it helps a bit with ensuring all contributors get the same environment, and it prevents source control untracked files from being generated during development. Let me know it you want that removed.

@deivid-rodriguez deivid-rodriguez force-pushed the remove_incorrect_validations branch from 4ff0f8e to c50ca61 Compare June 28, 2018 09:28
@deivid-rodriguez
Copy link
Contributor Author

I moved the package-lock.json addition to a separate PR to make reviewing this easier.

@deivid-rodriguez
Copy link
Contributor Author

@najlepsiwebdesigner Any news here? We'd prefer to not fork the library and have some valuable feedback from you :)

@najlepsiwebdesigner
Copy link
Owner

Hi! I'm sorry for late answer, I am currently finishing my PhD so I'm "little bit" busy. I plan to review all the changes proposed in PRs and issues during August. Thank you for understanding ;)

@deivid-rodriguez
Copy link
Contributor Author

No problem, thank you, and good look with your PhD 💪.

@deivid-rodriguez
Copy link
Contributor Author

deivid-rodriguez commented Sep 13, 2018

Hey @najlepsiwebdesigner, just a ping since it's september now. I hope you finished your PhD successfully and you're doing great! :)

@deivid-rodriguez
Copy link
Contributor Author

Any news?

@najlepsiwebdesigner najlepsiwebdesigner merged commit c555032 into najlepsiwebdesigner:master Jan 16, 2019
@deivid-rodriguez deivid-rodriguez deleted the remove_incorrect_validations branch January 16, 2019 22:52
@deivid-rodriguez
Copy link
Contributor Author

Thanks so much @najlepsiwebdesigner!

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