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.01k stars 13.51k forks source link

bug: insertBefore, prepend, seemless infinite scrolling up not working #22291

Closed mightytyphoon closed 4 years ago

mightytyphoon commented 4 years ago

Bug Report

Ionic version: [x] 5.3.4

Current behavior: When Inserting an element using element.insertBefore() the container view will always try to go to top even if not asked to.

Expected behavior: When inserting a new element while not being at the top scroll of the container (container.scrollTop > 0) the container view shouldn't scroll back to top and keep its position regardless of elements being added on top.

Steps to reproduce: use element.insertBefore()

Here is a repo to tryout the problem on iOS only for now Repo showing the problem

Related code:

Other information:

I found a way to try to fix the issue but it's not very good and I don't recommend it :

Use gestures to scroll from one element to another and insert elements when the scroll is stopped, at the moment of inserting the element, take the actual scroll + the height of the element (el.getBoundingClientRect().height) and add it to present scrollTop of the container. This way every time an element is added ionic will scroll to top and we scroll back to the element. Controlling the scroll let us stop, insert, refresh scrolling, let user able to scroll again. BUT this wasn't the wanted behavior.

Ionic info:

Ionic:

   Ionic CLI                     : 6.11.11 (/usr/local/lib/node_modules/@ionic/cli)
   Ionic Framework               : @ionic/angular 5.3.4
   @angular-devkit/build-angular : 0.1000.8
   @angular-devkit/schematics    : 10.0.8
   @angular/cli                  : 10.0.8
   @ionic/angular-toolkit        : 2.3.3

Capacitor:

   Capacitor CLI   : 2.4.2
   @capacitor/core : 2.4.2

Utility:

   cordova-res : not installed
   native-run  : not installed

System:

   NodeJS : v14.12.0 (/usr/local/bin/node)
   npm    : 6.14.8
   OS     : macOS Catalina

Regards.

liamdebeasi commented 4 years ago

Thanks for the issue. I am going to close this as this is not a bug in Ionic Framework. Ionic Framework does not alter your scroll position when using Element.insertBefore, additionally I was able to reproduce this issue with a regular scrollable div outside of Ionic Framework.

I recommend posting your question on our forums: http://forum.ionicframework.com/

mightytyphoon commented 4 years ago

Thanks for the issue. I am going to close this as this is not a bug in Ionic Framework. Ionic Framework does not alter your scroll position when using Element.insertBefore, additionally I was able to reproduce this issue with a regular scrollable div outside of Ionic Framework.

I recommend posting your question on our forums: http://forum.ionicframework.com/

Thanks for you answer, I will post this in ionic forums.

But in my opinion since ionic tries to handle everything with ionic-view it should be part of it, there should be a parameter like "scrollOnInsert" with true or false value in ionic-content.

The problem is : I will have to handle the whole thing myself just to make seemless infinite scroll in both directions to work (iOS calendar style). Which mean taking care of gestures and acceleration and I wouldn't have to do it if ionic-content or any scrollable div in ionic would work as a scrollable div in chrome.

So in my honest opinion it is definitely something that should be handled by ionic-content because it already has methods such as "scrollUp" or "scrollBottom", "scrollOnInsert" should be a thing.

Regards.

liamdebeasi commented 4 years ago

When it comes to restoring/preserving scrolling position, we are of the opinion that this should be handled by the browser where possible and the framework should not intervene. In the past we have tried to do this and it tends to interfere with user interactions on the screen.

That being said, you should not have to worry about gestures and acceleration. Before you call insertBefore you can read the scroll position of the page. Immediately after calling insertBefore you can scroll the page to the cached scroll position.

mightytyphoon commented 4 years ago

When it comes to restoring/preserving scrolling position, we are of the opinion that this should be handled by the browser where possible and the framework should not intervene. In the past we have tried to do this and it tends to interfere with user interactions on the screen.

That being said, you should not have to worry about gestures and acceleration. Before you call insertBefore you can read the scroll position of the page. Immediately after calling insertBefore you can scroll the page to the cached scroll position.

Thanks for your concern.

This is really not as easy as you may think, I tried exactly what you proposed but this doesn't work well when scrolling on mobile because the momentum is kept, so starting the scroll, then detecting the position, spawning a angular component, putting it back on top, taking the scroll position from before and then adding it makes the whole thing really bad with flashing components and a sort of flickering as well as bouncy feeling and jumps.

So, to take care of this, I will have to totally control the scroll, the accelerations and momentums through hammerjs, I don't see any other possibility, which means a bit of work where it could just have the behavior of a chrome browser DIV and I wouldn't have to use a week working on a seemless navigation. Because my only problem here is really just the scroll position going top whenever I insertElement above scroll position, on bottom direction there is no problem.

Regards.

liamdebeasi commented 4 years ago

You might want to give ParentNode.prepend a try.

I made a CodePen that uses ion-infinite-scroll and ParentNode.prepend and it works pretty well on Chrome and Safari: https://codepen.io/liamdebeasi/pen/yLJYorJ

You might notice a slight flicker when inserting elements, but otherwise both browsers return the view to the previous scrolling position. Perhaps you can use one of the ion-content scrolling methods to restore the scroll position quicker?

mightytyphoon commented 4 years ago

You might want to give ParentNode.prepend a try.

I made a CodePen that uses ion-infinite-scroll and ParentNode.prepend and it works pretty well on Chrome and Safari: https://codepen.io/liamdebeasi/pen/yLJYorJ

You might notice a slight flicker when inserting elements, but otherwise both browsers return the view to the previous scrolling position. Perhaps you can use one of the ion-content scrolling methods to restore the scroll position quicker?

Thanks a lot for the time you use on this.

Unfortunately I don't want the scroll to block with a loading animation. I am trying to make the same flow as iOS calendar which permits you to scroll indefinitely through months while they are spawned on the go without loading. In this behavior there shouldn't be a loading guizmo or scrolling being stopped. The user should be able to scroll indefinitely through past months by swiping up without loading time as the months are created procedurally, there aren't any requests sent needing a loading time.

Thank you for the codepen it's very kind anyway. I will try to use prepend as well as scrolling methods of ionic-content and keep you updated.

Regards.

liamdebeasi commented 4 years ago

Ok sounds good. You could also listen for the ionScroll event and do this loading logic yourself without ion-infinite-scroll (make sure you have <ion-content scroll-events="true">..</ion-content>). You would need to restore the scroll position yourself, but you could copy the logic we use for ion-infinite-scroll as it should still be applicable: https://github.com/ionic-team/ionic-framework/blob/master/core/src/components/infinite-scroll/infinite-scroll.tsx

mightytyphoon commented 4 years ago

I give an update in case someone comes to this issue, wanting to do the same thing as me.

First, (mea culpa) it's not a problem from Ionic, it's really a problem on how scrolling momentum is handled in html elements.

You can check this example in a mobile or using dev tools / mobile view. https://chase0213.github.io/nic-demo/ Every time you scroll up (past direction) you will notice there is the momentum you get from scrolling on a mobile device then a stop is needed to load the previous month, giving a really bad user experience imo. Even worse if you keep your finger in position, it will spawn months in a very ugly way one by one.

Now I found this one interesting : http://clauderic.github.io/react-infinite-calendar/#/basic-settings/basic-configuration?_k=4x9and You can scroll up for a long time without feeling these stops, but it's actually a trick, dates are loaded in page until 1980, then the calendar stops. It's the best (easy fast) trick so far and it's possible to optimize it by making full sets of months in html directly in the page. Of course you need to take care of localization. But yeah for now the best trick is actually to create a full set of past months in plain html from 1900 to 2020 for example and directly load it in the page (putting the html right away or using innerHTML)

Problem is : this works with data that don't need to be changed or loaded dynamically (like past dates). So ? Is there a way to make a seemless infinite scroll up with elements being loaded above top of screen without any stops during scrolling ? Using divs it's really a complete no for me, I tried many things such as using overflow:hidden and trying to handle the scrolling with my own script, detecting wheel or swipe up / down but even with that you can't handle the scrolling as you really want.

For now I will stick with the trick described above but I think it could be possible using canvas to do a scroll engine. The canvas should record swipe up / down and other gestures and then render elements, that's a big work but it's the only way to have infinite scroll in both directions while keeping momentum and a good fps as well as avoiding bounces + spawning dynamic data into newly prepended elements.

Maybe I will try to do it when I have more time but in case someone wants to achieve this, canvas is the way to go, imo.

Happy Coding !

ionitron-bot[bot] commented 3 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.