fix side view offset when main offset does not start at 0

[joachimvalente]

a few changes:

• fix side view offset when main offset does not start at 0
• fix typo itemExtend -> itemExtent
• remove listener on dispose, and only create ScrollController on initState (not on build)
• modernize dart style with no new, and with => shorthand notation where possible

[renefloor]

I agree with your changes to clean the code, and I absolutely agree with creating the controller in initstate, but I don't really understand this problem and how you fixed it:

fix side view offset when main offset does not start at 0

I don't see any significant change in the build method or in the calculation of the current position. But that's partly because so many lines changed without really changing anything that I have a hard time finding the real change. 1 tip for next time is to do such big formatting changes separately from the functional changes.

[renefloor]

I don't think this print is a good idea.

Suggested change

print('new position = ${(_controller.offset / widget.itemExtent).floor()}');

[joachimvalente]

yep that was an old commit (I push another one since then)

[renefloor]

Can't we do this in the build method? We might need to set some property if the position is set or not.

I'm also think if we need to do a reposition in didChangeDependencies

[joachimvalente]

So the problem is that if you try to do this anywhere before the first build (for example in initState or didChangeDependencies) it complains with ScrollController not attached to any scroll view.

The only workaround I could find was to wait until the build is done, so the controller is attached to a scroll view, and then update the offset. There might a better solution but not sure how exactly it can be done in build or didChangeDependencies

[joachimvalente]

I agree with your changes to clean the code, and I absolutely agree with creating the controller in initstate, but I don't really understand this problem and how you fixed it:

fix side view offset when main offset does not start at 0

I don't see any significant change in the build method or in the calculation of the current position. But that's partly because so many lines changed without really changing anything that I have a hard time finding the real change. 1 tip for next time is to do such big formatting changes separately from the functional changes.

Sorry I agree I should have split it in two PRs

The only "real" change is:

side_header_list_view/lib/side_header_list_view.dart

Line 51 in fdcaca7

WidgetsBinding.instance.addPostFrameCallback((_) => _reposition());

which repositions the side view without waiting for a scroll event

The problem is that if you save the scroll position (e.g. by using PageStorageKey) then when your widget rebuilds, the main list view is scrolled to the right offset, but the side view is always at offset 0. Only when the user scrolls do we update the side view because there's a listener.

[renefloor]

Ah now I understand, so it was about the offset of the scrollcontroller. You can't get it in initstate because the scrollcontroller isn't attached to the widget yet. This is still the case in the build widget (I assume), so you can indeed only get it in the frame after the build, but that's too late.

If i'm correct this only happens on a rebuild, and the state is preserved in that case including all the variables. InitState shouldn't be called in that case. What if we don't store the currentPosition, but the currentOffset and let the build method calculate the position? In that case in a rebuild we still have the currentOffset and can calculate the position in the build method?

[joachimvalente]

Ah now I understand, so it was about the offset of the scrollcontroller. You can't get it in initstate because the scrollcontroller isn't attached to the widget yet. This is still the case in the build widget (I assume), so you can indeed only get it in the frame after the build, but that's too late.

If i'm correct this only happens on a rebuild, and the state is preserved in that case including all the variables. InitState shouldn't be called in that case. What if we don't store the currentPosition, but the currentOffset and let the build method calculate the position? In that case in a rebuild we still have the currentOffset and can calculate the position in the build method?

So here's an example app reproducing the problem: https://gist.github.com/joachimvalente/3a098982c9cd9f93186f6f7e3811a8ac

And here's a video of it:

20210124_144357.mp4

When you change tabs, flutter actually calls dispose and then when you switch back it calls initState again (I checked by adding print statements). Hence the need for PageStorageKey to keep the offset.

I tried keeping the current offset instead of the current position, but it doesn't work for the same reason. The only thing I could do that didn't require a post-build reposition is to save the current offset as a static field, but obviously it's not desirable since you might have several list views in your app. See this branch to see what I'm talking about:
https://github.com/joachimvalente/side_header_list_view/compare/master...joachimvalente:joachimvalente-static?expand=1

Or maybe we could re-use the key and store our own key => offset mapping as a static field? A bit hacky but that could work...

[joachimvalente]

Hi @renefloor, any updates on this?

[renefloor]

I'll try to fix that jump, but I didn't have time for it yet unfortunately.