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

feat/block-chat-phrases: Add blocked phrases feature #514

Closed
wants to merge 32 commits into from

Conversation

berghall
Copy link
Contributor

@berghall berghall commented Apr 15, 2023

This will add a new section under Chat settings where you can add blocked phrases that you don't want to see in chat. It looks and works similarly to highlights but the other way around.

a7e58dede8d1d32afb69048a4111d428

Closes #403

@AnatoleAM
Copy link
Contributor

AnatoleAM commented Apr 15, 2023

Nice! That's been a highly requested feature, will have a look sometime today 👍

To fix CI issues simply run make format (if emoji.json comes up as tracked, don't commit it)

Copy link
Contributor

@AnatoleAM AnatoleAM left a comment

Choose a reason for hiding this comment

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

I believe this could be heavily simplified with a few steps

  • Compound everything into the highlights system. Instead of duplicating it, just add a mode property, or boolean flag, to define whether it's a highlight or filter
  • You can add an argument to useChatHighlights, to define the mode.
  • Store filters in a different config key
  • Generalize the component for the interface used between this and highlights

@berghall
Copy link
Contributor Author

@AnatoleAM Now a Highlight has a Blocked toggle from where it toggles it into a new config set
Still not quite sure about the "Blocked" naming, so if you have any suggestions please tell :D

PS.
The highlight saving should probably be improved in another issue, because now all the items in the table are sent over even though you're editing one.

@berghall berghall requested a review from AnatoleAM April 17, 2023 18:57
@AnatoleAM
Copy link
Contributor

The highlight saving should probably be improved in another issue

This needs a general refactor of the settings system. It's currently implemented in a way which does not allow non-shallow reactive updates to be detected. Hence why a save() method even has to exist in the first place.

Copy link
Contributor

@AnatoleAM AnatoleAM left a comment

Choose a reason for hiding this comment

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

I think "Ignores" may be a better name than "blocked". This wording is also used in Chatterino, which the highlights system is generally inspired from.

.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
src/composable/chat/useChatHighlights.ts Outdated Show resolved Hide resolved
src/composable/chat/useChatHighlights.ts Outdated Show resolved Hide resolved
@berghall
Copy link
Contributor Author

@AnatoleAM PR updated with the requested changes and the terminology is now "Ignores"

@berghall berghall requested a review from AnatoleAM April 18, 2023 13:05
Copy link
Contributor

@AnatoleAM AnatoleAM left a comment

Choose a reason for hiding this comment

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

Close, but needs some improvement on the UX side

  • Split ignores into a new setting category
  • Instead of adding isIgnored on highlights, pass an argument to useChatHighlights to define whether to use ignore or highlight mode
  • Define the config key used based on the mode passed to the composable

@berghall
Copy link
Contributor Author

@AnatoleAM now the Ignores are in Chat > Ignores

@berghall berghall requested a review from AnatoleAM April 19, 2023 18:11
src/site/global/settings/SettingsConfigIgnores.vue Outdated Show resolved Hide resolved
src/composable/chat/useChatHighlights.ts Outdated Show resolved Hide resolved
src/composable/chat/useChatHighlights.ts Outdated Show resolved Hide resolved
src/composable/chat/useChatHighlights.ts Outdated Show resolved Hide resolved
src/composable/chat/useChatHighlights.ts Outdated Show resolved Hide resolved
src/site/twitch.tv/modules/chat/ChatList.vue Outdated Show resolved Hide resolved
src/composable/chat/useChatHighlights.ts Outdated Show resolved Hide resolved
@berghall
Copy link
Contributor Author

@AnatoleAM added now a wrapper for ignores

@berghall berghall requested a review from AnatoleAM April 22, 2023 11:33
@berghall
Copy link
Contributor Author

I resolved merge conflicts, but not sure what the linter is complaining about now

@AnatoleAM
Copy link
Contributor

Closing as stale

@AnatoleAM AnatoleAM closed this Aug 21, 2023
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.

[Feature] Banned/muted words
2 participants