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(emote-popup): save size of popup #5415

Merged
merged 10 commits into from
Jun 1, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Minor: Add option to customise Moderation buttons with images. (#5369)
- Minor: Colored usernames now update on the fly when changing the "Color @usernames" setting. (#5300)
- Minor: Added `flags.action` filter variable, allowing you to filter on `/me` messages. (#5397)
- Minor: The size of the emote popup is now saved. (#5415)
- Bugfix: If a network request errors with 200 OK, Qt's error code is now reported instead of the HTTP status. (#5378)
- Dev: Use Qt's high DPI scaling. (#4868, #5400)
- Dev: Add doxygen build target. (#5377)
Expand Down
12 changes: 9 additions & 3 deletions src/common/WindowDescriptors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,15 @@ WindowLayout WindowLayout::loadFromFile(const QString &path)
}

// Load emote popup position
QJsonObject emote_popup_obj = windowObj.value("emotePopup").toObject();
layout.emotePopupPos_ = QPoint(emote_popup_obj.value("x").toInt(),
emote_popup_obj.value("y").toInt());
{
auto emotePopup = windowObj["emotePopup"].toObject();
layout.emotePopupBounds_ = QRect{
emotePopup["x"].toInt(),
emotePopup["y"].toInt(),
emotePopup["width"].toInt(),
emotePopup["height"].toInt(),
};
}

layout.windows_.emplace_back(std::move(window));
}
Expand Down
2 changes: 1 addition & 1 deletion src/common/WindowDescriptors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: invalid case style for member 'emotePopupBounds_' [readability-identifier-naming]

Suggested change
QRect emotePopupBounds_;
QRect emotePopupBounds;


std::vector<WindowDescriptor> windows_;

Expand Down
38 changes: 24 additions & 14 deletions src/singletons/WindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,18 @@ void WindowManager::scrollToMessage(const MessagePtr &message)
this->scrollToMessageSignal.invoke(message);
}

QPoint WindowManager::emotePopupPos()
QRect WindowManager::emotePopupBounds() const
Copy link
Contributor

Choose a reason for hiding this comment

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

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"

{
return this->emotePopupPos_;
return this->emotePopupBounds_;
}

void WindowManager::setEmotePopupPos(QPoint pos)
void WindowManager::setEmotePopupBounds(QRect bounds)
{
this->emotePopupPos_ = pos;
if (this->emotePopupBounds_ != bounds)
{
this->emotePopupBounds_ = bounds;
this->queueSave();
}
}

void WindowManager::initialize(Settings &settings, const Paths &paths)
Expand Down Expand Up @@ -371,7 +375,7 @@ void WindowManager::initialize(Settings &settings, const Paths &paths)
windowLayout.activateOrAddChannel(desired->provider, desired->name);
}

this->emotePopupPos_ = windowLayout.emotePopupPos_;
this->emotePopupBounds_ = windowLayout.emotePopupBounds_;

this->applyWindowLayout(windowLayout);
}
Expand Down Expand Up @@ -483,10 +487,12 @@ void WindowManager::save()
windowObj.insert("width", rect.width());
windowObj.insert("height", rect.height());

QJsonObject emotePopupObj;
emotePopupObj.insert("x", this->emotePopupPos_.x());
emotePopupObj.insert("y", this->emotePopupPos_.y());
windowObj.insert("emotePopup", emotePopupObj);
windowObj["emotePopup"] = QJsonObject{
{"x", this->emotePopupBounds_.x()},
{"y", this->emotePopupBounds_.y()},
{"width", this->emotePopupBounds_.width()},
{"height", this->emotePopupBounds_.height()},
};

// window tabs
QJsonArray tabsArr;
Expand Down Expand Up @@ -753,7 +759,7 @@ void WindowManager::applyWindowLayout(const WindowLayout &layout)
}

// Set emote popup position
this->emotePopupPos_ = layout.emotePopupPos_;
this->emotePopupBounds_ = layout.emotePopupBounds_;

for (const auto &windowData : layout.windows_)
{
Expand Down Expand Up @@ -802,10 +808,14 @@ void WindowManager::applyWindowLayout(const WindowLayout &layout)
// Have to offset x by one because qt moves the window 1px too
// far to the left:w

window.setInitialBounds({windowData.geometry_.x(),
windowData.geometry_.y(),
windowData.geometry_.width(),
windowData.geometry_.height()});
window.setInitialBounds(
{
windowData.geometry_.x(),
windowData.geometry_.y(),
windowData.geometry_.width(),
windowData.geometry_.height(),
},
widgets::BoundsChecking::Off);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/singletons/WindowManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ class WindowManager final : public Singleton
*/
void scrollToMessage(const MessagePtr &message);

QPoint emotePopupPos();
void setEmotePopupPos(QPoint pos);
QRect emotePopupBounds() const;
Copy link
Contributor

Choose a reason for hiding this comment

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

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"

void setEmotePopupBounds(QRect bounds);

void initialize(Settings &settings, const Paths &paths) override;
void save() override;
Expand Down Expand Up @@ -154,7 +154,7 @@ class WindowManager final : public Singleton
bool initialized_ = false;
bool shuttingDown_ = false;

QPoint emotePopupPos_;
QRect emotePopupBounds_;

std::atomic<int> generation_{0};

Expand Down
60 changes: 49 additions & 11 deletions src/util/WidgetHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "QSize" is directly included [misc-include-cleaner]

src/util/WidgetHelpers.cpp:7:

+ #include <qsize.h>

{
if (screen == nullptr)
{
Expand All @@ -21,9 +20,6 @@ void moveWithinScreen(QWidget *window, QScreen *screen, QPoint point)
bool stickRight = false;
bool stickBottom = false;

const auto w = window->frameGeometry().width();
const auto h = window->frameGeometry().height();

if (point.x() < bounds.left())
{
point.setX(bounds.left());
Expand All @@ -32,30 +28,72 @@ void moveWithinScreen(QWidget *window, QScreen *screen, QPoint point)
{
point.setY(bounds.top());
}
if (point.x() + w > bounds.right())
if (point.x() + frameSize.width() > bounds.right())
{
stickRight = true;
point.setX(bounds.right() - w);
point.setX(bounds.right() - frameSize.width());
}
if (point.y() + h > bounds.bottom())
if (point.y() + frameSize.height() > bounds.bottom())
{
stickBottom = true;
point.setY(bounds.bottom() - h);
point.setY(bounds.bottom() - frameSize.height());
}

if (stickRight && stickBottom)
{
const QPoint globalCursorPos = QCursor::pos();
point.setY(globalCursorPos.y() - window->height() - 16);
point.setY(globalCursorPos.y() - height - 16);
}

window->move(point);
return point;
}

/// Move the `window` into the `screen` geometry if it's not already in there.
void moveWithinScreen(QWidget *window, QScreen *screen, QPoint point)
{
auto checked =
applyBounds(screen, point, window->frameSize(), window->height());
window->move(checked);
}

} // namespace

namespace chatterino::widgets {

QRect checkInitialBounds(QRect initialBounds, BoundsChecking mode)
{
switch (mode)
{
case BoundsChecking::Off: {
return initialBounds;
}
break;

case BoundsChecking::CursorPosition: {
return QRect{
applyBounds(QGuiApplication::screenAt(QCursor::pos()),
initialBounds.topLeft(), initialBounds.size(),
initialBounds.height()),
initialBounds.size(),
};
}
break;

case BoundsChecking::DesiredPosition: {
return QRect{
applyBounds(QGuiApplication::screenAt(initialBounds.topLeft()),
initialBounds.topLeft(), initialBounds.size(),
initialBounds.height()),
initialBounds.size(),
};
}
break;
default:
assert(false && "Invalid bounds checking mode");
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "assert" is directly included [misc-include-cleaner]

src/util/WidgetHelpers.cpp:7:

+ #include <cassert>

return initialBounds;
}
}

void moveWindowTo(QWidget *window, QPoint position, BoundsChecking mode)
{
switch (mode)
Expand Down
9 changes: 9 additions & 0 deletions src/util/WidgetHelpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
class QWidget;
class QPoint;
class QScreen;
class QRect;

namespace chatterino::widgets {

Expand All @@ -17,6 +18,14 @@ enum class BoundsChecking {
DesiredPosition,
};

/// Applies bounds checking to @a initialBounds.
///
/// @param initialBounds The bounds to check.
/// @param mode The desired bounds checking.
/// @returns The potentially modified bounds.
QRect checkInitialBounds(QRect initialBounds,
BoundsChecking mode = BoundsChecking::DesiredPosition);

/// Moves the `window` to the (global) `position`
/// while doing bounds-checking according to `mode` to ensure the window stays on one screen.
///
Expand Down
9 changes: 6 additions & 3 deletions src/widgets/BaseWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
# pragma comment(lib, "Dwmapi.lib")

# include <QHBoxLayout>
# include <QMargins>
# include <QOperatingSystemVersion>
# include <QWindow>
#endif

#include "widgets/helper/TitlebarButton.hpp"
Expand Down Expand Up @@ -251,16 +253,17 @@ BaseWindow::~BaseWindow()
DebugCount::decrease("BaseWindow");
}

void BaseWindow::setInitialBounds(const QRect &bounds)
void BaseWindow::setInitialBounds(QRect bounds, widgets::BoundsChecking mode)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "QRect" is directly included [misc-include-cleaner]

src/widgets/BaseWindow.cpp:19:

+ #include <qpaintdevice.h>

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "chatterino::widgets::BoundsChecking" is directly included [misc-include-cleaner]

src/widgets/BaseWindow.cpp:9:

- #include "util/WindowsHelper.hpp"
+ #include "util/WidgetHelpers.hpp"
+ #include "util/WindowsHelper.hpp"

{
bounds = widgets::checkInitialBounds(bounds, mode);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "chatterino::widgets::checkInitialBounds" is directly included [misc-include-cleaner]

    bounds = widgets::checkInitialBounds(bounds, mode);
                      ^

#ifdef USEWINSDK
this->initalBounds_ = bounds;
#else
this->setGeometry(bounds);
#endif
}

QRect BaseWindow::getBounds()
QRect BaseWindow::getBounds() const
{
#ifdef USEWINSDK
return this->currentBounds_;
Expand Down Expand Up @@ -444,7 +447,7 @@ QWidget *BaseWindow::getLayoutContainer()
}
}

bool BaseWindow::hasCustomWindowFrame()
bool BaseWindow::hasCustomWindowFrame() const
{
return BaseWindow::supportsCustomWindowFrame() && this->enableCustomFrame_;
}
Expand Down
6 changes: 3 additions & 3 deletions src/widgets/BaseWindow.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ class BaseWindow : public BaseWidget
QWidget *parent = nullptr);
~BaseWindow() override;

void setInitialBounds(const QRect &bounds);
QRect getBounds();
void setInitialBounds(QRect bounds, widgets::BoundsChecking mode);
QRect getBounds() const;

QWidget *getLayoutContainer();
bool hasCustomWindowFrame();
bool hasCustomWindowFrame() const;
TitleBarButton *addTitleBarButton(const TitleBarButtonStyle &style,
std::function<void()> onClicked);
EffectLabel *addTitleBarLabel(std::function<void()> onClicked);
Expand Down
32 changes: 27 additions & 5 deletions src/widgets/dialogs/EmotePopup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,18 @@ EmoteMap filterEmoteMap(const QString &text,
namespace chatterino {

EmotePopup::EmotePopup(QWidget *parent)
: BasePopup(BaseWindow::EnableCustomFrame, parent)
: BasePopup({BaseWindow::EnableCustomFrame, BaseWindow::DisableLayoutSave},
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "chatterino::BasePopup" is directly included [misc-include-cleaner]

src/widgets/dialogs/EmotePopup.cpp:20:

- #include "widgets/helper/ChannelView.hpp"
+ #include "widgets/BasePopup.hpp"
+ #include "widgets/helper/ChannelView.hpp"

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "chatterino::BaseWindow" is directly included [misc-include-cleaner]

src/widgets/dialogs/EmotePopup.cpp:20:

- #include "widgets/helper/ChannelView.hpp"
+ #include "widgets/BaseWindow.hpp"
+ #include "widgets/helper/ChannelView.hpp"

parent)
, search_(new QLineEdit())
, notebook_(new Notebook(this))
{
// this->setStayInScreenRect(true);
this->moveTo(getIApp()->getWindows()->emotePopupPos(),
widgets::BoundsChecking::DesiredPosition);
auto bounds = getIApp()->getWindows()->emotePopupBounds();
if (bounds.size().isEmpty())
{
bounds.setSize({300, 500});
}
this->setInitialBounds(bounds, widgets::BoundsChecking::DesiredPosition);
Copy link
Contributor

Choose a reason for hiding this comment

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

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"


auto *layout = new QVBoxLayout();
this->getLayoutContainer()->setLayout(layout);
Expand Down Expand Up @@ -594,10 +599,27 @@ void EmotePopup::filterEmotes(const QString &searchText)
this->searchView_->show();
}

void EmotePopup::saveBounds() const
{
getIApp()->getWindows()->setEmotePopupBounds(this->getBounds());
}

void EmotePopup::resizeEvent(QResizeEvent *event)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "QResizeEvent" is directly included [misc-include-cleaner]

void EmotePopup::resizeEvent(QResizeEvent *event)
                             ^

{
this->saveBounds();
BasePopup::resizeEvent(event);
}

void EmotePopup::moveEvent(QMoveEvent *event)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "QMoveEvent" is directly included [misc-include-cleaner]

void EmotePopup::moveEvent(QMoveEvent *event)
                           ^

{
this->saveBounds();
BasePopup::moveEvent(event);
}

void EmotePopup::closeEvent(QCloseEvent *event)
{
getIApp()->getWindows()->setEmotePopupPos(this->pos());
BaseWindow::closeEvent(event);
this->saveBounds();
BasePopup::closeEvent(event);
}

} // namespace chatterino
6 changes: 6 additions & 0 deletions src/widgets/dialogs/EmotePopup.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ class EmotePopup : public BasePopup

pajlada::Signals::Signal<Link> linkClicked;

protected:
void resizeEvent(QResizeEvent *event) override;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "QResizeEvent" is directly included [misc-include-cleaner]

    void resizeEvent(QResizeEvent *event) override;
                     ^

void moveEvent(QMoveEvent *event) override;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "QMoveEvent" is directly included [misc-include-cleaner]

    void moveEvent(QMoveEvent *event) override;
                   ^


private:
ChannelView *globalEmotesView_{};
ChannelView *channelEmotesView_{};
Expand All @@ -47,6 +51,8 @@ class EmotePopup : public BasePopup
void filterEmotes(const QString &text);
void addShortcuts() override;
bool eventFilter(QObject *object, QEvent *event) override;

void saveBounds() const;
};

} // namespace chatterino
Loading