Hide comments based on own blocklist #6878

Merged

infinite-persistence merged 2 commits from ip/hide.blocked.comments into master

2021-08-24 10:59:05 +02:00

infinite-persistence commented

2021-08-13 07:17:44 +02:00

(Migrated from github.com)

Tom, this is the changes we talked about in Slack.
Available in the usual dev instance.
Given how close this is to Saturday's stream, we should only merge in next week (just in case).

Comments: include Commentron blocklists when filtering

Issue

When you block Channel-X, Channel-X's comments will still be visible on someone else's content. This feels odd.
https://github.com/lbryio/lbry-desktop/issues/6107#issuecomment-897884114

Change

In addition to the blacklist, filter-list and muted-list, we now include the Commentron blocklists (personal, admin, moderator) when filtering out comments.

Specifics

makeSelectCommentsForUri, makeSelectTopLevelCommentsForUri and makeSelectRepliesForParentId all perform the same filtering code. Factor that out to makeSelectFilteredComments and add the Commentron lists into the mix.

Downsides

This probably adds to the already-high CPU usage in rendering comments.

Comments: simplify blocked replies display

Issue

Previously, we decide when to display "Show More" based on the current fetched reply count vs. total replies in Commentron. It was already troublesome as comment.List and comment.replies give different values (one includes blocked content, while another does not).

Now, we are further filtering the list with Commentron blocklists (personal, admin, moderator), so it is not feasible to run the logic based on reply count.

Solution

Keep track of number of remaining pages instead and use that to determine when to display "Show More".
- While it doesn't solve the "Show N replies" mismatch (YT has this problem too), it prevents the button from lingering.
In the event that all replies are blocked, just show an empty space (same as YT). I didn't like the previous version that cluttered the space with "comment(s) blocked".

- Tom, this is the changes we talked about in Slack. - Available in the usual dev instance. - Given how close this is to Saturday's stream, we should only merge in next week (just in case). ---- <details> <summary>Comments: include Commentron blocklists when filtering</summary> ## Issue - When you block Channel-X, Channel-X's comments will still be visible on someone else's content. This feels odd. - https://github.com/lbryio/lbry-desktop/issues/6107#issuecomment-897884114 ## Change In addition to the blacklist, filter-list and muted-list, we now include the Commentron blocklists (personal, admin, moderator) when filtering out comments. ## Specifics `makeSelectCommentsForUri`, `makeSelectTopLevelCommentsForUri` and `makeSelectRepliesForParentId` all perform the same filtering code. Factor that out to `makeSelectFilteredComments` and add the Commentron lists into the mix. ## Downsides This probably adds to the already-high CPU usage in rendering comments. </details> <details> <summary>Comments: simplify blocked replies display</summary> ## Issue Previously, we decide when to display "Show More" based on the current fetched reply count vs. total replies in Commentron. It was already troublesome as `comment.List` and `comment.replies` give different values (one includes blocked content, while another does not). Now, we are further filtering the list with Commentron blocklists (personal, admin, moderator), so it is not feasible to run the logic based on reply count. ## Solution - Keep track of number of remaining pages instead and use that to determine when to display "Show More". - While it doesn't solve the "Show N replies" mismatch (YT has this problem too), it prevents the button from lingering. - In the event that all replies are blocked, just show an empty space (same as YT). I didn't like the previous version that cluttered the space with "comment(s) blocked". </details>

tzarebczan (Migrated from github.com) reviewed 2021-08-13 07:17:44 +02:00

tzarebczan commented

2021-08-13 17:31:41 +02:00

(Migrated from github.com)

Thanks! Will give this a test, but agreed, let's not merge til next week.

tzarebczan commented

2021-08-17 00:10:03 +02:00

(Migrated from github.com)

This was super buggy on kp.odysee.com

infinite-persistence commented

2021-08-22 13:37:44 +02:00

(Migrated from github.com)