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

Muted Words Settings Page #5934

Merged
merged 9 commits into from
May 26, 2021
Merged

Conversation

infinite-persistence
Copy link
Contributor

@infinite-persistence infinite-persistence commented Apr 22, 2021

Demo

KP dev instance.

Issue

Closes #5833 Support creator controlled commenting settings (mainly muted word list for now)

I believe only "Muted Words" is functional at this moment, as noted by the issue's title.
The rest, while set-able, has no effect, so we'll need to comment out the "General" and "Tip" cards before merging later.

Notes to reviewer

  • Double check the fetchCommentsApi: Relay error messages to the client's 'catch' block. commit. This would fail if any clients are depending on res.results on the failure case, but I don't think there are any.
  • I'm not really sure what are the units for "min amount" (LBC or satoshis?) and "slow mode gap" (seconds?) are. Looking at how superchats are being used, I think they are satoshies (integer) and the GUI doesn't need to do any translation?

GUI

  • Added "Creator Settings" card in the Settings Page when user has a channel.
  • "Creator Settings" currently only have the "Muted Words" card.
  • The toast will say The comment contains contents that are blocked by %author% when commenting with a blocked word.
  • Tried to somewhat address #5931 Fast actions will often be undone when there's a background update by waiting for a 1s user-idle before syncing the GUI with the API data.

API bugs

  • [Blocking] Due to an empty string bug in the block list, the system will block every comment even if you cleared the blocked-words list.
    • There is a workaround: create 2 or more blocked words, then delete 1 of them as the last action. Deleting all words will result in [""] instead of [], causing the matching to always be true.
  • unable to marshall error when changing any numerical setting.
  • Doesn't accept Unicode so it only works for English creators.
  • There is a 3-character minimum for the blocked word. If we support Unicode, the limit won't make sense since bad words in Asian languages can be single-character.

@infinite-persistence

This comment has been minimized.

@infinite-persistence infinite-persistence marked this pull request as ready for review April 26, 2021 07:15
Copy link

@neb-b neb-b left a comment

Choose a reason for hiding this comment

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

This mostly looks good. A few comments. The styling looks a little off on the muted words.

ui/component/app/view.jsx Outdated Show resolved Hide resolved
ui/page/settingsCreator/view.jsx Outdated Show resolved Hide resolved
ui/redux/actions/comments.js Show resolved Hide resolved
ui/scss/component/_tags.scss Outdated Show resolved Hide resolved
@infinite-persistence
Copy link
Contributor Author

infinite-persistence commented Apr 27, 2021

UPDATES PER REVIEW

  • Removed settings-fetch at startup.
    • I was thinking that we'll need to look at comments_enabled to put a reminder or something in the Comment section, but now I think that's not necessary.
  • Updated text to hyperchats.
  • Fix "word tag" colors in both themes.

@infinite-persistence infinite-persistence force-pushed the ip/creator-comment-settings-5833 branch 2 times, most recently from 73ff12d to e5cdb3c Compare May 2, 2021 07:45
@kauffj kauffj requested a review from jessopb May 3, 2021 19:07
@tzarebczan
Copy link
Contributor

Tried to run locally and ran into this on startup: bundle.es.js?ea01:1516 Uncaught Error: Claim sequence must be a number. at parseURIModifier (bundle.es.js?ea01:1516) at parseURI (bundle.es.js?ea01:1418) at generateShareUrl (url.js?9533:101) at ClaimMenuList (view.jsx?ebf8:47) at renderWithHooks (react-dom.development.js?4b4a:14820) at mountIndeterminateComponent (react-dom.development.js?4b4a:17476) at beginWork (react-dom.development.js?4b4a:18625) at HTMLUnknownElement.callCallback (react-dom.development.js?4b4a:185) at Object.invokeGuardedCallbackDev (react-dom.development.js?4b4a:237) at invokeGuardedCallback (react-dom.development.js?4b4a:290)

Will blow out node modules and try again.

@tzarebczan
Copy link
Contributor

I see it's up on kp.odysee.com too, nevermind! Poked Mark about the commentron side fix, he'll get it up shortly.

@tzarebczan
Copy link
Contributor

tzarebczan commented May 5, 2021

Gave some feedback for Mark to fix the things you ran into also. Comments should be enabled by default, gave him a heads up about that.

  • Add credits icon in min tip amounts
  • hide control tags (this is under the experimental UI tag) - should probably be combined with the first setting - it disables comments directly on claim/channels based on the tag being set. We'd need to issue a channel update to do that. Maybe one setting it a bit more temporary (turning off on comments server) and tag is more permanent. Not sure how to present this. The tag one would also disallow showing any previous comments across content, whereas the control setting only stops new ones.
  • for the comment tip setting, I think the intention was to force all comments to have tips in order to be posted - which would be an anti spam mechanism. That should be made clearer and ensure @tiger5226 enforces. That would apply to hyperchats as well. So that setting is more global and forces tips, whereas the other is a min when a tip is used.

Minor:

  • noticed settings list is run for all channels and page load, or when changing settings (though it's set on one).

image

@infinite-persistence infinite-persistence force-pushed the ip/creator-comment-settings-5833 branch 2 times, most recently from 96d4120 to 23dd135 Compare May 11, 2021 01:51
@infinite-persistence infinite-persistence marked this pull request as draft May 11, 2021 02:13
- Allow 'followed' and 'unfollowed' list to be overridden via props.
- Allow labels to be overridden via props.
- Add ability to disable "Suggestions"
## Changes:
1) TagSearch: hide the "control tags" in the Creator Settings page (irrelevant).
2) TagSearch: show the "control tags" when creating/editing Channel (let's use `setting.CommentsEnabled` instead).
3) TagSearch: show the "control tags" when creating/editing Content (`disable-comments` can be used to block comments at the per-claim level, e.g. allow comments in general but block only for specific claims).

## Missing pieces:
For (2) and (3), some work is needed to hide the comment GUI when `setting.CommentsEnabled` is disabled for a particular channel. That flag is not ready in Commentron yet, so I'm not sure how this will be done at the moment. In other words, the checkbox does nothing at the moment.

## Potential flaw:
This change will hide all control tags. If we have more tags in the future and would like to selectively disable some, we'll have to change this parameter to an array instead. Since the usage is not widespread at the moment, a single `disableControlFlag` seems cleaner (don't over-think it yet).
@infinite-persistence
Copy link
Contributor Author

infinite-persistence commented May 25, 2021

@tzarebczan, this is ready for final review before shipping Muted Words. Nothing new to test other than to look for corner-cases, I think.

Changes/fixes:

  • Hide the other features that are not ready.
  • "Empty string" issue fixed in Commentron.
  • Handle the case when bid is insufficient.
    • image

Merging Request

  • Let's not squash this time, as it'll be easier to revert sub-changes later. I've squashed to a minimal number of commits.
    • "Rebase(if not already)-then-create-merge-commit" would be nice.

@infinite-persistence infinite-persistence marked this pull request as ready for review May 25, 2021 05:29
@tzarebczan tzarebczan merged commit 06c6018 into master May 26, 2021
@tzarebczan tzarebczan deleted the ip/creator-comment-settings-5833 branch May 26, 2021 19:38
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 creator controlled commenting settings (mainly muted word list for now)
5 participants