Skip to content
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

bug: infinite-scroll with position="top" causes flicker when restoring scroll position #28182

Open
3 tasks done
j-oppenhuis opened this issue Sep 15, 2023 · 14 comments
Open
3 tasks done
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@j-oppenhuis
Copy link

Prerequisites

Ionic Framework Version

v4.x, v5.x, v6.x, v7.x

Current Behavior

When using the ion-infinite-scroll element in an ion-content element that has styling to display: flex and flex-direction: column-reverse. The ionInfinite emitter will never be triggered.

Expected Behavior

I expect that the ion-infinite-scroll emitter to be called like normal when position is bottom. Because when column-reverse is used the bottom is the top, so I think the ion-infinite-scroll element should account for that.

Steps to Reproduce

  1. Create an ion-content that has the column-reverse styling
.container::part(scroll) {
  display: flex;
  flex-direction: column-reverse;
}
  <ion-content class="container">
    <div class="ion-padding">
      <h1>Test</h1>
    </div>
    <div class="ion-padding">
      <h1>Test</h1>
    </div>
    <div class="ion-padding">
      <h1>Test</h1>
    </div>
    <div class="ion-padding">
      <h1>Test</h1>
    </div>
    <div class="ion-padding">
      <h1>Test</h1>
    </div>
    <div class="ion-padding">
      <h1>Test</h1>
    </div>
    <div class="ion-padding">
      <h1>Test</h1>
    </div>

    <app-infinite-scroll>
      <ion-infinite-scroll-content></ion-infinite-scroll-content>
    </app-infinite-scroll>
  </ion-content>
  1. If you scroll up infinite scroll will not be called.

Code Reproduction URL

No response

Ionic Info

Ionic:

Ionic CLI : 7.1.1
Ionic Framework : @ionic/angular 7.2.3
@angular-devkit/build-angular : 16.2.0
@angular-devkit/schematics : 16.2.0
@angular/cli : 16.2.0
@ionic/angular-toolkit : 9.0.0

Capacitor:

Capacitor CLI : 5.1.1
@capacitor/android : 5.1.1
@capacitor/core : 5.1.1
@capacitor/ios : 5.1.1

Utility:

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

System:

NodeJS : v16.19.0
npm : 8.19.3
OS : macOS Unknown

Additional Information

The scrollTop of the scrollElement is negative in the infinite scroll element. That is why the distanceFromInfinite is not correctly determined. What we could do, is use the absolute value of the scrollTop value. When this is implemented bottom position will work when flex-direction: column-reverse is used.

@ionitron-bot ionitron-bot bot added the triage label Sep 15, 2023
@averyjohnston
Copy link
Contributor

Thank you for the issue. Can you give details on your use case for reversing the content's direction?

@averyjohnston averyjohnston added the needs: reply the issue needs a response from the user label Sep 15, 2023
@ionitron-bot ionitron-bot bot removed the triage label Sep 15, 2023
@j-oppenhuis
Copy link
Author

When creating a chat for example. We could have a footer where we put in a text and in the ion content we may only have messages. When the ion-content is reversed, the messages will also be shown reversed, also the scrollbar will automatically be at the end.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Sep 15, 2023
@averyjohnston averyjohnston self-assigned this Sep 15, 2023
@averyjohnston
Copy link
Contributor

Thank you, that makes sense. I am able to replicate the described behavior, but it sounds like a better fit for your use case would be to use the position property of ion-infinite-scroll (docs here).

One way to accomplish this pattern would be to place the infinite scroll element at the top of the content, then wrap the chat items in a separate element and apply flex-direction: column-reverse to that instead. Here's an example showing a modified version of the first infinite scroll demo: https://stackblitz.com/edit/angular-jfai5a-9p8rxx?file=src%2Fapp%2Fexample.component.css,src%2Fapp%2Fexample.component.html

Can you give that pattern a try and let me know if it works for your use case?

@averyjohnston averyjohnston added the needs: reply the issue needs a response from the user label Sep 15, 2023
@averyjohnston averyjohnston removed their assignment Sep 15, 2023
@ionitron-bot ionitron-bot bot removed the triage label Sep 15, 2023
@j-oppenhuis
Copy link
Author

Thank you for checking. I won't be able to check this this weekend. Sorry for that, monday I can check if this works for my use case.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Sep 15, 2023
@averyjohnston averyjohnston added the needs: reply the issue needs a response from the user label Sep 15, 2023
@ionitron-bot ionitron-bot bot removed the triage label Sep 15, 2023
@j-oppenhuis
Copy link
Author

I have checked your solution. There are a few things that do not work as I expect.

  1. When I use an api there will be a delay when getting the initial value. To recreate this, in the ngOnInit we call this.generateItems() in a setTimeout. The problem that now arises is that the scrollbar will be at the top instead of the bottom.
  2. When scrolling to the top on my desktop, no new items will be added to the top. Only when I scroll down and up again the items are added. Video: https://recordit.co/bSeBtqbqCq

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Sep 18, 2023
@averyjohnston
Copy link
Contributor

For item 1, could you provide a Stackblitz link or Github repo demonstrating this? You can use another setTimeout or similar to add an artificial delay to generateItems() or wherever else the async action is normally performed.

For item 2, I was able to replicate that behavior by scrolling fast enough, but it also occurs with position="bottom". I'll either use this issue to track that or create a new one, depending on how item 1 goes. (There are also other existing issues that seem adjacent to that bug, but I need to dig into them further.)

@averyjohnston averyjohnston added the needs: reply the issue needs a response from the user label Sep 20, 2023
@ionitron-bot ionitron-bot bot removed the triage label Sep 20, 2023
@j-oppenhuis
Copy link
Author

Thank you for looking!
1: I created an example using your fork -> https://stackblitz.com/edit/angular-jfai5a-cmuxus?file=src%2Fapp%2Fexample.component.css,src%2Fapp%2Fexample.component.ts
2: That sounds good

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Sep 20, 2023
@averyjohnston
Copy link
Contributor

Apologies for the delay; I'm able to replicate item 1 but I'm still digging into what the best fix would be.

For item 2, we now have #18071 to track that, so I'll use this issue solely to track item 1.

@averyjohnston
Copy link
Contributor

Alright, so, the solution to item 1 is to call ion-content's scrollToBottom method after the items have been initialized. I've created a new demo here: https://stackblitz.com/edit/angular-jfai5a-wmvdyw?file=src%2Fapp%2Fexample.component.ts,src%2Fapp%2Fexample.component.html,src%2Fapp%2Fexample.component.css

There are a few notable pieces to this:

  1. generateItems has been updated to return a Promise, to bring the code closer to a real life scenario.
  2. ngOnInit now contains a MutationObserver which waits for the list to be fully hydrated before scrolling to the bottom. Otherwise, the list would have a height of 0, so the scroll wouldn't do anything. This part is only needed if the list items are ion-items or other Ionic components; since you used divs in your examples, you may not need this.
  3. scrollToBottom is called at the bottom of ngOnInit, after generateItems has completed.

Can you see if this pattern works for you? Let me know if you have any questions.

@averyjohnston averyjohnston added the needs: reply the issue needs a response from the user label Oct 17, 2023
@ionitron-bot ionitron-bot bot removed the triage label Oct 17, 2023
@j-oppenhuis
Copy link
Author

Thank you for further looking into the problem. It seems to work now. But it seems that there are two things that I don't like about this approach.

  1. The scrollToBottom is needed, this should not be necessary when using flex-direction: column-reverse.
  2. When scrolling up the items seem to flicker, that is probably related to the following code: https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/infinite-scroll/infinite-scroll.tsx#L155. This code is possibly not needed when using flex-direction: column-reverse? That is why I wanted to use the ion-infinite-scroll without position="top".

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Oct 18, 2023
@averyjohnston
Copy link
Contributor

Using flex-direction: column-reverse as in your original implementation is not feasible with ion-infinite-scroll because the ionInfinite event does not fire, as you originally reported. This is due to the position property being set to the wrong side, and position="top" is required so that ion-infinite-scroll knows whether to perform the scroll correction you linked to in item 2. This scroll correction would be needed even if the ionInfinite event could be made to fire in your original implementation.

As for the flicker, we may be able to improve the experience there. I'll update this issue to track investigating that, and we'll let you know when we have more to share.

@averyjohnston averyjohnston changed the title bug: Infinite scroll with position bottom in ion-content with flex-direction column-reverse does not trigger bug: infinite-scroll with position="top" causes flicker when restoring scroll position Oct 26, 2023
@j-oppenhuis
Copy link
Author

I know that the calculation of the position bottom is different from the top one. But I created a simple infinite scroll based on the Ionic infinite scroll code. What I noticed is that the calculation for the bottom position is fine when used with flex-direction: column-reverse. The problem is that the scrolltop is negative. It seems that scrolltop doesn't respond to negative values as read here: https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTop. So, when I made the scrolltop value an absolute value by using the Math.abs() function it seems to work fine when using flex-direction: column-reverse with position="bottom". I don't really know if that is a good solution or not, I know that using position="bottom" sounds weird, but in reality it is of course the bottom when everything is reversed.

If the flicker can be resolved that would also be great!

@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Feb 7, 2024
@ionitron-bot ionitron-bot bot removed the triage label Feb 7, 2024
@anilanar
Copy link

anilanar commented May 9, 2024

Applying flex: column-reverse on the element that does the scrolling (i.e. overflow-y: auto) is very important as that has a huge impact on how scroll anchoring works.

Therefore, I suggest Ionic team to add a property to ion-content for reversing the scroll direction and adjust ion-infinite-scroll accordingly so that it would work when scroll is reversed.

@jemantilla
Copy link

Having the same problem right here. +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests

5 participants