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

Feature Request: Top Level Comment Navigation #19

Closed
kigero opened this issue May 28, 2021 · 5 comments
Closed

Feature Request: Top Level Comment Navigation #19

kigero opened this issue May 28, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@kigero
Copy link

kigero commented May 28, 2021

One of the things that I've become accustomed to in my reddit client (Joey) is the comment navigation. At the bottom of the comments view is a navigation toolbar, the center lets you select how to navigate (with lots of options, but I really only use top level), and the arrows navigate up or down between comments. So if I start reading through a comment thread and get bored, I can quickly go to the next top level comment and start reading that one. It would be great if something similar could be added to Glider. Some other clients that I've looked at use the volume buttons for this sort of navigation, but I prefer something on screen.

image

@Mosc
Copy link
Owner

Mosc commented May 29, 2021

The best I can promise you right now is that I'll consider it! I'm currently mostly focusing on features that do not add any additional configuration for the users, and this is not a feature I would want turned on by default.

@kigero
Copy link
Author

kigero commented May 30, 2021

No worries, that sounds good to me. I can appreciate that it's a pretty niche feature. Thanks!

@Mosc Mosc added the enhancement New feature or request label Jul 16, 2021
@Mosc
Copy link
Owner

Mosc commented Aug 26, 2021

I've had a go at this, and annoyingly, I cannot get it to work the way I'd like. I'll summarize my findings for future-me, or anyone else interested in contributing.

Overlaying the actual buttons is easy enough. Then, on pressing the "next" button, one should do roughly the following:

  1. Figure out the top-most index currently visible in the list.
  2. Figure out the next top-level comment index and pick the first one higher than the index determined above.
  3. Automatically scroll to that index.

We have all the required information for step 2. The problem is that there are no built-in solutions for step 1 and 3 in the Flutter framework. You can scroll to an offset, but in a list with variable heights such as the comments list, it's impossible to calculate the correct offset without first rendering all the comments.

One solution for both step 1 and 3 comes in the form of the scrollable_positioned_list package. This solves both problems well but has limitations. It's a drop-in replacement for Flutter's ListView, while Glider uses SliverLists to be able to render views on top of and at the bottom of the lost while keeping the entire page scrollable. Additionally, Glider makes use of a NestedScrollView to get the app bar to float, making it automatically appear and disappear on scrolling. That functionality breaks when changing the scroll controller in the inner list which would be necessary for automatically scrolling to an index in the ScrollablePositionedList.

Another solution to step 3 may be scroll_to_index. That can coexist with our sliver usage, but doesn't provide a method for finding the index in step 1. Several options are discussed here, although I do prefer ScrollablePositionedList's ease of use. Unfortunately scroll_to_index's AutoScrollController also exhibits the same NestedScrollView conflict as scrollable_positioned_list's ItemScrollController.

@Mosc Mosc added the help wanted Extra attention is needed label Aug 26, 2021
@vonDubenshire
Copy link

I hope someone can get this to work over day.

I just installed the app, got it all set up and then realized that was the one feature I suddenly needed really badly!

@Mosc Mosc removed the help wanted Extra attention is needed label Oct 23, 2023
@Mosc
Copy link
Owner

Mosc commented Oct 23, 2023

This has been implemented as part of v2.0.0!

@Mosc Mosc closed this as completed Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants