-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix(datepicker): update subscriptions of datepicker-inline and daterangepicker-inline #6201
base: development
Are you sure you want to change the base?
Conversation
…ngepicker-inline Introduce updateSubscriptions method to unsubscribe from previous subscriptions and to create new subscriptions on new _datepickerRef.instance Reduce calls to setConfig by introducing a shouldSetConfig flag, thereby batching complex updates together Closes valor-software#5888 where EventEmitter does not fire after Input changes
Codecov Report
@@ Coverage Diff @@
## development #6201 +/- ##
===============================================
- Coverage 77.52% 77.47% -0.05%
===============================================
Files 302 302
Lines 10549 10566 +17
Branches 2583 2587 +4
===============================================
+ Hits 8178 8186 +8
- Misses 2361 2370 +9
Partials 10 10
Continue to review full report at Codecov.
|
Does anyone have an ETA on this PR being approved and merged? This is currently blocking my app from being deployed as the inline calendar has these bugs. |
Hey, I would love to reduce the amount of work you have to deal with on this PR. |
Context
Both datepicker-inline and daterangepicker-inline don't update their subscriptions to the datepicker instance after calling
setConfig
.This causes the datepicker to not fire
bsValueChanges
after one of the datepickers inputs have changed.Changes
_datepickerRef.instance
Closes #5888 where EventEmitter does not fire after Input changes
PR Checklist
Before creating new PR, please take a look at checklist below to make sure that you've done everything that needs to be done before we can merge it.