Replace scrollable_positioned_list with super_sliver_list

[hjiangsu]

Pull Request Description

For some background, I'm in the process of refactoring the comment list to align more with our current conventions of using Slivers for performance reasons. However, I encountered one issue where scrollable_positioned_list does not have a Sliver version.

Because of this, I started to look into other packages and stumbled upon super_sliver_list. This package allows us to scroll to a given index, and from my testing, seems to work fairly well. Additionally, it exposes a few more additional functions that could be useful (including the ability to specify the alignment when scrolling to a given index)

This PR essentially replaces the usage of scrollable_positioned_list with super_sliver_list. Additionally, I've adjusted the logic for the comment navigation FAB. It now keeps track of the index internally which hopefully simplifies some logic.

@micahmo I'm not sure if the changes made to the comment navigation FAB breaks anything, but do let me know (since you were mainly the one who implemented the scroll-to-index functionality)!

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

Checklist

• Did you update CHANGELOG.md?
• Did you use localized strings where applicable?
• Did you add semanticLabels where applicable for accessibility?

[micahmo]

This looks great! I had stumbled upon super_sliver_list too and it totally makes sense as the next logical step to use the slivers for the comments.

In addition, I think keeping track of the index internally rather than through a bloc event is a huge upgrade, because there were a number of times that the state wouldn't be right after jumping (because it wouldn't emit everything correctly).

The best part of this change, which might not have even been intentional, is that it seems to fix #588!!

Note that this PR will have some conflicts with #1319.

[hjiangsu]

In addition, I think keeping track of the index internally rather than through a bloc event is a huge upgrade, because there were a number of times that the state wouldn't be right after jumping (because it wouldn't emit everything correctly).

There are some situations where we might need to override the index (such as when we jump to a specific comment, or when performing search) which is why I kept navigateCommentIndex in post bloc. However, for everything else, I do agree that storing it locally simplifies a lot of the logic!

Note that this PR will have some conflicts with #1319.

I'll see what I can do here to mimic the behaviour from #1319, since we don't have access to itemScrollController anymore!

[micahmo]

I'll see what I can do here to mimic the behaviour from #1319, since we don't have access to itemScrollController anymore!

Thanks!! The main things are to delay the scrolling until the comments are built, and to calculate the height of the bottom spacer based on the height of the last comment so it can always be scrolled to the very top even if short.

[hjiangsu]

I'll go ahead and merge this in now that #1319 is merged in. Let me know if you encounter any issues running with this change!

[micahmo]

I just found the first problem with this. If you navigate down (e.g., to the 2nd comment), and then manually scroll up, navigating down should bring you back down to the 2nd comment. However, because of the internal state, it navigates you all the way to the 3rd comment.

The nice thing about scrollable_positioned_list (and I'm not sure if super_sliver_list has this functionality) is that it always understood where we were in relation to the items. So it could always bring the "next" item to the top, regardless of the current index.

Here's where the logic was to find the current index, if it helps.

https://github.com/thunder-app/thunder/pull/1361/files#diff-864f8928f394bbff7bfafd030900915382650e584d9c4ffb3ebefa4efba42b90L87

[hjiangsu]

Interesting, I didn't know that behaviour existed! There might be a way to do this, but I would have to look further into how super_sliver_list works. Thanks for finding this 😅

[micahmo]

Thanks!! I'm pretty sure it's how most people will expect the jump feature to work. Jump down or up to the next top-level comment. (It occurred to me that the same problem also occurs when jumping upward.) It needs to be relative to where we've scrolled, not absolute.