-
Notifications
You must be signed in to change notification settings - Fork 105
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
#442 breaks with onpush issues #453
Comments
Thank you for Reporting it, as of now I think it's safe to rollback. You can open a PR and we can get it in later with backwards compatibility. |
I think we can just set |
Unfortunately I'm not super familiar with the code base, I'll go with a rollback, and you can add me on a PR that contains a roll forward along with a patch, and I can patch it in and see if it works in our code base. |
close uiuniversal#453 resolve uiuniversal#455
@chivesrs I have merged the PR from @luca-peruzzo can you try the code from main, I will release a new version soon. |
Hello @santoshyadavdev I have patched in the code from main into our repository. On an initial glance, everything seems to be working fine - the bug I had reported does not seem to be reproducible with the fix @luca-peruzzo made. I will go through the steps to get the change submitted internally and let you know the results. Expect a week or two turnaround time. |
Sounds good meanwhile I will get it released, so we can merge our standalone component changes |
Released with 7.2.0 |
I'm currently having some issues where our unit tests are broken that I'm trying to figure out. |
@luca-peruzzo Can you check what happens if the carousel loses items? The test I have has 2 items in the carousel, user action deletes 1, and a new array is created and passed into the carousel. The carousel then seems to still have 2 items instead of the desired 1 (the deleted one gets shifted to the end). |
This issue has been automatically marked as stale because it has not had recent activity for 6 months. It will be closed if no further activity occurs. Thank you for your contributions. |
Closing this issue due to no activity for 6 months. |
(this is coming from code inside Google's internal code base, we use ngu-carousel for some of our Angular applications)
Describe the bug
#442 breaks initial carousel loading in an OnPush enabled application, where the carousel items do not render until you tab into the carousel via keyboard (as keyboard events trigger a change detection event)
The commit right before it fc0f98580590355d8a078b2f71c7a902e8d72730 works just fine without any issues. The issue I believe is 044a294#diff-80e3ec9310b23dec2353dad30b6d6e0d26bf487117733a72fdb8c40c38a95655L190, where there is a time of check vs time of use issue, basically this._unwrappedData is always undefined there, and so the initial ngDoCheck() does nothing, causing this issue.
I'll send out a PR to revert this commit, unfortunately it will break some users that rely on newer functionality but I think it's better so that older users don't have a bug on a simple upgrade.
@arturovt @santoshyadavdev
The text was updated successfully, but these errors were encountered: