ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.02k stars 13.52k forks source link

Scroll Acceleration and Pull to refresh with VirtualScroll List #6984

Closed ghenry22 closed 7 years ago

ghenry22 commented 8 years ago

Short description of the problem:

This needs to be run on the iOS simulator or a real device to reproduce.

Create a virtualscroll list with several items Add pull to refresh to the page

1) when scrolling fast the acceleration on the scroll is noticably slower than with regular ngfor based lists.

2) regardless of position in the list a swipe downwards will trigger the pull to refresh rather than scrolling the list.

What behavior are you expecting?

Scroll acceleration and deceleration to be similar to native and consistent with ngfor based lists

Pull to refresh only triggered when at the top of a list and pulling down. Should not trigger when you are scrolled halfway down a list.

Steps to reproduce:

  1. create a new starter app
  2. add a virtualscroll list to a page with more items than fit onto the screen
  3. add a pull to refresh component to the page.
  4. scroll down in the list and observe the acceleration / deceleration is wrong
  5. from any point in the list swipe down and pull the refresh is activated
insert any relevant code between the above and below backticks

Other information: (e.g. stacktraces, related issues, suggestions how to fix, stackoverflow links, forum links, etc)

Which Ionic Version? 1.x or 2.x 2.0 beta9

Plunker that shows an example of your issue

N/A needs to be in an iOS simulator or real device

Run ionic info from terminal/cmd prompt: (paste output below)

Cordova CLI: 6.2.0 Gulp version: CLI version 3.9.1 Gulp local: Local version 3.9.1 Ionic Framework Version: 2.0.0-beta.9 Ionic CLI Version: 2.0.0-beta.30 Ionic App Lib Version: 2.0.0-beta.16 ios-deploy version: 1.8.6 ios-sim version: 5.0.8 OS: Mac OS X El Capitan Node Version: v5.11.1 Xcode version: Xcode 7.3.1 Build version 7D1014

ghenry22 commented 8 years ago

@adamdbradley @jgw96 Hi guys, sorry to bug you but it would be brilliant if the conflict with the ion-refresher could at least be resolved. Since Beta9 virtualscroll is much smoother on iOS UIWebview performance wise but if there is an ion-refresher in the page I can only scroll in one direction, any downward swipe from any point in a list just triggers the refresh which makes the list completely unusable.

This was not an issue with beta8 and earlier so is likely related to the performance fixes introduced in beta9.

This only occurs on uiwebview on the ios simulator or a real device. Browser and android do not have this issue.

The scroll acceleration is less of an issue, it's just slow and feels laggy/non-native but that it doesn't block any functionality.

As beta11 is tagged as a bug fix release when I look at the milestone perhaps this one could creep in there?

please :)

jgw96 commented 8 years ago

@ghenry22 Would you mind making a repo with a very minimal example that i can use to reproduce this issue with? Thanks!

ghenry22 commented 8 years ago

Yep will do tomorrow

ghenry22 commented 8 years ago

https://github.com/ghenry22/virtualScrollTest.git

Here is a repo with a really simple project, it's just a virtualscroll list with a ion-refresher.

Scroll part way down the list, try to scroll back up by swiping downwards, if you swipe slowly you will see that the refresher starts to open regardless of where you are in the virtuallist. Refresher should only open if you are at the top of the list.

Scroll acceleration is harder to replicate. You could just cut and paste my source array a few times so you have a nice long list then you can test scroll acceleration, it's just slow to accelerate and decelerate and never feels like it gets up to full speed.

Again these issues are specific to iOS with UIWebview. On android none of the problems exist.

The conflict with the ion-refresher only started in Beta9 at the same time that performance optimizations were made for virtualscroll on UIWebview so this is probably related to a change made there.

ghenry22 commented 8 years ago

Any chance to take a look at this? I can only scroll in one direction in iOS which is somewhat limiting

0v3rst33r commented 8 years ago

I am having the exact same issue in my project. The moment I try to scroll back up, the Refresher kicks in.

0v3rst33r commented 8 years ago

Any news on this yet?

0v3rst33r commented 8 years ago

The problem on iOS is that the scrollHostScrollTop value stays 0:

https://github.com/driftyco/ionic/blob/master/src/components/refresher/refresher.ts#L217

which in turn causes this line never to be fired (and results in _onMove always being fired - whether you scroll up or down):

https://github.com/driftyco/ionic/blob/master/src/util/ui-event-manager.ts#L51

I am still looking at this though (and will report back with subsequent findings) seeing that I urgently need a fix in my current builds because without this resolved an Ionic app (using Refresher) is non-functional on the iOS platform.

Do note I don't know if scrollHostScrollTop is set with a proper value when NOT using VirtualScroll. Haven't confirmed/tested that part yet.

0v3rst33r commented 8 years ago

Not using VirtualScroll, then scrollHostScrollTop is populated correctly. The HTML (not src but from running application) in my project looks like this then:

<ion-content class="feed has-refresher has-infinite-scroll statusbar-padding" style="">
  <scroll-content style="margin-top: 44px; margin-bottom: 41px; transform: translateZ(0px); -webkit-transition: 0ms; transition: 0ms;">
    <ion-list>
      <ion-card tappable="" class="">
      ................

Using VirtualScroll, then scrollHostScrollTop does not get populated on iOS (value stays 0).The HTML looks like this then:

<ion-content class="feed has-refresher has-infinite-scroll statusbar-padding js-scroll" style="">
  <scroll-content style="margin-top: 44px; margin-bottom: 41px; transform: translateZ(0px); -webkit-transition: 0ms; transition: 0ms;">
    <ion-list ng-reflect-approx-item-height="500px" ng-reflect-virtual-scroll="[object Object],[object Object],[object Object],[object Object],[object Object],[object Object]" class="virtual-scroll" style="height: 2748px;">
      <ion-card tappable="" class="virtual-position" aria-posinset="1" aria-setsize="6" style="transform: translate3d(0px, 0px, 0px);">
      ................

Hopefully this might be of some help? I am not a CSS guru, and I don't know off the top of my head if any of those attributes and/or styles will cause the ElementRef.scrollTop value not to be populated properly with iOS.

@manucorporat I know you have assigned this to yourself a week ago. Have you made any progress on this, or does the info I provided help at all? Please let me know if there is anything more I can do/test from my side.

0v3rst33r commented 8 years ago

@jgw96 @manucorporat please advise if there is anything I should do more from my side? I haven't gone down the CSS rabbit hole (yet), due to time limits and I might waste a lot of time as well taking my lack of CSS skills into consideration :)

0v3rst33r commented 8 years ago

@ghenry22 @jgw96 @manucorporat any feedback from anyone please?

ghenry22 commented 8 years ago

It's assigned to someone on the ionic team, I presume they are working on it, just have to wait for them to update or close of this issue in a future release.

0v3rst33r commented 8 years ago

@ghenry22 Yeah I know, assigned to @manucorporat. I was just hoping for some feedback from Ionic side, but nothing. Not that I am expecting a fix yesterday, just some feedback would be appreciated.

This issue is not even in the Beta 11 milestone. Currently I am just swamped with my own work but looks like I will have to make a fix locally on my end very soon, because my iOS testing team currently sits with an app which they can not test due to this bug.

The only conclusion I can make is that not many people out there uses Refresher + VirtualScroll, because so strange that it seems that just you and I are affected by this bug?

ghenry22 commented 8 years ago

Well it is a beta product still, even if in very good shape for a beta. We should keep in mind that it isn't even at a v1.0 yet so the occasional show stopper bug would be expected.

I think if there was a bug with collection-repeat and refresher in ionic1 you would see a lot more replies as many people use these together as they are essential for any long list to perform.

ghenry22 commented 8 years ago

Still have this same problem on iOS UIWebview with beta11

This makes any app that has a list and a refresher unusable on iOS. Given that things are progressing towards an RC stage it would be good to get this resolved.

I have tested with the new wkwebview plugin and these issues do not exist when using that. Unfortunately though wkwebview still has serious limitations which make it unusable for me at the moment (and probably for many people). Most notably cannot access files outside of the application root so there is not proper persistent storage available.

@manucorporat - any chance of getting something for this into beta12?

ghenry22 commented 8 years ago

just to confirm this happens with virtualScroll, doesn't happen on pages that do not use virtualscroll, but then I have long lists that work really well with virtualscroll and not at all otherwise.

0v3rst33r commented 8 years ago

I also have a page which will be unusable without virtualscroll. I second what @ghenry22 is saying, would be great if this can be resolved with Beta 12.

ghenry22 commented 8 years ago

@manucorporat - Interestingly uiwebview behaves a lot better after updating to ios10. So please ensure that you test with an ios9 device.

ghenry22 commented 8 years ago

@manucorporat - Still unusable on ios9 :( I don't know if you're the best person to ask about it but could a bit of focus be put towards virtualscroll on ios? Particularly with ios9 or earlier where this issue still exists.

There are a load of other virtualscroll issues on ios such as images randomly not appearing, touches while scrolling not hitting the right item etc, these are all still present on rc0.

On android virtual scroll works beautifully, on ios10 it works much better than on ios9 and earlier but on those earlier ios versions it has been unusable since it was released due to poor performance and many bugs.

I know you guys have a lot of other things to fix as well but these have been outstanding for so long and it's really broken, a little bit of love for virtual scroll would be greatly appreciated.

axelauvinen commented 8 years ago

Hey, just to point out that this is is still happening on ios9 devices. Waiting for updates and comments.

ghenry22 commented 8 years ago

Is there any slim possibility of getting something for this into RC1?

I'd really like to start testing my app on iOS properly and these virtual scroll bugs are complete blockers unfortunately.

Works really nicely on android, just ios9 or earlier with uiwebview is pretty broken.

ghenry22 commented 7 years ago

@manucorporat is there any chance someone could please look at this issue? With ionic2 at rc3 it would be really great to be able to scroll a list both up and down and with a somewhat normal scroll acceleration. I don't know how else to ask this has been broken on iOS since the virtual scroll feature was released.

ghenry22 commented 7 years ago

@adamdbradley I saw several virtualscroll fixes in RC4 and it does look like virtual scroll behaviour has improved but this conflict with the pull to refresh function still exists. If you have a pull to refresh on a virtual scroll list on UIWEBVIEW then as soon as you swipe downwards, regardless of position in list, the refresher is triggered.

Could you please take a look at this for RC5?

I do feel like I am kind of banging my head against a wall here at this point though, I love ionic but this is getting really frustrating as it's blocking me from even testing my app properly in preparation for release on iOS. It all works perfectly on Android for me.

ghenry22 commented 7 years ago

@adamdbradley The issue with refresher is due to this line in the refresher component: var scrollHostScrollTop = this._content.getContentDimensions().scrollTop;

scrollTop always returns 0 when using virtualscroll on UIWEBVIEW. Behaves normally without virtualscroll or on android.

Looks like the refresher component is not listening for js scroll events so never updates as virtualscroll uses js scrolling. I have put a bit of logging in locally and I can see that virtualscroll listens for jsscroll events and updates scrolltop internally based on them.

Refresher does not listen for these events so never updates scrollTop when js scrolling is in use.

ghenry22 commented 7 years ago

animated gif showing scroll acceleration and refresher conflict with a simple static list of data on RC4. Captured on iphone7 sim with ios 10. Same on real device. Worse on ios 9.

test2

ghenry22 commented 7 years ago

Scroll acceleration is now fine, refresher issue is covered in #9765, closing this ticket

ionitron-bot[bot] commented 6 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.