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

Show system message when reloading subscriber emotes #3135

Merged

Conversation

jupjohn
Copy link
Contributor

@jupjohn jupjohn commented Aug 7, 2021

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

This PR aims to display a system message when reloading subscriber emotes. This behaviour is in line with the BTTV/FFZ emote reload behaviour.

I've passed through the channel to loadEmotes which might be ugly to some people but it ensures you get a 'reloaded' messages when the reloading has started instead of when the method was called. Also kept the old loadEmotes() overload around for the call in IRCMessagehandler.

@zneix zneix self-requested a review August 7, 2021 08:54
src/providers/twitch/TwitchAccount.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@zneix zneix left a comment

Choose a reason for hiding this comment

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

LGTM 👍

On a side note: Just wondering if wording "Twitch subscriber emotes" is correct nowadays. It used to be like that, but now we also have follower and bit emotes as well. Potential wording change would also mean changing that phrasing in KeybindingsPage.cpp and couple places in SplitHeader.cpp.

@jupjohn
Copy link
Contributor Author

jupjohn commented Aug 7, 2021

LGTM +1

On a side note: Just wondering if wording "Twitch subscriber emotes" is correct nowadays. It used to be like that, but now we also have follower and bit emotes as well. Potential wording change would also mean changing that phrasing in KeybindingsPage.cpp and couple places in SplitHeader.cpp.

Start a separate issue/discussion for that - out of scope image

@zneix
Copy link
Collaborator

zneix commented Aug 7, 2021

Start a separate issue/discussion for that - out of scope image

Yeah I don't think it should be a part of this one, I'm just sharing my idea here and an issue / PR could be made out of my comment later.

Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

👍

@pajlada pajlada enabled auto-merge (squash) August 8, 2021 10:20
Copy link
Collaborator

@zneix zneix left a comment

Choose a reason for hiding this comment

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

Pajlada's changes seem nice

@pajlada pajlada merged commit 6151cd5 into Chatterino:master Aug 8, 2021
zneix added a commit to SevenTV/chatterino7 that referenced this pull request Aug 8, 2021
Now we're on commit a216e11; Changes from upstream we've pulled:

- Minor: Remove TwitchEmotes.com attribution and the open/copy options when right-clicking a Twitch Emote. (Chatterino#2214, Chatterino#3136)
- Minor: Strip leading @ and trailing , from username in /user and /usercard commands. (Chatterino#3143)
- Minor: Display a system message when reloading subscription emotes to match BTTV/FFZ behavior (Chatterino#3135)
- Bugfix: Moderation mode and active filters are now preserved when opening a split as a popup. (Chatterino#3113, Chatterino#3130)
- Bugfix: Fixed a bug that caused all badge highlights to use the same color. (Chatterino#3132, Chatterino#3134)
- Dev: Renamed CMake's build option `USE_SYSTEM_QT5KEYCHAIN` to `USE_SYSTEM_QTKEYCHAIN`. (Chatterino#3103)
- Dev: Add benchmarks that can be compiled with the `BUILD_BENCHMARKS` CMake flag. Off by default. (Chatterino#3038)
- Dev: Allow building against Qt 5.11 (Chatterino#3105)
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.

None yet

4 participants