-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(collapse): respect display input value #6071
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
base: development
Are you sure you want to change the base?
fix(collapse): respect display input value #6071
Conversation
f7a37ef to
afe0e89
Compare
This change removes race conditions caused by TypeScript setters being called by Angular in non-deterministic order. Setters side effects (reading values set in other setters) are moved to ngOnChanges lifecycle hook. The consequence is that those side effects are no longer called when those inputs are set other than in the template. This is a breaking change but methods to call instead of setting those properties are already provided (show/hide) so migration path is straightforward. Setting display to 'none' no longer hides the collapse, setting collapse input to true or calling hide method is the way to go. BREAKING CHANGE: setting display or collapse property on CollapseDirective no longer expands/collapses the collapse - use show/hide methods instead or set collapse input in template
afe0e89 to
6a4ada2
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #6071 +/- ##
===============================================
+ Coverage 69.78% 69.83% +0.04%
===============================================
Files 343 343
Lines 9235 9227 -8
Branches 2007 2006 -1
===============================================
- Hits 6445 6444 -1
+ Misses 2195 2189 -6
+ Partials 595 594 -1
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
This change removes race conditions caused by TypeScript setters being called by Angular in non-deterministic order. Setters side effects (reading values set in other setters) are moved to
ngOnChangeslifecycle hook. The consequence is that those side effects are no longer called when those inputs are set other than in the template. This is a breaking change but methods to call instead of setting those properties are already provided (show/hide) so migration path is straightforward. Settingdisplayto'none'no longer hides the collapse, settingcollapseinput totrueor callinghidemethod is the way to go.BREAKING CHANGE: setting
displayorcollapseproperty onCollapseDirectiveno longer expands/collapses the collapse - useshow/hidemethods instead or setcollapseinput in templateNo changes in tests since those are currently structured such that I cannot add display property for only one case (there is single shared template). I can restructure the tests to use separate templates (with
overrideComponent) if you wish (I consider this major change since most likely majority of lines in spec file will be affected) .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.