Repository navigation
feat: seperate watchstreak highlights from sub highlights - #6571
Conversation
pajlada
left a comment
pajlada
left a comment
There was a problem hiding this comment.
Just some nits, otherwise this looks fine.
Looks like you might need to run the formatter too, let me know if you need any help with that.
|
Do these changes look good? |
pajlada
left a comment
pajlada
left a comment
There was a problem hiding this comment.
Just the number in the row index definition, and if you could add a changelog entry to CHANGELOG.md under the ## Unversioned header, that would be good
Something like
- Minor: Add a separate highlight option for watchstreak notifications. (#6571)
|
I changed the indexes back to how they were, I changed those in my first commit, so now I've reverted them |
pajlada
changed the title
pajlada
changed the title
Nerixyz
left a comment
Nerixyz
left a comment
There was a problem hiding this comment.
Could you add a test in https://github.com/Chatterino/chatterino2/tree/8a58fcf0dfa26b44b2026758741a737b7ffd96c8/tests/snapshots/IrcMessageHandler?
Should be pretty similar to the sub-message.json and sub-no-message.json.
| calculateMessageTime(message).time(), channel); | ||
|
|
||
| msg->flags.set(MessageFlag::Subscription); | ||
| if (msgType == "viewermilestone") |
There was a problem hiding this comment.
Do you want to fix this & add a unit test in a follow-up PR @devPoland? If not we can do it later :)
There was a problem hiding this comment.
I see what you mean, this check should not have been put inside the
msgType == "subgift"
check, this is why this returned false, when I built chatterino2 with my changes it worked, so I assumed it was all fine. Thank you for pointing this out, I can commit this right now without the unit test if thats okay?
…#6571) Co-authored-by: pajlada <rasmus.karlsson@pajlada.com>
…#6571) Co-authored-by: pajlada <rasmus.karlsson@pajlada.com>
…#6571) Co-authored-by: pajlada <rasmus.karlsson@pajlada.com>
This simply seperates watchstreaks alerts, so they dont look like sub alerts, and so the user can also set the color to whatever they want