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

Comments Pagination #6390

Merged
merged 8 commits into from
Jul 14, 2021
Merged

Comments Pagination #6390

merged 8 commits into from
Jul 14, 2021

Conversation

infinite-persistence
Copy link
Contributor

@infinite-persistence infinite-persistence commented Jul 5, 2021

Status

  • Test instance: https://kp.odysee.com
  • Good to go when we get Tom's seal of approval.
    • While I've tested a lot, it's still a big change (affected many files), so a second look by someone else will be good before we deploy

Known issues

  • Livestream remains unpaginated. Wait for Cursor Pagination in v3.
  • Pinned comments is broken. Fixed
  • Linked comments doesn't work right if nested or yet to be fetched. Fixed

Rough summary of changes

  • Top level comments are paginated using infinite-scroll style.
  • Replies are paginated using a "Show more" button.
  • The existing "expand all replies" behavior is possible, but disabled for now, except for linked-comments.
  • Reactions: now paginated as well, instead of always re-fetching for the entire list of comments.
    • When switching active channels, you might see like-counts temporarily reset if the fetch is slow.

Issue

Closes #6158 - Support Comment Pagination

@tzarebczan
Copy link
Contributor

https://kp.odysee.com/@Odysee:8?view=discussion is a good large discussion to test on. Some findings:

  1. When on large monitor, it shows a couple comments and the loading icon, but not actually loading anything:
    image

  2. this shows 2 replies, but there's only 1. When you expand, you see the 1 + loading icon:
    image

Other feedback
It's annoying to scroll in large lists like above, especially when you do it quickly - it seems like it only loads a few things, and then keeps showing loading icon for more. Scrolling slower seems to fair better.

Seems pretty solid otherwise, even on mobile!

## Issue
6158 - Support Comment Pagination
Can be removed when bugs are fixed, but would still work if left there.
Seeing the spinner too much can be annoying.

- This approach works, but currently, when the list is very long, something is taking up resources and the handler couldn't be processed, so the effect is lost (still seeing the spinner). See 6473.

- Since we are now prefetching, bumped the debounceMs a bit.
It was previously only responding to scroll events.
@infinite-persistence
Copy link
Contributor Author

infinite-persistence commented Jul 14, 2021

Update

6c80acd to 4731588

  • Tell user when linked-comment is not available.
  • Load comments if spinner is visible from the start (it was previously only responding to scroll events)
  • Load comments when approaching viewport
    • This approach works, but currently, the issue in 6473 negates the effect at times.
  • Bumped comment and reply page size to 10 (was 5 and 3, respectively)

Test instance updated as well.

Pending

this shows 2 replies, but there's only 1. When you expand, you see the 1 + loading icon:

Asking Mark if it's possible for replies to reflect the final listing.

## Issue
- `Comment.replies` currently represent all replies, while `comment.List` returns a filtered version, so the actual replies could be less.
- The actual replies is represented by `total_filtered_items`, but we only get that after making a fetch. So, users could click "Show more" but get nothing.

## Fix
- Stop showing "Show more" based on `total_filtered_items`.
- If there is a balance, display 1 dummy comment to represent all blocked replies. This handles the case of "Show more" being displayed but ended up with 0 replies if all replies were blocked.

## Future
Note that `Comment.replies` might be changed to represented filtered comments in the near future (refer to Beamer), so the GUI is made such that the dummy just won't appear when that change happens.
@infinite-persistence infinite-persistence merged commit 16ef013 into master Jul 14, 2021
@infinite-persistence infinite-persistence deleted the ip/comment.pagination branch July 14, 2021 14:42
infinite-persistence added a commit that referenced this pull request Jul 15, 2021
This reverts commit 16ef013, reversing
changes made to fba8b89.
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

Successfully merging this pull request may close these issues.

Support Comment Pagination
3 participants