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

Restoring scroll position not reliable #5

Closed
anormal81 opened this issue Mar 24, 2022 · 20 comments
Closed

Restoring scroll position not reliable #5

anormal81 opened this issue Mar 24, 2022 · 20 comments

Comments

@anormal81
Copy link

Hello all. Most of flutter_list_view works fine, but I have the following problem:

In my app, there is a list of chat messages that are rendered as HTML using flutter_html. Each message can have its own height, which I can't set myself.

When I try to initialize the list with initIndex and initOffset, sometimes the scroll position is just wrong. This depends on the preferItemHeight value. If I set a small value, the scroll position is fine when there are only single-line messages, but breaks for large messages. If a large value is set, the scroll position is fine when there are large values, but has problems when there are many one-line items.

It seems that the initial scroll position is restored with preferItemHeight or onItemHeight(...), just before the actual height of the item is known.

I tried to create a reproducible example, but was not able to create this behavior with test data.

Code I can currently share:

FlutterListView(
      key: ObjectKey(...),
      controller: viewController,
      delegate: FlutterListViewDelegate(
        (BuildContext context, int index) {
         ...
          return Container(
            // container is required, otherwise there will be layouting errors
            child: XXX(),
          );
        },
        onItemKey: (index) {
         ...
        },
        onItemHeight: (index) {
         ...
        },
        childCount: itemCount,
        initIndex: initialScrollIndex,
        initOffset: initialAlignment,
        firstItemAlign: FirstItemAlign.end,
      ),
    );

I hope you find an approach to find the problem

@robert-bitguild
Copy link
Collaborator

@anormal81 I can't reproduce your issue after I tried to write a sample like https://github.com/robert-luoqing/flutter_list_view/blob/master/example/lib/init_jump_after_loaddata.dart. Could you please check the code and help me to reproduce it?

The two things you need to be aware of:

  1. The preferItemHeight and onItemHeight are not related to the issues. You don't need to set the two properties necessarily. Because the list item's height will be recorded after it is rendered, otherwise, the item's height will be set to preferItemHeight

  2. initIndex will work in two situations
    a. When childCount from 0 to non-zore, the widget will scroll to [initIndex].
    b. When initIndex changed and the child count is not 0, the widget will scroll to [initIndex].

BTW:Does your list item's height can be changed after mount?

Thanks.

@anormal81
Copy link
Author

Thanks for your response. I'm very new to flutter, so maybe I made some huge mistakes and need more understanding in how everything works together.

My data is a list of up to 200 items. When scrolling, the data changes, because the server returns a rolling window of max 200 items. After I got new data from the server, the code does the following:

  1. Saving a scrolling state (the first visible items (offset > 0) with its offset and id of the entry in my data list)
  2. calling setState on my widget which contains the FlutterListView.
  3. build function of the widget determines the new index and offset from the saved scrolling state and sets this index and offset as initialScrollIndex and initOffset

FlutterListView has set an ObjectKey with the hashCode of the data. As far as I understand, that will rebuild the FlutterListView when the data have changed.

In this setup, preferItemHeight and onItemHeight have an effect on how scrolling behaves.

I'm not sure whether the height may change. I'm using flutter_html to display the formatted data.

I will try to check your example. Maybe I will find a way to reproduce it or find what I'm doing wrong here ;-)

@anormal81
Copy link
Author

@robert-bitguild I've just modified your example to simulate the behavior of the server API. I've attached the file.

jump.txt

As you can see, the list starts by scrolling to the end of the 100 entries (index 99). When you press the button, a new list is generated with the old data. A new entry is inserted at index 0 of the list.
A registered listener will save the visible positions. At button press the scroll state is saved (uses the second visible element with offset>0)

The problem:
When having the list scrolled that only half of the 98er element is visible at the bottom of the viewport and press the button, the list will scroll to the end.

But:
When you leave the list scrolled to the end or having e.g. item 85 visible, the button press will correctly restore the scroll position.

@robert-bitguild
Copy link
Collaborator

@anormal81 After I checked your code. What you want to implement is called Keep Position
Keep position: The widget will not scroll down when some items insert before the current display item.
I have made an example to you, please check
https://github.com/robert-luoqing/flutter_list_view/blob/master/example/lib/init_jump_keep_position.dart

Please check https://github.com/robert-luoqing/flutter_list_view#keep-position to know more about keep position

@robert-bitguild
Copy link
Collaborator

robert-bitguild commented Mar 25, 2022

@anormal81 BTW, Please remove Scrollbar around flutterListView. An issue exist in ScrollBar. See: flutter/flutter#97873

@robert-bitguild
Copy link
Collaborator

@anormal81 Please update flutter_list_view: ^1.1.5. The issue you post has been fixed. Of cause, you also can use "Keep position" functionality to resolve your issue.

@anormal81
Copy link
Author

anormal81 commented Mar 25, 2022

@robert-bitguild Thanks so much! It works great now in my app.

I tried using keepPosition, but due to the rolling window, the whole list may change. Also some entries may get a different height when previous items are added.

There seems to be another problem when scrolled to the top position. Could you please check again with my already provided code sample jump.txt

  1. Set preferItemHeight: 10
  2. Manually scroll to the top position (Update: Bottom position seems to be a problem too)
  3. Press the button to insert an item and try to restore the scroll position
  4. The scroll position is not restored to the correct offset

When setting the preferItemHeight to a larger value which will not be exceeded by the items, the scroll position seems fine.
In my app I can't define how the height of the items will be. Could be single line or a lot of content.

Would be great if you could fix that too.

Many, many thanks

@robert-bitguild
Copy link
Collaborator

@anormal81 The issue is not a bug, because you have set key to making FlutterListView become a different element.

You can remove key or hardcode a key like below.

child: FlutterListView(
scrollDirection: Axis.vertical,
// key: ObjectKey(data.hashCode),
key: ObjectKey("ListView"),
controller: viewController,
delegate: FlutterListViewDelegate(
(BuildContext context, int index) {

@anormal81
Copy link
Author

anormal81 commented Mar 25, 2022

Yes, I've set the key to ensure that the whole list rebuilds when the data in background changes. As mentioned, I can not ensure, that existing elements will always stay the same height. The content of the items can have changed from e.g. single line to 2000 lines of content. Without the key with the hashCode, the scroll positions could not be updated properly.

In my sample, I now removed the key completely and tried again. Scrolling item 97 or 96 to be partly visible at the bottom will also jump to another position after a button press (not with each press, had to try different positions. Seems to depend on the generated data in the list). preferItemHeight is set to 10. As I understand you, this should not happen without having a key set?

So to understand it correctly: Initially the list will use the preferItemHeight or onItemHeight values to create the scroll positions? If I set them too low, there will be problems setting the correct initial scroll positions?

Do you see any other possibility for my scenario? My app behaves like a messenger app, initially scrolled to the most recent message. The server provides a rolling window while scrolling or data changed by other clients. If the list of data does not contain the first and/or last message, a loading indicator is added as top and/or bottom item. Its size will be different as soon as the server has sent an updated list of data that will be visible.

@robert-bitguild
Copy link
Collaborator

The widget is not what you said to follow preferItemHeight and onItemHeight. You can just remove it if the item height is not stable.

The item height is not fixed after it is rendered. It is based on your list item.

@anormal81
Copy link
Author

I've attached an updated sample. preferItemHeight ist set to 10 (which should have no impact as you said. 50 is the default and should not make any difference here). I removed the ObjectKey. Items are now larger (min. 10 lines)

jump2.txt

The both attached screenshot show the scroll position before and after the button was pressed.

Before:
before

After:
after

Expected: The scroll position must be restored to the correct offset.

Can you please check what causes the wrong position here? There seems to be a case where a wrong offset is calculated.

PS: With the ObjectKey active and preferItemHeight=200, the scroll positions are fine after button press (when scrolled to e.g. 96). With preferItemHeight=10, the positions sometimes is wrong. If the initial scroll position (based on initIndex and initOffset) is correctly calculated, the items should not have moved.

@robert-bitguild
Copy link
Collaborator

robert-bitguild commented Mar 25, 2022

@anormal81 I made a full chat example for you. Please check https://github.com/robert-luoqing/flutter_list_view/blob/master/example/lib/chat.dart

If you don't want jump to last read after data loaded, just remove initIndex. You can manual invoke listViewController.sliverController.jumpToIndex(lastReadMessageIndex) when some button clicked;

BTW: Need to update the package to flutter_list_view: ^1.1.6

@anormal81
Copy link
Author

Thanks so much for your example. I will analyze it and try to convert my code into something similar. The key difference seems to be the reverse order which prevents to always trying to scroll to the end. I hope there is also the difference why my list has some problems restoring the right positions.

I will give feedback when I'm done. 😃

@anormal81
Copy link
Author

anormal81 commented Mar 25, 2022

@robert-bitguild I've restructured a lot of my code, reversed the list of data, and so on. Now I have a working list with no strange scroll positions.

KeepPosition works now. The big differences that I did not understand before were:

  1. use a reverse list and always scroll to the top instead of bottom
  2. keepPositionOffset: >0 must be set to automatically scroll to the new messages

One suggestion: When the list of data is getting smaller (e.g. messages removed), I got an RangeError (index): Invalid value: Not in inclusive range 0..113: 134 durch onItemKey. It asks an index that is greater than the list of data. So I have to return "" here. The listView implementation better handles this, because "" is really not unique ;-)

Now only one feature is not working as expected, maybe you have an idea:
When scrolling up to the top position, more messages are loaded from the server. During the loading time, a loading indicator is displayed. When the messages are received and the data list is updated, the indicator is replaced with the previous messages and the scroll position kept the offset. So scrolling up works like expected.
But: When scrolling back to the newer messages, the list is scrolled to the bottom. A loading indicator is available at index 0 to show that there are more messages incoming. When the new data is received and updated, the list keeps the scroll position at the bottom. Therefor it is scrolled to the end again, until all messages are loaded.
Expected is, that the messages, that remain in the messages list, also remain in the UI. In the screenshot below, the message "grrr" should remain at this position and the loading indicator should be replaced by the new messages. Currently the scroll position keeps the loading indicator visible and triggers loading further messages.

Is there anything that can be set using your list view to control this behavior?

Screenshot_20220325-152702

The used settings:

return FlutterListView(
      reverse: true,
      controller: viewController,
      delegate: FlutterListViewDelegate(
        (BuildContext context, int index) => _renderItem(index),
        childCount: messageIds.length,
        onItemKey: (index) {
          if (index >= messageIds.length) {
            return '';
          }
          return messageIds[index];
        },
        keepPosition: true,
        keepPositionOffset: 10,
        initIndex: 0,
        initOffset: 0,
        initOffsetBasedOnBottom: true,
        firstItemAlign: FirstItemAlign.start,
      ),
    );

Thanks so much for the time your are investing. It helps a lot!

@robert-bitguild
Copy link
Collaborator

@anormal81 I have added loadmore functionality into chat example. You can update it to review the code. The onItemKey issue has fixed, please update to flutter_list_view: ^1.1.7

BTW: If you don't need keepPosition, just set keepPosition: false,
keepPositionOffset, It is mean if you read messsages which is not on top, You should not scroll up or down to disturb user to read.
For example if you set keepPositionOffset=40, The mean if scroll size less 40, It will scroll up if new message comming, else it will keep not scrolling

@anormal81
Copy link
Author

@robert-bitguild Thanks again.

Version 1.1.7 does not fix the Range Error.

Implementation in MessageListState:

childCount: messageIds.length,
onItemKey: (index) => messageIds[index],

Stack:

List.[] (growable_array.dart:281)
MessageListState.build.<anonymous closure> (conversation.dart:187)
FlutterListViewElement.getKeyByItemIndex (flutter_list_view_element.dart:330)
FlutterListViewRender.findIndexByKeyAndOldIndex (flutter_list_view_render.dart:50)
FlutterListViewRender._handleKeepPositionInLayout (flutter_list_view_render.dart:315)
FlutterListViewRender.performLayout (flutter_list_view_render.dart:107)
RenderObject.layout (object.dart:1887)
RenderViewportBase.layoutChildSequence (viewport.dart:510)
RenderViewport._attemptLayout (viewport.dart:1580)
RenderViewport.performLayout (viewport.dart:1489)
RenderObject._layoutWithoutResize (object.dart:1731)
PipelineOwner.flushLayout (object.dart:887)
RendererBinding.drawFrame (binding.dart:497)
WidgetsBinding.drawFrame (binding.dart:883)
RendererBinding._handlePersistentFrameCallback (binding.dart:363)
SchedulerBinding._invokeFrameCallback (binding.dart:1144)
SchedulerBinding.handleDrawFrame (binding.dart:1081)
SchedulerBinding._handleDrawFrame (binding.dart:995)
_rootRun (zone.dart:1426)
_CustomZone.run (zone.dart:1328)
_CustomZone.runGuarded (zone.dart:1236)
_invoke (hooks.dart:151)
PlatformDispatcher._drawFrame (platform_dispatcher.dart:308)
_drawFrame (hooks.dart:115)

Getting back to my scenario:

Setting keepPosition to off will create wrong scroll positions. The same in your chat sample when you disable keepPosition and use the "Mock To Receive" action from the AppBar.

The added loadmore functionality in the chat sample only works to load more older messages. In my case, I have a rolling window, which means that the data list contains up to 200 messages, not more. When scrolling to a really old message, only 99 previous and 100 newer messages will be available in the list. There could be a lot more newer messages that are not available on client side. Scrolling and loading of messages will be available in both directions.

In the current case, a loading indicator is available at the bottom of the list, as shown in my previous screenshot. When the user scrolls to the end of the current list, new data will be loaded. When new data is available, the list should stay at exactly the position of the valid messages, not the loading indicator. In result, the user will be able to scroll further to the bottom to see the newer messages.

  • keepPosition: false did not change anything except its breaking the position when older messages are added.
  • using a unique key for each loading indicator had no influence

Do you think that an enhancement would be possible to e.g. define per function which element is allowed to be used for keep position?

Here are two screenshots to show the expected behavior (not the current behavior):

User scrolled to the end of the list, but there are more messages available on server side
before2

New list of messages was sent to the client. The list keeps the positions of the visible items (identified by their key)
after2

@anormal81
Copy link
Author

anormal81 commented Mar 27, 2022

After several different tries to get it working with keepPosition, I returned to my initial idea with setting an ObjectKey in order to create a new listView each time the data has changed. With this implementation I first need to save the first visible item including the offset and setting the new index and offset in the build call.

Restoring the correct scroll position seems to work better when using a reverse list and initOffsetBasedOnBottom: true.

In conclusion: I implemented my own keepPosition ;-)

@robert-bitguild
Copy link
Collaborator

It is great. You can using initIndex to keep position. lol.
About refresh to keep position, I also has made two sample. First one using initIndex, second one is used keep position. It may give you some idea about it.

https://github.com/robert-luoqing/flutter_list_view/blob/master/example/lib/chat.dart
https://github.com/robert-luoqing/flutter_list_view/blob/master/example/lib/chat2.dart

@anormal81
Copy link
Author

anormal81 commented Mar 27, 2022

Thank you. I'll have a look the next days. The first example with initIndex seems to be similar to my implementation.

Will let my co-workers test the current implementation. Hopefully they will not find another problem here and I can go on implementing further features ;-)

@anormal81
Copy link
Author

I just updated to Version 1.1.11

Did not found another problem with scrolling in my current implementation. The Range Error is fixed too.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants