Skip to content

Commit

Permalink
Fix tooltip & popup positioning (Chatterino#4740)
Browse files Browse the repository at this point in the history
* Fix tooltip & popup positioning

This tries to ensure the tooltip & popups are created on the correct
monitor

* Add changelog entry

* Clean up debug output

* Use the full frame geometry to figure out screen bound movements

* Remove the now-unused `setStayInScreenRect` function

* Change the UserInfoPopup offset to be based on its width & height
instead

* Remove more debug output
  • Loading branch information
pajlada authored and Nerixyz committed Aug 12, 2023
1 parent 3550121 commit 47c16be
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 54 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- Bugfix: Fix visual glitches with smooth scrolling. (#4501)
- Bugfix: Fixed pings firing for the "Your username" highlight when not signed in. (#4698)
- Bugfix: Fixed partially broken filters on Qt 6 builds. (#4702)
- Bugfix: Fixed tooltips & popups sometimes showing up on the wrong monitor. (#4740)
- Bugfix: Fixed some network errors having `0` as their HTTP status. (#4704)
- Bugfix: Fixed crash that could occurr when closing the usercard too quickly after blocking or unblocking a user. (#4711)
- Bugfix: Fixed highlights sometimes not working after changing sound device, or switching users in your operating system. (#4729)
Expand Down
3 changes: 2 additions & 1 deletion src/controllers/commands/CommandController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,8 @@ void CommandController::initialize(Settings &, Paths &paths)
static_cast<QWidget *>(&(getApp()->windows->getMainWindow())),
currentSplit);
userPopup->setData(userName, channel);
userPopup->move(QCursor::pos());
userPopup->moveTo(QCursor::pos(), false,
BaseWindow::BoundsChecker::CursorPosition);
userPopup->show();
return "";
});
Expand Down
64 changes: 32 additions & 32 deletions src/widgets/BaseWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "widgets/helper/EffectLabel.hpp"
#include "widgets/Label.hpp"
#include "widgets/TooltipWidget.hpp"
#include "widgets/Window.hpp"

#include <QApplication>
#include <QFont>
Expand Down Expand Up @@ -240,18 +241,6 @@ void BaseWindow::init()
#endif
}

void BaseWindow::setStayInScreenRect(bool value)
{
this->stayInScreenRect_ = value;

this->moveIntoDesktopRect(this->pos());
}

bool BaseWindow::getStayInScreenRect() const
{
return this->stayInScreenRect_;
}

void BaseWindow::setActionOnFocusLoss(ActionOnFocusLoss value)
{
this->actionOnFocusLoss_ = value;
Expand Down Expand Up @@ -514,15 +503,34 @@ void BaseWindow::leaveEvent(QEvent *)
TooltipWidget::instance()->hide();
}

void BaseWindow::moveTo(QWidget *parent, QPoint point, bool offset)
void BaseWindow::moveTo(QPoint point, bool offset, BoundsChecker boundsChecker)
{
if (offset)
{
point.rx() += 16;
point.ry() += 16;
}

this->moveIntoDesktopRect(point);
switch (boundsChecker)
{
case BoundsChecker::Off: {
// The bounds checker is off, *just* move the window
this->move(point);
}
break;

case BoundsChecker::CursorPosition: {
// The bounds checker is on, use the cursor position as the origin
this->moveWithinScreen(point, QCursor::pos());
}
break;

case BoundsChecker::DesiredPosition: {
// The bounds checker is on, use the desired position as the origin
this->moveWithinScreen(point, point);
}
break;
}
}

void BaseWindow::resizeEvent(QResizeEvent *)
Expand Down Expand Up @@ -576,24 +584,13 @@ void BaseWindow::closeEvent(QCloseEvent *)

void BaseWindow::showEvent(QShowEvent *)
{
this->moveIntoDesktopRect(this->pos());
if (this->frameless_)
{
QTimer::singleShot(30, this, [this] {
this->moveIntoDesktopRect(this->pos());
});
}
}

void BaseWindow::moveIntoDesktopRect(QPoint point)
void BaseWindow::moveWithinScreen(QPoint point, QPoint origin)
{
if (!this->stayInScreenRect_)
{
return;
}

// move the widget into the screen geometry if it's not already in there
auto *screen = QApplication::screenAt(point);
auto *screen = QApplication::screenAt(origin);

if (screen == nullptr)
{
screen = QApplication::primaryScreen();
Expand All @@ -603,6 +600,9 @@ void BaseWindow::moveIntoDesktopRect(QPoint point)
bool stickRight = false;
bool stickBottom = false;

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

if (point.x() < bounds.left())
{
point.setX(bounds.left());
Expand All @@ -611,15 +611,15 @@ void BaseWindow::moveIntoDesktopRect(QPoint point)
{
point.setY(bounds.top());
}
if (point.x() + this->width() > bounds.right())
if (point.x() + w > bounds.right())
{
stickRight = true;
point.setX(bounds.right() - this->width());
point.setX(bounds.right() - w);
}
if (point.y() + this->height() > bounds.bottom())
if (point.y() + h > bounds.bottom())
{
stickBottom = true;
point.setY(bounds.bottom() - this->height());
point.setY(bounds.bottom() - h);
}

if (stickRight && stickBottom)
Expand Down
26 changes: 19 additions & 7 deletions src/widgets/BaseWindow.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ class BaseWindow : public BaseWidget
DisableLayoutSave = 128,
};

enum class BoundsChecker {
// Don't attempt to do any "stay in screen" stuff, just move me!
Off,

// Attempt to keep the window within bounds of the screen the cursor is on
CursorPosition,

// Attempt to keep the window within bounds of the screen the desired position is on
DesiredPosition,
};

enum ActionOnFocusLoss { Nothing, Delete, Close, Hide };

explicit BaseWindow(FlagsEnum<Flags> flags_ = None,
Expand All @@ -51,15 +62,12 @@ class BaseWindow : public BaseWidget
std::function<void()> onClicked);
EffectLabel *addTitleBarLabel(std::function<void()> onClicked);

void setStayInScreenRect(bool value);
bool getStayInScreenRect() const;

void setActionOnFocusLoss(ActionOnFocusLoss value);
ActionOnFocusLoss getActionOnFocusLoss() const;

void moveTo(QWidget *widget, QPoint point, bool offset = true);
void moveTo(QPoint point, bool offset, BoundsChecker boundsChecker);

virtual float scale() const override;
float scale() const override;
float qtFontScale() const;

pajlada::Signals::NoArgSignal closing;
Expand Down Expand Up @@ -101,7 +109,12 @@ class BaseWindow : public BaseWidget

private:
void init();
void moveIntoDesktopRect(QPoint point);

/**
*
**/
void moveWithinScreen(QPoint point, QPoint origin);

void calcButtonsSizes();
void drawCustomWindowFrame(QPainter &painter);
void onFocusLost();
Expand All @@ -121,7 +134,6 @@ class BaseWindow : public BaseWidget
bool enableCustomFrame_;
ActionOnFocusLoss actionOnFocusLoss_ = Nothing;
bool frameless_;
bool stayInScreenRect_ = false;
bool shown_ = false;
FlagsEnum<Flags> flags_;
float nativeScale_ = 1;
Expand Down
2 changes: 0 additions & 2 deletions src/widgets/TooltipWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ TooltipWidget::TooltipWidget(BaseWidget *parent)
this->setAttribute(Qt::WA_TranslucentBackground);
this->setWindowFlag(Qt::WindowStaysOnTopHint, true);

this->setStayInScreenRect(true);

// Default to using vertical layout
this->initializeVLayout();
this->setLayout(this->vLayout_);
Expand Down
5 changes: 3 additions & 2 deletions src/widgets/dialogs/EmotePopup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,9 @@ EmotePopup::EmotePopup(QWidget *parent)
, search_(new QLineEdit())
, notebook_(new Notebook(this))
{
this->setStayInScreenRect(true);
this->moveTo(this, getApp()->windows->emotePopupPos(), false);
// this->setStayInScreenRect(true);
this->moveTo(getApp()->windows->emotePopupPos(), false,
BaseWindow::BoundsChecker::DesiredPosition);

auto *layout = new QVBoxLayout();
this->getLayoutContainer()->setLayout(layout);
Expand Down
1 change: 0 additions & 1 deletion src/widgets/dialogs/ReplyThreadPopup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ ReplyThreadPopup::ReplyThreadPopup(bool closeAutomatically, QWidget *parent,
, split_(split)
{
this->setWindowTitle(QStringLiteral("Reply Thread"));
this->setStayInScreenRect(true);

HotkeyController::HotkeyMap actions{
{"delete",
Expand Down
4 changes: 0 additions & 4 deletions src/widgets/dialogs/UserInfoPopup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ UserInfoPopup::UserInfoPopup(bool closeAutomatically, QWidget *parent,
assert(split != nullptr &&
"split being nullptr causes lots of bugs down the road");
this->setWindowTitle("Usercard");
this->setStayInScreenRect(true);

HotkeyController::HotkeyMap actions{
{"delete",
Expand Down Expand Up @@ -734,9 +733,6 @@ void UserInfoPopup::setData(const QString &name,
this->userStateChanged_.invoke();

this->updateLatestMessages();
QTimer::singleShot(1, this, [this] {
this->setStayInScreenRect(true);
});
}

void UserInfoPopup::updateLatestMessages()
Expand Down
1 change: 0 additions & 1 deletion src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ QuickSwitcherPopup::QuickSwitcherPopup(QWidget *parent)

this->initWidgets();

this->setStayInScreenRect(true);
const QRect geom = parent->geometry();
// This places the popup in the middle of the parent widget
this->setGeometry(QStyle::alignedRect(Qt::LeftToRight, Qt::AlignCenter,
Expand Down
8 changes: 5 additions & 3 deletions src/widgets/helper/ChannelView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1811,7 +1811,8 @@ void ChannelView::mouseMoveEvent(QMouseEvent *event)
}
}

tooltipWidget->moveTo(this, event->globalPos());
tooltipWidget->moveTo(event->globalPos(), true,
BaseWindow::BoundsChecker::CursorPosition);
tooltipWidget->setWordWrap(isLinkValid);
tooltipWidget->show();
}
Expand Down Expand Up @@ -2664,8 +2665,9 @@ void ChannelView::showUserInfoPopup(const QString &userName,
: this->underlyingChannel_;
userPopup->setData(userName, contextChannel, openingChannel);

QPoint offset(int(150 * this->scale()), int(70 * this->scale()));
userPopup->move(QCursor::pos() - offset);
QPoint offset(userPopup->width() / 3, userPopup->height() / 5);
userPopup->moveTo(QCursor::pos() - offset, false,
BaseWindow::BoundsChecker::CursorPosition);
userPopup->show();
}

Expand Down
2 changes: 1 addition & 1 deletion src/widgets/splits/SplitHeader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ void SplitHeader::enterEvent(QEvent *event)
auto pos = this->mapToGlobal(this->rect().bottomLeft()) +
QPoint((this->width() - tooltip->width()) / 2, 1);

tooltip->moveTo(this, pos, false);
tooltip->moveTo(pos, false, BaseWindow::BoundsChecker::CursorPosition);
tooltip->show();
}

Expand Down

0 comments on commit 47c16be

Please sign in to comment.