-
-
Notifications
You must be signed in to change notification settings - Fork 449
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
Add transparent overlay window #4746
Conversation
I've been using https://github.com/baffler/Transparent-Twitch-Chat-Overlay for the functionality that this PR is going to add to Chatterino, very glad to see it finally happening. One major feature that is an oversight for many chat overlays is that it needs to have toggleable click-through, so that the widget can be sized and moved, but when toggled won't be able to be clicked or interfere with mouse pointer tracking in games and other programs. |
Windows only rn
* Align code style * Support multiple shortcuts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
Looks great, I'd like to see the interactability toggleable as part of a Chatterino hotkey instead of a Global Shortcut. If the need arises for speed, we can re-investigate Global Shortcuts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
src/widgets/OverlayWindow.cpp
Outdated
using namespace literals; | ||
|
||
/// Progress the user has made in exploring the overlay | ||
enum class Knowledge : std::int32_t { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: enum 'Knowledge' uses a larger base type ('std::int32_t' (aka 'int'), size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
enum class Knowledge : std::int32_t {
^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small nits
src/singletons/WindowManager.hpp
Outdated
@@ -128,6 +128,9 @@ class WindowManager final | |||
// again | |||
void queueSave(); | |||
|
|||
/// Toggles the inertia in all open popup windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Toggles the inertia in all open popup windows | |
/// Toggles the inertia in all open overlay windows |
src/widgets/splits/Split.cpp
Outdated
if (arg == "all") | ||
{ | ||
getApp()->getWindows()->toggleAllPopupInertia(); | ||
return {}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the default keybind action should be all
, and I think toggleAllPopupIntertia
should not toggle each overlay individually, rather think of all of them as a group.
If any overlay is interactable, make all overlays inert. Otherwise, make all overlays interactable.
src/widgets/helper/ChannelView.cpp
Outdated
this->messageColors_.applyTheme(getTheme()); | ||
if (this->colorVisitor_) | ||
{ | ||
this->colorVisitor_(this->messageColors_, getTheme()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could applyTheme
on MessageColors accept the overlayBackgroundOpacity
plus a boolean whether this is an overlay enough, and let it do the logic for how to apply the colors instead?
so MessageColors::applyTheme(Theme*)
would instead be MessageColors::applyTheme(Theme*, bool isInOverlay, int backgroundOpacity
) - that should let us get rid of the visitor pattern
These weren't used, but I'd like to have something like this in the future. Not eaxactly sure what the api should be (settings/theme)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
75e3dbf is a bit bigger, as I didn't use the correct colors from the theme for the text (it used the "regular" ones instead of the ones for the overlay). Light themes don't look as good as dark themes, as the text colors are "normalized". We should do that a bit differently, but that's for a future PR imo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just found one small nit - not committing it myself in case there's a better way to do it
Description
This PR adds a hotkey to open a transparent overlay channel-view. The colors of the overlay can be customized in the theme. To move, resize, and close the overlay, you have to press the SHIFT key.
This is an early feature and requires more PRs in the future to support more stuff like scaling.
TODO
/watching
.ChannelView
s without aSplit
#4747)Add minimize buttonIf that's desired, it should be done in a future PR.NCHITTEST
?Update theme migrations (wiki)Not needed as they will use a fallbackoverlayunderlay)firefox_2023-08-02_19-57-18.mp4
Link to the video in case the embed is broken:
https://github.com/Chatterino/chatterino2/assets/19953266/3e798169-a82f-4123-b834-7ce55daf0fc1
Closes #1618.