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

regression: datepicker marked invalid if custom adapter is used, since 4.2.0 #3215

Closed
jnizet opened this issue May 30, 2019 · 6 comments · Fixed by #3219
Closed

regression: datepicker marked invalid if custom adapter is used, since 4.2.0 #3215

jnizet opened this issue May 30, 2019 · 6 comments · Fixed by #3219

Comments

@jnizet
Copy link
Member

jnizet commented May 30, 2019

Bug description:

When a custom adapter is set on a datepicker popup, the form control is marked invalid by the invalidDate validator.
My debugging showed that every time we pick a date, the calendar.isValid()method is called several times with the NgbDate, but ends up being called with the control value, which is not an NgbDate, but is the adapted value, which thus fails the validation.

Link to minimally-working StackBlitz that reproduces the issue:

https://stackblitz.com/edit/angular-c54t21?file=app%2Fdatepicker-adapter.html

This is just a trimmed down version of the "custom adapter" demo example. Select a date, and you'll see that it's always marked invalid.

Versions of Angular, ng-bootstrap and Bootstrap:

Angular: 7.x, 8.0

ng-bootstrap: 4.2.0

Bootstrap: 4.x

@jnizet
Copy link
Member Author

jnizet commented May 30, 2019

Note that it only works by accident when no adapter is present, because the value of the control is an NgbDateStruct, which happens to have the same properties as an NgbDate.

@SimonWilliams-STL
Copy link

We are experiencing this issue using NgbDateNativeAdapter.

@eran10
Copy link

eran10 commented May 30, 2019

+1

@jnizet
Copy link
Member Author

jnizet commented May 30, 2019

Also note that the other datepicker validators are broken too, but in the other way: they return null (no error) when the control value is not a date struct. So the minDate and maxDate validators consider any value as valid as soon as a date adapter is used.

Can we revert the commit 509bba3 and make a fix release, until this is figured out?

Some design is necessary before exposing the validators, IMHO.

They should at least use the adapter to get back an NgbDateStruct, then transform it to a NgbDate before checking the validity of the NgbDate with the calendar. But even with this additional argument, I wonder if it's a good idea to transform the NgbDate to some model value and then transform it back to an NgbDate to validate the value. Maybe the initial problem solved by this commit should be resolved in another way.

@maxokorokov
Copy link
Member

Thanks for comments and sorry about this!
Unfortunately, I don't have the time to investigate this properly at the moment, so quickest fix at the moment would be to revert the commit with 4.2.1 as @jnizet suggested.

@maxokorokov maxokorokov added this to the 4.2.1 milestone Jun 3, 2019
@jnizet
Copy link
Member Author

jnizet commented Jun 4, 2019

Thanks for the quick fix and release @maxokorokov ❤️

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