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

refactor: Rewrite comments fragments using Paging library #5589

Merged
merged 5 commits into from
Feb 11, 2024

Conversation

Isira-Seneviratne
Copy link
Collaborator

@Isira-Seneviratne Isira-Seneviratne commented Feb 2, 2024

Improve the comment and comment replies fragments by using the paging library.

@Isira-Seneviratne Isira-Seneviratne force-pushed the Comment_paging branch 2 times, most recently from d82eafb to 0440a44 Compare February 2, 2024 03:11
@Isira-Seneviratne Isira-Seneviratne marked this pull request as ready for review February 3, 2024 05:20
@Isira-Seneviratne Isira-Seneviratne changed the title refactor: Rewrite comments fragment using Paging library refactor: Rewrite comments fragments using Paging library Feb 3, 2024
@Bnyro Bnyro self-requested a review February 3, 2024 16:35
Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed 3 small regressions:

  1. The "back" button on the top left is no longer visible when visiting the comment replies fragment, as it's been before the changes.
  2. There's no loading indicator when opening the replies of a comment / it's positioned at the bottom sometimes instead of the top.
  3. The channel avatars of the commentors/repliers are having the same issue as in refactor: Rewrite search functionality using Paging #5528 (comment).

Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment fragment works very well now, thank you for looking into my comments!

I noticed one small visual issue when opening the replies fragment when the comment has a lot of issues - it seems like the main comment is duplicated - once at the bottom of the replies and once at the top. When scrolling up again, the duplicated comment at the bottom vanishes. It looks like the layout isn't properly re-created here when the comments are loaded?

I'm not really familiar with the paging library, so I can't really tell what's going on.

@Isira-Seneviratne
Copy link
Collaborator Author

The comment fragment works very well now, thank you for looking into my comments!

I noticed one small visual issue when opening the replies fragment when the comment has a lot of issues - it seems like the main comment is duplicated - once at the bottom of the replies and once at the top. When scrolling up again, the duplicated comment at the bottom vanishes. It looks like the layout isn't properly re-created here when the comments are loaded?

I'm not really familiar with the paging library, so I can't really tell what's going on.

That's because the parent comment is first displayed before the replies are loaded into the adapter.

This guide should help with understanding the paging library.

@Isira-Seneviratne Isira-Seneviratne merged commit 69cf74e into libre-tube:master Feb 11, 2024
3 of 4 checks passed
@Isira-Seneviratne Isira-Seneviratne deleted the Comment_paging branch February 11, 2024 12:38
@Pamilg8
Copy link
Contributor

Pamilg8 commented Feb 11, 2024

The comment fragment works very well now, thank you for looking into my comments!

I noticed one small visual issue when opening the replies fragment when the comment has a lot of issues - it seems like the main comment is duplicated - once at the bottom of the replies and once at the top. When scrolling up again, the duplicated comment at the bottom vanishes. It looks like the layout isn't properly re-created here when the comments are loaded?

I'm not really familiar with the paging library, so I can't really tell what's going on.

In single-answer comments, you must go back and reload the answer to see the answer. Do you have a solution?

@Isira-Seneviratne
Copy link
Collaborator Author

In single-answer comments, you must go back and reload the answer to see the answer. Do you have a solution?

Changing position - 1 to position in the comment adapter might fix the issue (I'm not able to test it at the moment).

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.

3 participants