SplitContainer refactor (#4261)

* Remove unused include util/Helpers.hpp

* SplitContainer::setTag fix parameter naming

* autofy/constify where possible

* More const auto ptr magicifying

* Make SplitNode::Type an enum class

* Move QuickSwitcherPopup includes from header to source file

* Remove unused DropRegion code

* use empty() instead of size() == 0

* Add curly braces everywhere

* Remove useless reinterpret_cast

It was casting Node* to Node*

* Clarify that the connect is QObject::connect

* SplitContainer::setSelected fix parameter naming

* Rename function variables to remove unneccesary underscore

Also move addSpacing parameter out of the layout function

* emplace_back where possible

* Name parameters

* Remove ineffective const from return type

* Make node getters const

* Flatten Node::releaseSplit

* Rename in-function variable to match code style

* [ACTUAL CODE CHANGE/MOVE] Move clamp logic to its own function

* name params

* applyFromDescriptorRecursively: rename node param to baseNode

* [ACTUAL CODE CHANGE/MOVE] Remove the many overloads for append/insertSplit

This utilizes the C++20 designed initializers aggregate initialization feature

* Remove unused includes

* [ACTUAL CODE CHANGE] Clean up dragging logic

There's no need to keep a pointer around to which split is being
dragged, it's already stored in the QDropEvent source()

* UNRELATED .clang-tidy: Only suggest UPPER_CASE for constant global variables

* Remove unused SplitContainer::getSplitCount function

* Use std::max in Node's clamp function

* Remove test code

* DraggedSplit.hpp: remove unused include

* Split `setDraggingSplit` into two functions, `startDraggingSplit` and `stopDraggingSplit`

.clang-tidy

@@ -48,7 +48,7 @@ CheckOptions:
     value:           _
   - key:             readability-identifier-naming.UnionCase
     value:           CamelCase
-  - key:             readability-identifier-naming.GlobalVariableCase
+  - key:             readability-identifier-naming.GlobalConstantCase
     value:           UPPER_CASE
   - key:             readability-identifier-naming.VariableCase
     value:           camelBack

src/CMakeLists.txt

@@ -525,6 +525,8 @@ set(SOURCE_FILES
 
         widgets/splits/ClosedSplits.cpp
         widgets/splits/ClosedSplits.hpp
+        widgets/splits/DraggedSplit.cpp
+        widgets/splits/DraggedSplit.hpp
         widgets/splits/InputCompletionItem.cpp
         widgets/splits/InputCompletionItem.hpp
         widgets/splits/InputCompletionPopup.cpp

src/singletons/WindowManager.cpp

@@ -555,7 +555,7 @@ void WindowManager::encodeNodeRecursively(SplitNode *node, QJsonObject &obj)
 {
     switch (node->getType())
     {
-        case SplitNode::_Split: {
+        case SplitNode::Type::Split: {
             obj.insert("type", "split");
             obj.insert("moderationMode", node->getSplit()->getModerationMode());
 
@@ -569,11 +569,12 @@ void WindowManager::encodeNodeRecursively(SplitNode *node, QJsonObject &obj)
             obj.insert("filters", filters);
         }
         break;
-        case SplitNode::HorizontalContainer:
-        case SplitNode::VerticalContainer: {
-            obj.insert("type", node->getType() == SplitNode::HorizontalContainer
-                                   ? "horizontal"
-                                   : "vertical");
+        case SplitNode::Type::HorizontalContainer:
+        case SplitNode::Type::VerticalContainer: {
+            obj.insert("type",
+                       node->getType() == SplitNode::Type::HorizontalContainer
+                           ? "horizontal"
+                           : "vertical");
 
             QJsonArray itemsArr;
             for (const std::unique_ptr<SplitNode> &n : node->getChildren())

src/widgets/Window.cpp

@@ -424,7 +424,7 @@ void Window::addShortcuts()
              split->setChannel(
                  getApp()->twitch->getOrAddChannel(si.channelName));
              split->setFilters(si.filters);
-             splitContainer->appendSplit(split);
+             splitContainer->insertSplit(split);
              splitContainer->setSelected(split);
              this->notebook_->select(splitContainer);
              return "";

src/widgets/dialogs/UserInfoPopup.cpp

@@ -313,7 +313,7 @@ UserInfoPopup::UserInfoPopup(bool closeAutomatically, QWidget *parent,
                                 SplitContainer *container = nb.addPage(true);
                                 Split *split = new Split(container);
                                 split->setChannel(channel);
-                                container->appendSplit(split);
+                                container->insertSplit(split);
                             });
                         menu->popup(QCursor::pos());
                         menu->raise();

src/widgets/dialogs/switcher/NewTabItem.cpp

@@ -26,7 +26,7 @@ void NewTabItem::action()
 
     Split *split = new Split(container);
     split->setChannel(getApp()->twitch->getOrAddChannel(this->channelName_));
-    container->appendSplit(split);
+    container->insertSplit(split);
 }
 
 void NewTabItem::paint(QPainter *painter, const QRect &rect) const

src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp

@@ -1,6 +1,7 @@
 #include "widgets/dialogs/switcher/QuickSwitcherPopup.hpp"
 
 #include "Application.hpp"
+#include "common/Channel.hpp"
 #include "singletons/Theme.hpp"
 #include "singletons/WindowManager.hpp"
 #include "util/LayoutCreator.hpp"
@@ -10,6 +11,8 @@
 #include "widgets/helper/NotebookTab.hpp"
 #include "widgets/listview/GenericListView.hpp"
 #include "widgets/Notebook.hpp"
+#include "widgets/splits/Split.hpp"
+#include "widgets/splits/SplitContainer.hpp"
 #include "widgets/Window.hpp"
 
 namespace chatterino {

src/widgets/dialogs/switcher/QuickSwitcherPopup.hpp

@@ -1,10 +1,7 @@
 #pragma once
 
-#include "common/Channel.hpp"
 #include "widgets/BasePopup.hpp"
 #include "widgets/dialogs/switcher/QuickSwitcherModel.hpp"
-#include "widgets/splits/Split.hpp"
-#include "widgets/splits/SplitContainer.hpp"
 
 #include <QLineEdit>
 

src/widgets/helper/NotebookButton.cpp

@@ -1,8 +1,10 @@
 #include "widgets/helper/NotebookButton.hpp"
 
+#include "common/QLogging.hpp"
 #include "singletons/Theme.hpp"
 #include "widgets/helper/Button.hpp"
 #include "widgets/Notebook.hpp"
+#include "widgets/splits/DraggedSplit.hpp"
 #include "widgets/splits/Split.hpp"
 #include "widgets/splits/SplitContainer.hpp"
 
@@ -67,7 +69,7 @@ void NotebookButton::paintEvent(QPaintEvent *event)
         case Plus: {
             painter.setPen([&] {
                 QColor tmp = foreground;
-                if (SplitContainer::isDraggingSplit)
+                if (isDraggingSplit())
                 {
                     tmp = this->theme->tabs.selected.line.regular;
                 }
@@ -181,22 +183,30 @@ void NotebookButton::dragLeaveEvent(QDragLeaveEvent *)
 
 void NotebookButton::dropEvent(QDropEvent *event)
 {
-    if (SplitContainer::isDraggingSplit)
+    auto *draggedSplit = dynamic_cast<Split *>(event->source());
+    if (!draggedSplit)
     {
-        event->acceptProposedAction();
+        qCDebug(chatterinoWidget)
+            << "Dropped something that wasn't a split onto a notebook button";
+        return;
+    }
 
-        Notebook *notebook = dynamic_cast<Notebook *>(this->parentWidget());
+    auto *notebook = dynamic_cast<Notebook *>(this->parentWidget());
+    if (!notebook)
+    {
+        qCDebug(chatterinoWidget) << "Dropped a split onto a notebook button "
+                                     "without a parent notebook";
+        return;
+    }
 
-        if (notebook != nuuls)
-        {
-            SplitContainer *page = new SplitContainer(notebook);
-            auto *tab = notebook->addPage(page);
-            page->setTab(tab);
+    event->acceptProposedAction();
 
-            SplitContainer::draggingSplit->setParent(page);
-            page->appendSplit(SplitContainer::draggingSplit);
-        }
-    }
+    auto *page = new SplitContainer(notebook);
+    auto *tab = notebook->addPage(page);
+    page->setTab(tab);
+
+    draggedSplit->setParent(page);
+    page->insertSplit(draggedSplit);
 }
 
 void NotebookButton::hideEvent(QHideEvent *)

src/widgets/helper/NotebookTab.cpp

@@ -12,6 +12,7 @@
 #include "util/Helpers.hpp"
 #include "widgets/dialogs/SettingsDialog.hpp"
 #include "widgets/Notebook.hpp"
+#include "widgets/splits/DraggedSplit.hpp"
 #include "widgets/splits/SplitContainer.hpp"
 
 #include <boost/bind/bind.hpp>
@@ -695,10 +696,15 @@ void NotebookTab::leaveEvent(QEvent *event)
 void NotebookTab::dragEnterEvent(QDragEnterEvent *event)
 {
     if (!event->mimeData()->hasFormat("chatterino/split"))
+    {
         return;
+    }
 
-    if (!SplitContainer::isDraggingSplit)
+    if (!isDraggingSplit())
+    {
+        // Ensure dragging a split from a different Chatterino instance doesn't switch tabs around
         return;
+    }
 
     if (this->notebook_->getAllowUserTabManagement())
     {

src/widgets/settingspages/GeneralPageView.hpp

@@ -12,6 +12,7 @@
 #include <QDebug>
 #include <QPushButton>
 #include <QSpinBox>
+#include <QVBoxLayout>
 
 class QScrollArea;
 

src/widgets/splits/DraggedSplit.cpp

@@ -0,0 +1,28 @@
+#include "widgets/splits/DraggedSplit.hpp"
+
+#include <cassert>
+
+namespace chatterino {
+
+static bool currentlyDraggingSplit = false;
+
+bool isDraggingSplit()
+{
+    return currentlyDraggingSplit;
+}
+
+void startDraggingSplit()
+{
+    assert(currentlyDraggingSplit == false);
+
+    currentlyDraggingSplit = true;
+}
+
+void stopDraggingSplit()
+{
+    assert(currentlyDraggingSplit == true);
+
+    currentlyDraggingSplit = false;
+}
+
+}  // namespace chatterino

src/widgets/splits/DraggedSplit.hpp

@@ -0,0 +1,17 @@
+#pragma once
+
+namespace chatterino {
+
+// Returns true if the user is currently dragging a split in this Chatterino instance
+// We need to keep track of this to ensure splits from other Chatterino instances aren't treated as memory we own
+[[nodiscard]] bool isDraggingSplit();
+
+// Set that a split is currently being dragged
+// Used by the Split::drag function when a drag is initiated
+void startDraggingSplit();
+
+// Set that a split is no longer being dragged
+// Used by the Split::drag function when a drag is finished
+void stopDraggingSplit();
+
+}  // namespace chatterino

src/widgets/splits/Split.cpp

@@ -34,6 +34,7 @@
 #include "widgets/helper/SearchPopup.hpp"
 #include "widgets/Notebook.hpp"
 #include "widgets/Scrollbar.hpp"
+#include "widgets/splits/DraggedSplit.hpp"
 #include "widgets/splits/SplitContainer.hpp"
 #include "widgets/splits/SplitHeader.hpp"
 #include "widgets/splits/SplitInput.hpp"
@@ -628,7 +629,7 @@ void Split::joinChannelInNewTab(ChannelPtr channel)
 
     Split *split = new Split(container);
     split->setChannel(channel);
-    container->appendSplit(split);
+    container->insertSplit(split);
 }
 
 void Split::openChannelInBrowserPlayer(ChannelPtr channel)
@@ -899,7 +900,7 @@ void Split::popup()
     split->setModerationMode(this->getModerationMode());
     split->setFilters(this->getFilters());
 
-    window.getNotebook().getOrAddSelectedPage()->appendSplit(split);
+    window.getNotebook().getOrAddSelectedPage()->insertSplit(split);
     window.show();
 }
 
@@ -1256,25 +1257,31 @@ static Iter select_randomly(Iter start, Iter end)
 
 void Split::drag()
 {
-    if (auto container = dynamic_cast<SplitContainer *>(this->parentWidget()))
+    auto *container = dynamic_cast<SplitContainer *>(this->parentWidget());
+    if (!container)
     {
-        SplitContainer::isDraggingSplit = true;
-        SplitContainer::draggingSplit = this;
+        qCWarning(chatterinoWidget)
+            << "Attempted to initiate split drag without a container parent";
+        return;
+    }
 
-        auto originalLocation = container->releaseSplit(this);
-        auto drag = new QDrag(this);
-        auto mimeData = new QMimeData;
+    startDraggingSplit();
 
-        mimeData->setData("chatterino/split", "xD");
-        drag->setMimeData(mimeData);
+    auto originalLocation = container->releaseSplit(this);
+    auto drag = new QDrag(this);
+    auto mimeData = new QMimeData;
 
-        if (drag->exec(Qt::MoveAction) == Qt::IgnoreAction)
-        {
-            container->insertSplit(this, originalLocation);
-        }
+    mimeData->setData("chatterino/split", "xD");
+    drag->setMimeData(mimeData);
 
-        SplitContainer::isDraggingSplit = false;
+    // drag->exec is a blocking action
+    if (drag->exec(Qt::MoveAction) == Qt::IgnoreAction)
+    {
+        // The split wasn't dropped in a valid spot, return it to its original position
+        container->insertSplit(this, {.position = originalLocation});
     }
+
+    stopDraggingSplit();
 }
 
 void Split::setInputReply(const std::shared_ptr<MessageThread> &reply)