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

Larger user card drag area #3508

Merged

Conversation

goldbattle
Copy link
Contributor

@goldbattle goldbattle commented Jan 14, 2022

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

Currently the user profile card has a very small drag area towards the top (I am on windows 10). I also see HasanAbi struggle with this on stream when trying to drag the user card. This adds a mouse callback which allows for dragging if a user clicks anyplace in the window. There is also a small deadzone to prevent from people dragging the window when they really just wanted to click on something.

Before:

bandicam.2022-01-14.18-29-33-565.mp4

After:

bandicam.2022-01-14.18-30-17-354.mp4

[Felanbird]: Fixes #2170 (tested with 150% resolution as well just to be sure)

Copy link
Collaborator

@Felanbird Felanbird left a comment

Choose a reason for hiding this comment

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

Needs a changelog but otherwise the functionality works as intended 👍
image

@pajlada
Copy link
Member

pajlada commented Jan 15, 2022

🤓 Does not work as expected on Linux using i3.
Calling move from within mouseMoveEvent will cause another mouseMoveEvent to occur, creating a potential infinite recursion.
Windows might be better at handling it, deduplicating the move events or something. See https://doc.qt.io/qt-5/qwidget.html#pos-prop

My patch for a decent Linux+i3 experience looks like this
diff --git a/lib/qtkeychain b/lib/qtkeychain
--- a/lib/qtkeychain
+++ b/lib/qtkeychain
@@ -1 +1 @@
-Subproject commit de954627363b0b4bff9a2616f1a409b7e14d5df9
+Subproject commit de954627363b0b4bff9a2616f1a409b7e14d5df9-dirty
diff --git a/src/widgets/dialogs/UserInfoPopup.cpp b/src/widgets/dialogs/UserInfoPopup.cpp
index bab6c307..3dd85cd1 100644
--- a/src/widgets/dialogs/UserInfoPopup.cpp
+++ b/src/widgets/dialogs/UserInfoPopup.cpp
@@ -32,6 +32,8 @@
 #include <QNetworkAccessManager>
 #include <QNetworkReply>
 
+#include <chrono>
+
 const QString TEXT_VIEWS("Views: %1");
 const QString TEXT_FOLLOWERS("Followers: %1");
 const QString TEXT_CREATED("Created: %1");
@@ -133,10 +135,10 @@ UserInfoPopup::UserInfoPopup(bool closeAutomatically, QWidget *parent)
                                     : userInfoPopupFlags,
                  parent)
     , hack_(new bool)
+    , dragTimer_(this)
 {
     this->setWindowTitle("Usercard");
     this->setStayInScreenRect(true);
-    this->setMouseTracking(true);
 
     if (closeAutomatically)
         this->setActionOnFocusLoss(BaseWindow::Delete);
@@ -429,6 +431,21 @@ UserInfoPopup::UserInfoPopup(bool closeAutomatically, QWidget *parent)
 
     this->installEvents();
     this->setSizePolicy(QSizePolicy::Ignored, QSizePolicy::Policy::Ignored);
+
+    this->dragTimer_.callOnTimeout(
+        [this, hack = std::weak_ptr<bool>(this->hack_)] {
+            if (!hack.lock())
+            {
+                // Ensure this timer is never called after the object has been destroyed
+                return;
+            }
+            if (!this->isMoving_)
+            {
+                return;
+            }
+
+            this->move(this->requestedDragPos_);
+        });
 }
 
 void UserInfoPopup::themeChangedEvent()
@@ -458,15 +475,16 @@ void UserInfoPopup::mousePressEvent(QMouseEvent *event)
 {
     if (event->button() == Qt::MouseButton::LeftButton)
     {
-        this->isDragging_ = true;
+        this->dragTimer_.start(std::chrono::milliseconds(17));
         this->startPosDrag_ = event->pos();
+        this->movingRelativePos = event->localPos();
     }
 }
 
 void UserInfoPopup::mouseReleaseEvent(QMouseEvent *event)
 {
-    this->isDragging_ = false;
     this->isMoving_ = false;
+    this->dragTimer_.stop();
 }
 
 void UserInfoPopup::mouseMoveEvent(QMouseEvent *event)
@@ -477,7 +495,8 @@ void UserInfoPopup::mouseMoveEvent(QMouseEvent *event)
     auto movePos = event->pos() - this->startPosDrag_;
     if (this->isMoving_ || movePos.manhattanLength() > 10.0)
     {
-        move(window()->pos() + movePos);
+        this->requestedDragPos_ =
+            (event->screenPos() - this->movingRelativePos).toPoint();
         this->isMoving_ = true;
     }
 }
diff --git a/src/widgets/dialogs/UserInfoPopup.hpp b/src/widgets/dialogs/UserInfoPopup.hpp
index 0d7874af..34e2cff1 100644
--- a/src/widgets/dialogs/UserInfoPopup.hpp
+++ b/src/widgets/dialogs/UserInfoPopup.hpp
@@ -44,10 +44,18 @@ private:
     QString avatarUrl_;
     ChannelPtr channel_;
 
-    bool isDragging_ = false;
+    // isMoving_ is set to true if the user is holding the left mouse button down and has moved the mouse a small amount away from the original click point (startPosDrag_)
     bool isMoving_ = false;
+    // startPosDrag_ is the coordinates where the user originally pressed the mouse button down to start dragging
     QPoint startPosDrag_;
 
+    // requestDragPos_ is the final screen coordinates where the widget should be moved to.
+    // Takes the relative position of where the user originally clicked the widget into account
+    QPoint requestedDragPos_;
+
+    // dragTimer_ is called ~60 times per second once the user has initiated dragging
+    QTimer dragTimer_;
+
     pajlada::Signals::NoArgSignal userStateChanged_;
 
     std::unique_ptr<pajlada::Signals::ScopedConnection> refreshConnection_;

Please try this patch out on Windows to make sure it works as expected, then I'll re-review it

@goldbattle
Copy link
Contributor Author

Patch worked well on windows, I think the logic is good too.

@Felanbird
Copy link
Collaborator

As noted this patch seems perfectly fine on Windows.

Here's my changelog variation I forgot to suggest 🤷
- Bugfix: Fixed being unable to drag the user card window from certain spots. (#3508)

Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Confirmed that this change works as expected on macOS both with the "automatically close popup when it loses focus" enabled and disabled

@pajlada pajlada merged commit 201cd67 into Chatterino:master Jan 16, 2022
zneix added a commit to SevenTV/chatterino7 that referenced this pull request Jan 16, 2022
Now we're on commit 1d272c3; Changes from upstream we've pulled

- Minor: Make animated emote playback speed match browser (Firefox and Chrome) behaviour. (Chatterino#3506)
- Minor: Add feedback when using the whisper command `/w` incorrectly. (Chatterino#3439)
- Minor: Add feedback when writing a non-command message in the `/whispers` split. (Chatterino#3439)
- Minor: Opening streamlink through hotkeys and/or split header menu matches `/streamlink` command and shows feedback in chat as well. (Chatterino#3510)
- Minor: Removed timestamp from AutoMod messages. (Chatterino#3503)
- Minor: Added ability to copy message ID with `Shift + Right Click`. (Chatterino#3481)
- Bugfix: Fixed crash that would occur if the user right-clicked AutoMod badge. (Chatterino#3496)
- Bugfix: Fixed being unable to drag the user card window from certain spots. (Chatterino#3508)
- Bugfix: Fixed being unable to open a usercard from inside a usercard while "Automatically close user popup when it loses focus" was enabled. (Chatterino#3518)
- Bugfix: Usercards no longer close when the originating window (e.g. a search popup) is closed. (Chatterino#3518)
@goldbattle goldbattle deleted the to-upstream-larger-drag-area-user-card branch January 17, 2022 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User card issue
3 participants