feat(emote-popup): save size of popup #5415
Conversation
Nerixyz
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -98,7 +98,7 @@ class WindowLayout | |||
| { | |||
| public: | |||
| // A complete window layout has a single emote popup position that is shared among all windows | |||
| QPoint emotePopupPos_; | |||
| QRect emotePopupBounds_; | |||
There was a problem hiding this comment.
warning: invalid case style for member 'emotePopupBounds_' [readability-identifier-naming]
| QRect emotePopupBounds_; | |
| QRect emotePopupBounds; |
| @@ -328,14 +328,18 @@ void WindowManager::scrollToMessage(const MessagePtr &message) | |||
| this->scrollToMessageSignal.invoke(message); | |||
| } | |||
|
|
|||
| QPoint WindowManager::emotePopupPos() | |||
| QRect WindowManager::emotePopupBounds() const | |||
There was a problem hiding this comment.
warning: no header providing "QRect" is directly included [misc-include-cleaner]
src/singletons/WindowManager.cpp:5:
- #include "debug/AssertInGuiThread.hpp"
+ #include "common/WindowDescriptors.hpp"
+ #include "debug/AssertInGuiThread.hpp"| @@ -99,8 +99,8 @@ class WindowManager final : public Singleton | |||
| */ | |||
| void scrollToMessage(const MessagePtr &message); | |||
|
|
|||
| QPoint emotePopupPos(); | |||
| void setEmotePopupPos(QPoint pos); | |||
| QRect emotePopupBounds() const; | |||
There was a problem hiding this comment.
warning: no header providing "QRect" is directly included [misc-include-cleaner]
src/singletons/WindowManager.hpp:4:
- #include "widgets/splits/SplitContainer.hpp"
+ #include "common/WindowDescriptors.hpp"
+ #include "widgets/splits/SplitContainer.hpp"| @@ -8,8 +8,7 @@ | |||
|
|
|||
| namespace { | |||
|
|
|||
| /// Move the `window` into the `screen` geometry if it's not already in there. | |||
| void moveWithinScreen(QWidget *window, QScreen *screen, QPoint point) | |||
| QPoint applyBounds(QScreen *screen, QPoint point, QSize frameSize, int height) | |||
There was a problem hiding this comment.
warning: no header providing "QSize" is directly included [misc-include-cleaner]
src/util/WidgetHelpers.cpp:7:
+ #include <qsize.h>| } | ||
| break; | ||
| default: | ||
| assert(false && "Invalid bounds checking mode"); |
There was a problem hiding this comment.
warning: no header providing "assert" is directly included [misc-include-cleaner]
src/util/WidgetHelpers.cpp:7:
+ #include <cassert>| { | ||
| bounds.setSize(QSize{300, 500} * this->scale()); | ||
| } | ||
| this->setInitialBounds(bounds, widgets::BoundsChecking::DesiredPosition); |
There was a problem hiding this comment.
warning: no header providing "chatterino::widgets::BoundsChecking" is directly included [misc-include-cleaner]
src/widgets/dialogs/EmotePopup.cpp:20:
- #include "widgets/helper/ChannelView.hpp"
+ #include "util/WidgetHelpers.hpp"
+ #include "widgets/helper/ChannelView.hpp"| getIApp()->getWindows()->setEmotePopupBounds(this->getBounds()); | ||
| } | ||
|
|
||
| void EmotePopup::resizeEvent(QResizeEvent *event) |
There was a problem hiding this comment.
warning: no header providing "QResizeEvent" is directly included [misc-include-cleaner]
void EmotePopup::resizeEvent(QResizeEvent *event)
^| BasePopup::resizeEvent(event); | ||
| } | ||
|
|
||
| void EmotePopup::moveEvent(QMoveEvent *event) |
There was a problem hiding this comment.
warning: no header providing "QMoveEvent" is directly included [misc-include-cleaner]
void EmotePopup::moveEvent(QMoveEvent *event)
^| @@ -25,6 +25,10 @@ class EmotePopup : public BasePopup | |||
|
|
|||
| pajlada::Signals::Signal<Link> linkClicked; | |||
|
|
|||
| protected: | |||
| void resizeEvent(QResizeEvent *event) override; | |||
There was a problem hiding this comment.
warning: no header providing "QResizeEvent" is directly included [misc-include-cleaner]
void resizeEvent(QResizeEvent *event) override;
^| @@ -25,6 +25,10 @@ | |||
|
|
|||
| pajlada::Signals::Signal<Link> linkClicked; | |||
|
|
|||
| protected: | |||
| void resizeEvent(QResizeEvent *event) override; | |||
| void moveEvent(QMoveEvent *event) override; | |||
There was a problem hiding this comment.
warning: no header providing "QMoveEvent" is directly included [misc-include-cleaner]
void moveEvent(QMoveEvent *event) override;
^
I don't know if this PR completely resolves #5414, but it does fix some bugs related to Qt'sframeMarginsthat are added even though we're a frameless window (probably a Qt bug - I'll try to reproduce and report it). As mentioned in #5414, the emote-popup doesn't remember its position if the main window is closed while the popup is open. This PR fixes that bug as well.As mentioned in #5414 (comment), saving the width and height of the popup in addition to its position is reasonable. The position is now remembered correctly without Qt's invisible margins (
geometrydoesn't have that "bug"). The only tiny issue I can see is closing Chatterino with the popup open can cause the position to be off by a few pixels because of the delayed bounds on Windows, however, I don't really think this is a major issue.Fixes #5414.