Use Qt's dialog where applicable (#1843)

CHANGELOG.md

@@ -27,6 +27,8 @@
 - Minor: Improve UX of the "Login expired!" message (#2029)
 - Minor: PageUp and PageDown now scroll in the selected split (#2070, #2081)
 - Minor: Allow highlights to be excluded from `/mentions`. Excluded highlights will not trigger tab highlights either. (#1793, #2036)
+- Minor: Flag all popup dialogs as actual dialogs so they get the relevant window manager hints (#1843)
+- Bugfix: Fix crash occurring when pressing Escape in the Color Picker Dialog (#1843)
 - Bugfix: Fix bug preventing users from setting the highlight color of the second entry in the "User" highlights tab (#1898)
 - Bugfix: Fix bug where the "check user follow state" event could trigger a network request requesting the user to follow or unfollow a user. By itself its quite harmless as it just repeats to Twitch the same follow state we had, so no follows should have been lost by this but it meant there was a rogue network request that was fired that could cause a crash (#1906)
 - Bugfix: /usercard command will now respect the "Automatically close user popup" setting (#1918)

src/common/FlagsEnum.hpp

@@ -59,6 +59,15 @@ public:
         return static_cast<Q>(this->value_) & static_cast<Q>(flag);
     }
 
+    FlagsEnum operator|(T flag)
+    {
+        FlagsEnum xd;
+        xd.value_ = this->value_;
+        xd.set(flag, true);
+
+        return xd;
+    }
+
     bool hasAny(FlagsEnum flags) const
     {
         return static_cast<Q>(this->value_) & static_cast<Q>(flags.value_);

src/controllers/commands/CommandController.cpp

@@ -14,8 +14,10 @@
 #include "singletons/Paths.hpp"
 #include "singletons/Settings.hpp"
 #include "singletons/Theme.hpp"
+#include "singletons/WindowManager.hpp"
 #include "util/CombinePath.hpp"
 #include "util/Twitch.hpp"
+#include "widgets/Window.hpp"
 #include "widgets/dialogs/UserInfoPopup.hpp"
 
 #include <QApplication>
@@ -405,7 +407,9 @@ void CommandController::initialize(Settings &, Paths &paths)
             return "";
         }
 
-        auto *userPopup = new UserInfoPopup(getSettings()->autoCloseUserPopup);
+        auto *userPopup = new UserInfoPopup(
+            getSettings()->autoCloseUserPopup,
+            static_cast<QWidget *>(&(getApp()->windows->getMainWindow())));
         userPopup->setData(words[1], channel);
         userPopup->move(QCursor::pos());
         userPopup->show();

src/singletons/WindowManager.cpp

@@ -49,10 +49,12 @@ namespace {
 using SplitNode = SplitContainer::Node;
 using SplitDirection = SplitContainer::Direction;
 
-void WindowManager::showSettingsDialog(SettingsDialogPreference preference)
+void WindowManager::showSettingsDialog(QWidget *parent,
+                                       SettingsDialogPreference preference)
 {
-    QTimer::singleShot(
+    QTimer::singleShot(80, [parent, preference] {
-        80, [preference] { SettingsDialog::showDialog(preference); });
+        SettingsDialog::showDialog(parent, preference);
+    });
 }
 
 void WindowManager::showAccountSelectPopup(QPoint point)

src/singletons/WindowManager.hpp

@@ -30,6 +30,7 @@ public:
     static IndirectChannel decodeChannel(const SplitDescriptor &descriptor);
 
     void showSettingsDialog(
+        QWidget *parent,
         SettingsDialogPreference preference = SettingsDialogPreference());
 
     // Show the account selector widget at point

src/widgets/AccountSwitchPopup.cpp

@@ -30,8 +30,9 @@ AccountSwitchPopup::AccountSwitchPopup(QWidget *parent)
     hbox->addWidget(manageAccountsButton);
     vbox->addLayout(hbox);
 
-    connect(manageAccountsButton, &QPushButton::clicked, []() {
+    connect(manageAccountsButton, &QPushButton::clicked, [this]() {
-        SettingsDialog::showDialog(SettingsDialogPreference::Accounts);  //
+        SettingsDialog::showDialog(this,
+                                   SettingsDialogPreference::Accounts);  //
     });
 
     this->getLayoutContainer()->setLayout(vbox);

src/widgets/BasePopup.cpp

@@ -3,7 +3,7 @@
 namespace chatterino {
 
 BasePopup::BasePopup(FlagsEnum<Flags> _flags, QWidget *parent)
-    : BaseWindow(std::move(_flags), parent)
+    : BaseWindow(_flags | Dialog, parent)
 {
 }
 

src/widgets/BaseWindow.cpp

@@ -45,9 +45,9 @@
 namespace chatterino {
 
 BaseWindow::BaseWindow(FlagsEnum<Flags> _flags, QWidget *parent)
-    : BaseWidget(parent,
+    : BaseWidget(parent, (_flags.has(Dialog) ? Qt::Dialog : Qt::Window) |
-                 Qt::Window | (_flags.has(TopMost) ? Qt::WindowStaysOnTopHint
+                             (_flags.has(TopMost) ? Qt::WindowStaysOnTopHint
-                                                   : Qt::WindowFlags()))
+                                                  : Qt::WindowFlags()))
     , enableCustomFrame_(_flags.has(EnableCustomFrame))
     , frameless_(_flags.has(Frameless))
     , flags_(_flags)

src/widgets/BaseWindow.hpp

@@ -30,6 +30,7 @@ public:
         DisableCustomScaling = 8,
         FramelessDraggable = 16,
         DontFocus = 32,
+        Dialog = 64,
     };
 
     enum ActionOnFocusLoss { Nothing, Delete, Close, Hide };

src/widgets/Notebook.cpp

@@ -652,7 +652,7 @@ void SplitNotebook::addCustomButtons()
     settingsBtn->setIcon(NotebookButton::Settings);
 
     QObject::connect(settingsBtn, &NotebookButton::leftClicked,
-                     [] { getApp()->windows->showSettingsDialog(); });
+                     [this] { getApp()->windows->showSettingsDialog(this); });
 
     // account
     auto userBtn = this->addCustomButton();

src/widgets/Window.cpp

@@ -158,8 +158,9 @@ void Window::addCustomTitlebarButtons()
         return;
 
     // settings
-    this->addTitleBarButton(TitleBarButtonStyle::Settings,
+    this->addTitleBarButton(TitleBarButtonStyle::Settings, [this] {
-                            [] { getApp()->windows->showSettingsDialog(); });
+        getApp()->windows->showSettingsDialog(this);  //
+    });
 
     // updates
     auto update = this->addTitleBarButton(TitleBarButtonStyle::None, [] {});
@@ -291,7 +292,9 @@ void Window::addShortcuts()
 {
     /// Initialize program-wide hotkeys
     // Open settings
-    createWindowShortcut(this, "CTRL+P", [] { SettingsDialog::showDialog(); });
+    createWindowShortcut(this, "CTRL+P", [this] {
+        SettingsDialog::showDialog(this);  //
+    });
 
     // Switch tab
     createWindowShortcut(this, "CTRL+T", [this] {
@@ -398,8 +401,9 @@ void Window::addMenuBar()
     QMenu *menu = mainMenu->addMenu(QString());
     QAction *prefs = menu->addAction(QString());
     prefs->setMenuRole(QAction::PreferencesRole);
-    connect(prefs, &QAction::triggered, this,
+    connect(prefs, &QAction::triggered, this, [this] {
-            [] { SettingsDialog::showDialog(); });
+        SettingsDialog::showDialog(this);  //
+    });
 
     // Window menu.
     QMenu *windowMenu = mainMenu->addMenu(QString("Window"));

src/widgets/dialogs/ColorPickerDialog.cpp

@@ -123,7 +123,7 @@ QColor ColorPickerDialog::selectedColor() const
 
 void ColorPickerDialog::closeEvent(QCloseEvent *)
 {
-    this->closed.invoke();
+    this->closed.invoke(this->selectedColor());
 }
 
 void ColorPickerDialog::themeChangedEvent()

src/widgets/dialogs/ColorPickerDialog.hpp

@@ -27,7 +27,7 @@ public:
      * You can connect to the ::closed signal of this instance to get notified
      * when the dialog is closed.
      */
-    ColorPickerDialog(const QColor &initial, QWidget *parent = nullptr);
+    ColorPickerDialog(const QColor &initial, QWidget *parent);
 
     ~ColorPickerDialog();
 
@@ -42,7 +42,7 @@ public:
      */
     QColor selectedColor() const;
 
-    pajlada::Signals::NoArgSignal closed;
+    pajlada::Signals::Signal<QColor> closed;
 
 protected:
     void closeEvent(QCloseEvent *);

src/widgets/dialogs/QualityPopup.cpp

@@ -1,10 +1,15 @@
 #include "QualityPopup.hpp"
+#include "Application.hpp"
+#include "singletons/WindowManager.hpp"
 #include "util/StreamLink.hpp"
+#include "widgets/Window.hpp"
 
 namespace chatterino {
 
 QualityPopup::QualityPopup(const QString &_channelName, QStringList options)
-    : channelName_(_channelName)
+    : BasePopup({},
+                static_cast<QWidget *>(&(getApp()->windows->getMainWindow())))
+    , channelName_(_channelName)
 {
     this->ui_.okButton.setText("OK");
     this->ui_.cancelButton.setText("Cancel");

src/widgets/dialogs/QualityPopup.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "widgets/BaseWindow.hpp"
+#include "widgets/BasePopup.hpp"
 
 #include <QComboBox>
 #include <QDialogButtonBox>
@@ -9,7 +9,7 @@
 
 namespace chatterino {
 
-class QualityPopup : public BaseWindow
+class QualityPopup : public BasePopup
 {
 public:
     QualityPopup(const QString &_channelName, QStringList options);

src/widgets/dialogs/SelectChannelDialog.cpp

@@ -26,7 +26,9 @@
 namespace chatterino {
 
 SelectChannelDialog::SelectChannelDialog(QWidget *parent)
-    : BaseWindow(BaseWindow::EnableCustomFrame, parent)
+    : BaseWindow(
+          {BaseWindow::Flags::EnableCustomFrame, BaseWindow::Flags::Dialog},
+          parent)
     , selectedChannel_(Channel::getEmpty())
 {
     this->setWindowTitle("Select a channel to join");

src/widgets/dialogs/SettingsDialog.cpp

@@ -24,8 +24,10 @@
 
 namespace chatterino {
 
-SettingsDialog::SettingsDialog()
+SettingsDialog::SettingsDialog(QWidget *parent)
-    : BaseWindow(BaseWindow::DisableCustomScaling)
+    : BaseWindow(
+          {BaseWindow::Flags::DisableCustomScaling, BaseWindow::Flags::Dialog},
+          parent)
 {
     this->setWindowTitle("Chatterino Settings");
     this->resize(815, 600);
@@ -41,6 +43,10 @@ SettingsDialog::SettingsDialog()
         this->ui_.search->setFocus();
         this->ui_.search->selectAll();
     });
+
+    // Disable the ? button in the titlebar until we decide to use it
+    this->setWindowFlags(this->windowFlags() &
+                         ~Qt::WindowContextHelpButtonHint);
 }
 
 void SettingsDialog::initUi()
@@ -238,9 +244,10 @@ SettingsDialogTab *SettingsDialog::tab(SettingsTabId id)
     return nullptr;
 }
 
-void SettingsDialog::showDialog(SettingsDialogPreference preferredTab)
+void SettingsDialog::showDialog(QWidget *parent,
+                                SettingsDialogPreference preferredTab)
 {
-    static SettingsDialog *instance = new SettingsDialog();
+    static SettingsDialog *instance = new SettingsDialog(parent);
     static bool hasShownBefore = false;
     if (hasShownBefore)
         instance->refresh();

src/widgets/dialogs/SettingsDialog.hpp

@@ -31,10 +31,11 @@ enum class SettingsDialogPreference {
 
 class SettingsDialog : public BaseWindow
 {
-    SettingsDialog();
+    SettingsDialog(QWidget *parent);
 
 public:
-    static void showDialog(SettingsDialogPreference preferredTab =
+    static void showDialog(QWidget *parent,
+                           SettingsDialogPreference preferredTab =
                                SettingsDialogPreference::NoPreference);
 
 protected:

src/widgets/dialogs/UserInfoPopup.cpp

@@ -104,13 +104,22 @@ namespace {
 
 }  // namespace
 
-UserInfoPopup::UserInfoPopup(bool closeAutomatically)
+#ifdef Q_OS_LINUX
-    : BaseWindow(
+FlagsEnum<BaseWindow::Flags> userInfoPopupFlags{BaseWindow::Dialog,
-          closeAutomatically
+                                                BaseWindow::EnableCustomFrame};
-              ? FlagsEnum<BaseWindow::Flags>{BaseWindow::EnableCustomFrame,
+FlagsEnum<BaseWindow::Flags> userInfoPopupFlagsCloseAutomatically{
-                                             BaseWindow::Frameless,
+    BaseWindow::EnableCustomFrame};
-                                             BaseWindow::FramelessDraggable}
+#else
-              : BaseWindow::EnableCustomFrame)
+FlagsEnum<BaseWindow::Flags> userInfoPopupFlags{BaseWindow::EnableCustomFrame};
+FlagsEnum<BaseWindow::Flags> userInfoPopupFlagsCloseAutomatically{
+    BaseWindow::EnableCustomFrame, BaseWindow::Frameless,
+    BaseWindow::FramelessDraggable};
+#endif
+
+UserInfoPopup::UserInfoPopup(bool closeAutomatically, QWidget *parent)
+    : BaseWindow(closeAutomatically ? userInfoPopupFlagsCloseAutomatically
+                                    : userInfoPopupFlags,
+                 parent)
     , hack_(new bool)
 {
     this->setWindowTitle("Usercard");

src/widgets/dialogs/UserInfoPopup.hpp

@@ -18,7 +18,7 @@ class UserInfoPopup final : public BaseWindow
     Q_OBJECT
 
 public:
-    UserInfoPopup(bool closeAutomatically);
+    UserInfoPopup(bool closeAutomatically, QWidget *parent);
     ~UserInfoPopup();
 
     void setData(const QString &name, const ChannelPtr &channel);

src/widgets/helper/ChannelView.cpp

@@ -1937,7 +1937,8 @@ void ChannelView::hideEvent(QHideEvent *)
 
 void ChannelView::showUserInfoPopup(const QString &userName)
 {
-    auto *userPopup = new UserInfoPopup(getSettings()->autoCloseUserPopup);
+    auto *userPopup =
+        new UserInfoPopup(getSettings()->autoCloseUserPopup, this);
     userPopup->setData(userName, this->hasSourceChannel()
                                      ? this->sourceChannel_
                                      : this->underlyingChannel_);
@@ -1997,7 +1998,8 @@ void ChannelView::handleLinkClick(QMouseEvent *event, const Link &link,
         break;
 
         case Link::OpenAccountsPage: {
-            SettingsDialog::showDialog(SettingsDialogPreference::Accounts);
+            SettingsDialog::showDialog(this,
+                                       SettingsDialogPreference::Accounts);
         }
         break;
 

src/widgets/helper/DebugPopup.hpp

@@ -1,10 +1,10 @@
 #pragma once
 
-#include <QWidget>
+#include "widgets/BasePopup.hpp"
 
 namespace chatterino {
 
-class DebugPopup : public QWidget
+class DebugPopup : public BasePopup
 {
 public:
     DebugPopup();

src/widgets/settingspages/AboutPage.cpp

@@ -4,6 +4,7 @@
 #include "common/Version.hpp"
 #include "util/LayoutCreator.hpp"
 #include "util/RemoveScrollAreaBackground.hpp"
+#include "widgets/BasePopup.hpp"
 #include "widgets/helper/SignalLabel.hpp"
 
 #include <QFormLayout>
@@ -230,17 +231,25 @@ void AboutPage::addLicense(QFormLayout *form, const QString &name,
     auto *a = new QLabel("<a href=\"" + website + "\">" + name + "</a>");
     a->setOpenExternalLinks(true);
     auto *b = new QLabel("<a href=\"" + licenseLink + "\">show license</a>");
-    QObject::connect(b, &QLabel::linkActivated, [licenseLink, name] {
+    QObject::connect(
-        auto *edit = new QTextEdit;
+        b, &QLabel::linkActivated, [parent = this, name, licenseLink] {
+            auto window =
+                new BasePopup(BaseWindow::Flags::EnableCustomFrame, parent);
+            window->setWindowTitle("Chatterino - License for " + name);
+            window->setAttribute(Qt::WA_DeleteOnClose);
+            auto layout = new QVBoxLayout();
+            auto *edit = new QTextEdit;
 
-        edit->setWindowTitle(
+            QFile file(licenseLink);
-            QString("Chatterino - showing %1's license").arg(name));
+            file.open(QIODevice::ReadOnly);
-        QFile file(licenseLink);
+            edit->setText(file.readAll());
-        file.open(QIODevice::ReadOnly);
+            edit->setReadOnly(true);
-        edit->setText(file.readAll());
+
-        edit->setReadOnly(true);
+            layout->addWidget(edit);
-        edit->show();
+
-    });
+            window->getLayoutContainer()->setLayout(layout);
+            window->show();
+        });
 
     form->addRow(a, b);
 }

src/widgets/settingspages/GeneralPageView.cpp

@@ -165,13 +165,11 @@ ColorButton *GeneralPageView::addColorButton(
     layout->addWidget(colorButton);
     this->addLayout(layout);
     QObject::connect(
-        colorButton, &ColorButton::clicked, [&setting, colorButton]() {
+        colorButton, &ColorButton::clicked, [this, &setting, colorButton]() {
-            auto dialog = new ColorPickerDialog(QColor(setting));
+            auto dialog = new ColorPickerDialog(QColor(setting), this);
             dialog->setAttribute(Qt::WA_DeleteOnClose);
             dialog->show();
-            dialog->closed.connect([&setting, colorButton, &dialog] {
+            dialog->closed.connect([&setting, colorButton](QColor selected) {
-                QColor selected = dialog->selectedColor();
-
                 if (selected.isValid())
                 {
                     setting = selected.name(QColor::HexArgb);

src/widgets/settingspages/HighlightingPage.cpp

@@ -244,9 +244,7 @@ void HighlightingPage::tableCellClicked(const QModelIndex &clicked,
         auto dialog = new ColorPickerDialog(initial, this);
         dialog->setAttribute(Qt::WA_DeleteOnClose);
         dialog->show();
-        dialog->closed.connect([=] {
+        dialog->closed.connect([=](QColor selected) {
-            QColor selected = dialog->selectedColor();
-
             if (selected.isValid())
             {
                 view->getModel()->setData(clicked, selected,

src/widgets/splits/Split.cpp

@@ -57,7 +57,7 @@ namespace {
                            const QString &title, const QString &description)
     {
         auto window =
-            new BaseWindow(BaseWindow::Flags::EnableCustomFrame, parent);
+            new BasePopup(BaseWindow::Flags::EnableCustomFrame, parent);
         window->setWindowTitle("Chatterino - " + title);
         window->setAttribute(Qt::WA_DeleteOnClose);
         auto layout = new QVBoxLayout();

src/widgets/splits/SplitHeader.cpp

@@ -240,8 +240,8 @@ void SplitHeader::initializeLayout()
                             if (getSettings()->moderationActions.empty())
                             {
                                 getApp()->windows->showSettingsDialog(
-                                    SettingsDialogPreference::
+                                    this, SettingsDialogPreference::
-                                        ModerationActions);
+                                              ModerationActions);
                                 this->split_->setModerationMode(true);
                             }
                             else
@@ -258,6 +258,7 @@ void SplitHeader::initializeLayout()
                         case Qt::RightButton:
                         case Qt::MiddleButton:
                             getApp()->windows->showSettingsDialog(
+                                this,
                                 SettingsDialogPreference::ModerationActions);
                             break;
                     }