angular-ui / ui-scroll

Unlimited bidirectional scrolling over a limited element buffer for AngularJS applications
http://angular-ui.github.io/ui-scroll/demo/
MIT License
327 stars 107 forks source link

Jumps while scrolling to top in Ionic project #163

Closed misterdev closed 7 years ago

misterdev commented 7 years ago

I'm developing a chat app using Ionic 1.3.1, the same code works perfectly on desktop browsers but has problems on real devices (android 6.0.1/ios 10.3.2), that's why I think this is a ui-scroll bug.

No matter which values I use for buffer-size and padding, when I reach the top element in the buffer (eg. index -40) it loads buffer_size elements and jumps to the new top element (e.g. index -60) then loads other buffer_size elements correctly.

Also, when I set the same height to every element, works fine on ios, but this is not a possible solution since every message may have different heights.

I don't have enough time to provide a repro, can you please give some tips on what I can look at to try to solve this?

PS: Thanks for developing this, switching from ng-repeat to ui-scroll has improved performance hugely!

dhilt commented 7 years ago

@MisterDev I have spent time and set up the Ionic environment. I followed this official guide. I've injected latest ui-scroll (v1.6.1) into the app, build simple controller and run it locally in my Chrome browser (Win 10 desktop). And I didn't see any issues. What am I doing wrong?

Also I want to attach a link to my test project, maybe this may help! please download it and try to run it on your end.

misterdev commented 7 years ago

@dhilt this issue happens only when running on mobile devices (tested on android 6.0.1/ios 10.3.2).

You can find the instructions here.

  1. Add the platform using ionic platform add android or ios
  2. Connect the device via USB then run ionic run android --device or ionic run ios --device depending on the platform. For Android you may need to run adb devices before ionic run to boot adb.

BTW Thanks a lot for your help! I'm going to test this and let you know

misterdev commented 7 years ago

I've edited and uploaded your code here.

This works perfectly on chrome desktop but fails on my android device.

Also, it seems to work if I set the same height to every element in css height: 100px, but fails if I set it using ng-stye="{'height': '100px'}"

dhilt commented 7 years ago

@MisterDev Good news (not sure we should be happy): I was able to reproduce the issue on my Android device. Let's think what could be done.

misterdev commented 7 years ago

@dhilt Nice to know this is actually an issue. I'm going to work on fixing this in my app next week, do you have any tip on which could be the cause or what to look at? I can submit a PR If I'll find a good solution.

Also, on mobile devices ui-scroll is slightly less performant because of forced reflows, but I don't know if it is possible to do something on this.

dhilt commented 7 years ago

@MisterDev I'm working in a separate branch and looks like I'm prety close to the issue resolving. May I ask you to try a pre-build https://github.com/angular-ui/ui-scroll/blob/interpolation/dist/ui-scroll.js?

dhilt commented 7 years ago

@MisterDev I've finished it. You were right, the problem was in the ui-scroll core and I was able to reproduce the issue on the desktop... Since I've covered my changes with tests, I can say that this fix will be in a nearest (1.6.2) release. Meanwhile, you may try the branch code: https://github.com/angular-ui/ui-scroll/tree/interpolation/dist

misterdev commented 7 years ago

@dhilt That's great, thank you! I can see it is improved.

I still have some problems: 1) I experience the bug described in the first message, 10% of the times 2) When scrolling to bottom it flashes before loading new items 3) When scrolling in any direction, there's a moment when the scroll freezes to load new items. If I scroll during that time it actually scrolls after loading new items, causing a jump

I hope I explained it well, I'll try to replicate these on your test project this later and let you know.

robertdempsey commented 7 years ago

@dhilt The symptoms @MisterDev is describing seem similar to those I've mentioned in https://github.com/angular-ui/ui-scroll/issues/160. Possibly related?

dhilt commented 7 years ago

@robertdempsey Hmm... It was realy hard to reproduce it on a desktop (this is why I asked you for a repro), but now I see some correlation... By the way, could you also try this pre-fix: https://github.com/angular-ui/ui-scroll/tree/interpolation/dist – does it solve your issue?

@MisterDev Let's separate performance problems (freezing/flashing) and the initial problem (upward jumping). Following you, I've made a repo, where I have uploaded old 1.6.1 and new pre1.6.2 versions of the ui.scroll module (https://github.com/dhilt/ui-scroll-ionic/tree/master/www/lib/ionic/js/angular-ui – I set it here). I tried both of them on my device, and my little investigation showed me that a) both versions have similar performance params; b) new version has no jumping issue, while the old one does.

I have to say, I continue the code refactoring, maybe 1.6.2 will be more complex.

misterdev commented 7 years ago

@dhilt I'm not able to reproduce the initial problem in your test app, but it happens if I use the same code inside my app. I'll have to refactor my code and test it again. Thank you!

dhilt commented 7 years ago

@MisterDev Good luck! and tell the results, the issue should be closed once.

I've just started new branch, looks like this may improve performance a bit cause the result code produces less digest cycles then it was before. Not sure it could be noticed in common case, but you also may try the distributive: https://github.com/angular-ui/ui-scroll/tree/sync/dist

misterdev commented 7 years ago

@dhilt Unfortunately, updating my code to the latest version of ionic 1 doesn't solved my issue. Do you have any tip on how could I solve/debug this?

dhilt commented 7 years ago

@MisterDev Does 1.7.0-rc.1 not work for you? You may fork that ionic demo repo (https://github.com/dhilt/ui-scroll-ionic), make breaking changes and PR them. Then will try to peroduce...

misterdev commented 7 years ago

@dhilt at the moment I'm using pre-1.6.1 because 1.7.0 broke my chat. I had a similar problem using version 1.3.1, while scrolling to top it keeps requesting wrong indexes.

It should ask for -29 -> 0, -59 -> -30, -89 -> -60, ecc.. but this is what happens:

screen shot 2017-07-07 at 17 00 14

This happens to me on desktop browsers too.

BTW, I'll try to make breaking changes on your repo. Thanks a lot for your help.

misterdev commented 7 years ago

@dhilt After hours of scrolling I've not been able to repro this problem on your test app. At least I've found a solution to fix this in my app:

screen shot 2017-07-10 at 11 08 44

setting m-header's heightto a fixed value instead of just setting min-height made it.

Further investigations revealed that this was the source of all my pains (when .m-header doesn't have a fixed height)

<div class="m-header">
  <span>Hello</span>
  <span class="ion-ios-arrow-down expand"></span>
</div>
.m-header {
    display: flex;
    flex-direction: row;
    align-items: center;
    min-height: 32px;
}
.expand {
    position: absolute;
    right: 0px;
}
misterdev commented 7 years ago

@dhilt "Good" news, seems like I have been able to reproduce this here

screen shot 2017-07-10 at 11 50 49

dhilt commented 7 years ago

@MisterDev This is defenitly not the ionic issue. I made a little reseach and create https://github.com/angular-ui/ui-scroll/issues/171 issue. As workaround you may try to increase buffer-size or add non-zero delay on items retrieving.

misterdev commented 7 years ago

@dhilt well done! I don't know if that's related to the initial problem:

No matter which values I use for buffer-size and padding, when I reach the top element in the buffer (eg. index -40) it loads buffer_size elements and jumps to the new top element (e.g. index -60) then loads other buffer_size elements correctly.

I'm still experiencing this sometimes, especially when I reach the top element and scroll before the new elements are loaded.

I'll post more if I'll be able to repro this or if #171 's fix will solve this too.

dhilt commented 7 years ago

@MisterDev Sure! I hope we'll overcome it sooner or later.