element-x-ios icon indicating copy to clipboard operation
element-x-ios copied to clipboard

Scroll jumps during backpagination

Open ara4n opened this issue 1 year ago • 2 comments

Steps to reproduce

  1. Scroll backwards
  2. Run out of scrollback and view a spinner while backpaginating from the server
  3. Observe that when the spinner disappears the gap suddenly pops out of existence, causing a scroll jump
  4. Observe that if you are transitioning into a bubble continuation (ie the topmost bubble becomes a continuation), the sender profile needs to be animated away. This also seems to cause a jump rather than being a smooth transition.

Look frame by frame:

https://github.com/element-hq/element-x-ios/assets/1294269/1457c096-03f5-4ed9-be6a-1b1aa55cf417

Critically: the position of the topmost bubble should remain constant on the screen during pagination, even if the sender gets removed, the loading spinner gets removed, and new scrollback get loaded. Otherwise it’ll be perceived as a jarring jump.

Outcome

What did you expect?

smooth scrolling

What happened instead?

scroll jumps.

@pixlwave improved this in build 604, but it’s still jumpy

Your phone model

No response

Operating system version

No response

Application version

604

Homeserver

No response

Will you send logs?

No

ara4n avatar May 22 '24 13:05 ara4n

From looking at the video I would call this expected behaviour. When you're in the overscroll area, the scrollview is going to snap back to the resting position and I think it's fair to say that iOS users are happy with that behaviour.

pixlwave avatar May 30 '24 14:05 pixlwave

Let's check with design if this behaviour is to be expected.

Velin92 avatar Jun 03 '24 09:06 Velin92

So I had a look at it while digging into https://github.com/element-hq/element-x-ios/issues/4127 - I think @pixlwave may have misunderstood the video: the point is that there's a nasty scroll jump at frame 10, at the point that the PaginationIndicatorRoomTimelineView transitions out. A hacky solution which fixes this is to make the spinner zero-height, so that as it comes & goes it doesn't cause scroll jumps:

--- a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/PaginationIndicatorRoomTimelineView.swift
+++ b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/PaginationIndicatorRoomTimelineView.swift
@@ -11,9 +11,11 @@ struct PaginationIndicatorRoomTimelineView: View {
     let timelineItem: PaginationIndicatorRoomTimelineItem
     
     var body: some View {
-        ProgressView()
-            .frame(maxWidth: .infinity)
-            .padding(.top, 12) // Bottom spacing comes from the next item (date separator).
+        ZStack {
+            ProgressView()
+                .frame(maxWidth: .infinity)
+        }.frame(maxHeight: 0)
+//            .padding(.top, 12) // Bottom spacing comes from the next item (date separator).
     }
 }

However, this is cosmetically not idea (it looks like this):

Image

...but at least there's no jump once the spinner disappears. Thoughts welcome on how to show the spinner as an overlay which isn't cosmetically dirty, and then between this and https://github.com/element-hq/element-x-ios/issues/4127#issuecomment-2889276967 this bug could be closed.

ara4n avatar May 19 '25 17:05 ara4n

Ohhhh, is that what's causing it!

I think an tidier solution here would be to always add the PaginationIndicatorRoomTimelineItem when we're idle as well as paginating backwards, and then give it a property isPaginating along with an .opacity(timelineItem.isPaginating ? 1 : 0) modifier on the ProgressView.

pixlwave avatar May 19 '25 17:05 pixlwave

@ara4n This would be where to modify the insertion if you want to give it a go: https://github.com/element-hq/element-x-ios/blob/2f938d3cbf5c3ebe2afcbc48222c59cc30147e80/ElementX/Sources/Services/Timeline/TimelineController/TimelineController.swift#L436-L441

pixlwave avatar May 19 '25 17:05 pixlwave

hm - but won't it still pop when you inevitably have to remove the spinner, even if it's transparent? otherwise you'll have a gap in the timeline?

ara4n avatar Jun 06 '25 12:06 ara4n

Sure, but in practice when you can see the top of the timeline, the app should either be paginating more items or you should be looking at the start of the room. So it would be a reasonable approximation (in my eyes) to say that you reserve the space for the spinner until it can't be spun anymore (i.e. you've reached the start of the room).

pixlwave avatar Jun 06 '25 12:06 pixlwave