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::Type::HorizontalContainer:
-        case SplitNode::VerticalContainer: {
+        case SplitNode::Type::VerticalContainer: {
-            obj.insert("type", node->getType() == SplitNode::HorizontalContainer
+            obj.insert("type",
-                                   ? "horizontal"
+                       node->getType() == SplitNode::Type::HorizontalContainer
-                                   : "vertical");
+                           ? "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";
-        Notebook *notebook = dynamic_cast<Notebook *>(this->parentWidget());
+        return;
-
-        if (notebook != nuuls)
-        {
-            SplitContainer *page = new SplitContainer(notebook);
-            auto *tab = notebook->addPage(page);
-            page->setTab(tab);
-
-            SplitContainer::draggingSplit->setParent(page);
-            page->appendSplit(SplitContainer::draggingSplit);
-        }
     }
+
+    auto *notebook = dynamic_cast<Notebook *>(this->parentWidget());
+    if (!notebook)
+    {
+        qCDebug(chatterinoWidget) << "Dropped a split onto a notebook button "
+                                     "without a parent notebook";
+        return;
+    }
+
+    event->acceptProposedAction();
+
+    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;
+        qCWarning(chatterinoWidget)
-        SplitContainer::draggingSplit = this;
+            << "Attempted to initiate split drag without a container parent";
-
+        return;
-        auto originalLocation = container->releaseSplit(this);
-        auto drag = new QDrag(this);
-        auto mimeData = new QMimeData;
-
-        mimeData->setData("chatterino/split", "xD");
-        drag->setMimeData(mimeData);
-
-        if (drag->exec(Qt::MoveAction) == Qt::IgnoreAction)
-        {
-            container->insertSplit(this, originalLocation);
-        }
-
-        SplitContainer::isDraggingSplit = false;
     }
+
+    startDraggingSplit();
+
+    auto originalLocation = container->releaseSplit(this);
+    auto drag = new QDrag(this);
+    auto mimeData = new QMimeData;
+
+    mimeData->setData("chatterino/split", "xD");
+    drag->setMimeData(mimeData);
+
+    // 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)

src/widgets/splits/SplitContainer.cpp

@@ -2,38 +2,30 @@
 
 #include "Application.hpp"
 #include "common/Common.hpp"
+#include "common/QLogging.hpp"
 #include "debug/AssertInGuiThread.hpp"
 #include "singletons/Fonts.hpp"
 #include "singletons/Theme.hpp"
 #include "singletons/WindowManager.hpp"
-#include "util/Helpers.hpp"
-#include "util/LayoutCreator.hpp"
 #include "widgets/helper/ChannelView.hpp"
 #include "widgets/helper/NotebookTab.hpp"
 #include "widgets/Notebook.hpp"
 #include "widgets/splits/ClosedSplits.hpp"
+#include "widgets/splits/DraggedSplit.hpp"
 #include "widgets/splits/Split.hpp"
 #include "widgets/Window.hpp"
 
 #include <boost/foreach.hpp>
 #include <QApplication>
-#include <QDebug>
-#include <QHBoxLayout>
 #include <QJsonArray>
 #include <QJsonObject>
 #include <QMimeData>
-#include <QObject>
 #include <QPainter>
-#include <QVBoxLayout>
-#include <QWidget>
 
 #include <algorithm>
 
 namespace chatterino {
 
-bool SplitContainer::isDraggingSplit = false;
-Split *SplitContainer::draggingSplit = nullptr;
-
 SplitContainer::SplitContainer(Notebook *parent)
     : BaseWidget(parent)
     , overlay_(this)
@@ -91,9 +83,9 @@ NotebookTab *SplitContainer::getTab() const
     return this->tab_;
 }
 
-void SplitContainer::setTab(NotebookTab *_tab)
+void SplitContainer::setTab(NotebookTab *tab)
 {
-    this->tab_ = _tab;
+    this->tab_ = tab;
 
     this->tab_->page = this;
 
@@ -120,8 +112,8 @@ Split *SplitContainer::appendNewSplit(bool openChannelNameDialog)
 {
     assertInGuiThread();
 
-    Split *split = new Split(this);
+    auto *split = new Split(this);
-    this->appendSplit(split);
+    this->insertSplit(split);
 
     if (openChannelNameDialog)
     {
@@ -136,41 +128,46 @@ Split *SplitContainer::appendNewSplit(bool openChannelNameDialog)
     return split;
 }
 
-void SplitContainer::appendSplit(Split *split)
+void SplitContainer::insertSplit(Split *split, InsertOptions &&options)
-{
-    this->insertSplit(split, Direction::Right);
-}
-
-void SplitContainer::insertSplit(Split *split, const Position &position)
-{
-    this->insertSplit(split, position.direction_,
-                      reinterpret_cast<Node *>(position.relativeNode_));
-}
-
-void SplitContainer::insertSplit(Split *split, Direction direction,
-                                 Split *relativeTo)
-{
-    Node *node = this->baseNode_.findNodeContainingSplit(relativeTo);
-    assert(node != nullptr);
-
-    this->insertSplit(split, direction, node);
-}
-
-void SplitContainer::insertSplit(Split *split, Direction direction,
-                                 Node *relativeTo)
 {
     // Queue up save because: Split added
     getApp()->windows->queueSave();
 
     assertInGuiThread();
 
+    if (options.position)
+    {
+        // options.position must not be set together with any other options
+        assert(!options.relativeSplit);
+        assert(!options.relativeNode);
+        assert(!options.direction.has_value());
+
+        options.relativeNode = options.position->relativeNode_;
+        options.direction = options.position->direction_;
+    }
+
+    if (options.relativeSplit)
+    {
+        // options.relativeNode must not be set together with relativeSplit
+        assert(!options.relativeNode);
+
+        Node *node =
+            this->baseNode_.findNodeContainingSplit(options.relativeSplit);
+        assert(node != nullptr);
+
+        options.relativeNode = node;
+    }
+
+    auto *relativeTo = options.relativeNode;
+    const auto direction = options.direction.value_or(Direction::Right);
+
     if (relativeTo == nullptr)
     {
-        if (this->baseNode_.type_ == Node::EmptyRoot)
+        if (this->baseNode_.type_ == Node::Type::EmptyRoot)
         {
             this->baseNode_.setSplit(split);
         }
-        else if (this->baseNode_.type_ == Node::_Split)
+        else if (this->baseNode_.type_ == Node::Type::Split)
         {
             this->baseNode_.nestSplitIntoCollection(split, direction);
         }
@@ -254,7 +251,7 @@ void SplitContainer::addSplit(Split *split)
                 case Split::Action::Delete: {
                     this->deleteSplit(split);
                     auto *tab = this->getTab();
-                    tab->connect(tab, &QWidget::destroyed, [tab]() mutable {
+                    QObject::connect(tab, &QWidget::destroyed, [tab]() mutable {
                         ClosedSplits::invalidateTab(tab);
                     });
                     ClosedSplits::push({split->getChannel()->getName(),
@@ -277,10 +274,14 @@ void SplitContainer::addSplit(Split *split)
             }
         });
 
-    conns.managedConnect(split->insertSplitRequested, [this](int dir,
+    conns.managedConnect(
-                                                             Split *parent) {
+        split->insertSplitRequested, [this](int dir, Split *parent) {
-        this->insertSplit(new Split(this), static_cast<Direction>(dir), parent);
+            this->insertSplit(new Split(this),
-    });
+                              {
+                                  .relativeSplit = parent,
+                                  .direction = static_cast<Direction>(dir),
+                              });
+        });
 
     this->layout();
 }
@@ -325,7 +326,7 @@ SplitContainer::Position SplitContainer::releaseSplit(Split *split)
     split->setParent(nullptr);
     Position position = node->releaseSplit();
     this->layout();
-    if (splits_.size() == 0)
+    if (splits_.empty())
     {
         this->setSelected(nullptr);
         this->setCursor(Qt::PointingHandCursor);
@@ -414,13 +415,13 @@ void SplitContainer::focusSplitRecursive(Node *node)
 {
     switch (node->type_)
     {
-        case Node::_Split: {
+        case Node::Type::Split: {
             node->split_->setFocus(Qt::FocusReason::OtherFocusReason);
         }
         break;
 
-        case Node::HorizontalContainer:
+        case Node::Type::HorizontalContainer:
-        case Node::VerticalContainer: {
+        case Node::Type::VerticalContainer: {
             auto &children = node->children_;
 
             auto it = std::find_if(
@@ -447,15 +448,19 @@ Split *SplitContainer::getTopRightSplit(Node &node)
 {
     switch (node.getType())
     {
-        case Node::_Split:
+        case Node::Type::Split:
             return node.getSplit();
-        case Node::VerticalContainer:
+        case Node::Type::VerticalContainer:
             if (!node.getChildren().empty())
+            {
                 return getTopRightSplit(*node.getChildren().front());
+            }
             break;
-        case Node::HorizontalContainer:
+        case Node::Type::HorizontalContainer:
             if (!node.getChildren().empty())
+            {
                 return getTopRightSplit(*node.getChildren().back());
+            }
             break;
         default:;
     }
@@ -470,23 +475,28 @@ void SplitContainer::layout()
     }
 
     // update top right split
-    auto topRight = this->getTopRightSplit(this->baseNode_);
+    auto *topRight = this->getTopRightSplit(this->baseNode_);
     if (this->topRight_)
+    {
         this->topRight_->setIsTopRightSplit(false);
+    }
     this->topRight_ = topRight;
     if (topRight)
+    {
         this->topRight_->setIsTopRightSplit(true);
+    }
 
     // layout
     this->baseNode_.geometry_ = this->rect().adjusted(-1, -1, 0, 0);
 
-    std::vector<DropRect> _dropRects;
+    std::vector<DropRect> dropRects;
-    std::vector<ResizeRect> _resizeRects;
+    std::vector<ResizeRect> resizeRects;
-    this->baseNode_.layout(
-        Split::modifierStatus == showAddSplitRegions || this->isDragging_,
-        this->scale(), _dropRects, _resizeRects);
 
-    this->dropRects_ = _dropRects;
+    const bool addSpacing =
+        Split::modifierStatus == showAddSplitRegions || this->isDragging_;
+    this->baseNode_.layout(addSpacing, this->scale(), dropRects, resizeRects);
+
+    this->dropRects_ = dropRects;
 
     for (Split *split : this->splits_)
     {
@@ -495,52 +505,51 @@ void SplitContainer::layout()
         Node *node = this->baseNode_.findNodeContainingSplit(split);
 
         // left
-        _dropRects.push_back(
+        dropRects.emplace_back(
-            DropRect(QRect(g.left(), g.top(), g.width() / 3, g.height()),
+            QRect(g.left(), g.top(), g.width() / 3, g.height()),
-                     Position(node, Direction::Left)));
+            Position(node, Direction::Left));
         // right
-        _dropRects.push_back(DropRect(QRect(g.right() - g.width() / 3, g.top(),
+        dropRects.emplace_back(QRect(g.right() - g.width() / 3, g.top(),
-                                            g.width() / 3, g.height()),
+                                     g.width() / 3, g.height()),
-                                      Position(node, Direction::Right)));
+                               Position(node, Direction::Right));
 
         // top
-        _dropRects.push_back(
+        dropRects.emplace_back(
-            DropRect(QRect(g.left(), g.top(), g.width(), g.height() / 2),
+            QRect(g.left(), g.top(), g.width(), g.height() / 2),
-                     Position(node, Direction::Above)));
+            Position(node, Direction::Above));
         // bottom
-        _dropRects.push_back(
+        dropRects.emplace_back(QRect(g.left(), g.bottom() - g.height() / 2,
-            DropRect(QRect(g.left(), g.bottom() - g.height() / 2, g.width(),
+                                     g.width(), g.height() / 2),
-                           g.height() / 2),
+                               Position(node, Direction::Below));
-                     Position(node, Direction::Below)));
     }
 
     if (this->splits_.empty())
     {
         QRect g = this->rect();
-        _dropRects.push_back(
+        dropRects.emplace_back(
-            DropRect(QRect(g.left(), g.top(), g.width() - 1, g.height() - 1),
+            QRect(g.left(), g.top(), g.width() - 1, g.height() - 1),
-                     Position(nullptr, Direction::Below)));
+            Position(nullptr, Direction::Below));
     }
 
-    this->overlay_.setRects(std::move(_dropRects));
+    this->overlay_.setRects(std::move(dropRects));
 
     // handle resizeHandles
-    if (this->resizeHandles_.size() < _resizeRects.size())
+    if (this->resizeHandles_.size() < resizeRects.size())
     {
-        while (this->resizeHandles_.size() < _resizeRects.size())
+        while (this->resizeHandles_.size() < resizeRects.size())
         {
             this->resizeHandles_.push_back(
                 std::make_unique<ResizeHandle>(this));
         }
     }
-    else if (this->resizeHandles_.size() > _resizeRects.size())
+    else if (this->resizeHandles_.size() > resizeRects.size())
     {
-        this->resizeHandles_.resize(_resizeRects.size());
+        this->resizeHandles_.resize(resizeRects.size());
     }
 
     {
         size_t i = 0;
-        for (ResizeRect &resizeRect : _resizeRects)
+        for (ResizeRect &resizeRect : resizeRects)
         {
             ResizeHandle *handle = this->resizeHandles_[i].get();
             handle->setGeometry(resizeRect.rect);
@@ -572,7 +581,7 @@ void SplitContainer::mouseReleaseEvent(QMouseEvent *event)
 {
     if (event->button() == Qt::LeftButton)
     {
-        if (this->splits_.size() == 0)
+        if (this->splits_.empty())
         {
             // "Add Chat" was clicked
             this->appendNewSplit(true);
@@ -589,17 +598,17 @@ void SplitContainer::mouseReleaseEvent(QMouseEvent *event)
                              });
             if (it != this->dropRects_.end())
             {
-                this->insertSplit(new Split(this), it->position);
+                this->insertSplit(new Split(this), {.position = it->position});
             }
         }
     }
 }
 
-void SplitContainer::paintEvent(QPaintEvent *)
+void SplitContainer::paintEvent(QPaintEvent * /*event*/)
 {
     QPainter painter(this);
 
-    if (this->splits_.size() == 0)
+    if (this->splits_.empty())
     {
         painter.fillRect(rect(), this->theme->splits.background);
 
@@ -611,7 +620,7 @@ void SplitContainer::paintEvent(QPaintEvent *)
 
         QString text = "Click to add a split";
 
-        Notebook *notebook = dynamic_cast<Notebook *>(this->parentWidget());
+        auto *notebook = dynamic_cast<Notebook *>(this->parentWidget());
 
         if (notebook != nullptr)
         {
@@ -690,10 +699,14 @@ void SplitContainer::paintEvent(QPaintEvent *)
 void SplitContainer::dragEnterEvent(QDragEnterEvent *event)
 {
     if (!event->mimeData()->hasFormat("chatterino/split"))
+    {
         return;
+    }
 
-    if (!SplitContainer::isDraggingSplit)
+    if (!isDraggingSplit())
+    {
         return;
+    }
 
     this->isDragging_ = true;
     this->layout();
@@ -714,13 +727,13 @@ void SplitContainer::mouseMoveEvent(QMouseEvent *event)
     this->update();
 }
 
-void SplitContainer::leaveEvent(QEvent *)
+void SplitContainer::leaveEvent(QEvent * /*event*/)
 {
     this->mouseOverPoint_ = QPoint(-10000, -10000);
     this->update();
 }
 
-void SplitContainer::focusInEvent(QFocusEvent *)
+void SplitContainer::focusInEvent(QFocusEvent * /*event*/)
 {
     if (this->baseNode_.findNodeContainingSplit(this->selected_) != nullptr)
     {
@@ -740,12 +753,7 @@ void SplitContainer::refreshTab()
     this->refreshTabLiveStatus();
 }
 
-int SplitContainer::getSplitCount()
+std::vector<Split *> SplitContainer::getSplits() const
-{
-    return this->splits_.size();
-}
-
-const std::vector<Split *> SplitContainer::getSplits() const
 {
     return this->splits_;
 }
@@ -757,7 +765,7 @@ SplitContainer::Node *SplitContainer::getBaseNode()
 
 void SplitContainer::applyFromDescriptor(const NodeDescriptor &rootNode)
 {
-    assert(this->baseNode_.type_ == Node::EmptyRoot);
+    assert(this->baseNode_.type_ == Node::Type::EmptyRoot);
 
     this->disableLayouting_ = true;
     this->applyFromDescriptorRecursively(rootNode, &this->baseNode_);
@@ -768,7 +776,7 @@ void SplitContainer::applyFromDescriptor(const NodeDescriptor &rootNode)
 void SplitContainer::popup()
 {
     Window &window = getApp()->windows->createWindow(WindowType::Popup);
-    auto popupContainer = window.getNotebook().getOrAddSelectedPage();
+    auto *popupContainer = window.getNotebook().getOrAddSelectedPage();
 
     QJsonObject encodedTab;
     WindowManager::encodeTab(this, true, encodedTab);
@@ -793,26 +801,33 @@ void SplitContainer::popup()
 }
 
 void SplitContainer::applyFromDescriptorRecursively(
-    const NodeDescriptor &rootNode, Node *node)
+    const NodeDescriptor &rootNode, Node *baseNode)
 {
     if (std::holds_alternative<SplitNodeDescriptor>(rootNode))
     {
-        auto *n = std::get_if<SplitNodeDescriptor>(&rootNode);
+        // This is a leaf, no further recursion happens from here
+
+        const auto *n = std::get_if<SplitNodeDescriptor>(&rootNode);
         if (!n)
         {
             return;
         }
         const auto &splitNode = *n;
+
         auto *split = new Split(this);
         split->setChannel(WindowManager::decodeChannel(splitNode));
         split->setModerationMode(splitNode.moderationMode_);
         split->setFilters(splitNode.filters_);
 
-        this->appendSplit(split);
+        this->insertSplit(split);
+
+        return;
     }
-    else if (std::holds_alternative<ContainerNodeDescriptor>(rootNode))
+
+    if (std::holds_alternative<ContainerNodeDescriptor>(rootNode))
     {
-        auto *n = std::get_if<ContainerNodeDescriptor>(&rootNode);
+        // This is a branch, it will contain one or more splits/containers
+        const auto *n = std::get_if<ContainerNodeDescriptor>(&rootNode);
         if (!n)
         {
             return;
@@ -821,14 +836,14 @@ void SplitContainer::applyFromDescriptorRecursively(
 
         bool vertical = containerNode.vertical_;
 
-        node->type_ =
+        baseNode->type_ = vertical ? Node::Type::VerticalContainer
-            vertical ? Node::VerticalContainer : Node::HorizontalContainer;
+                                   : Node::Type::HorizontalContainer;
 
         for (const auto &item : containerNode.items_)
         {
             if (std::holds_alternative<SplitNodeDescriptor>(item))
             {
-                auto *n = std::get_if<SplitNodeDescriptor>(&item);
+                const auto *n = std::get_if<SplitNodeDescriptor>(&item);
                 if (!n)
                 {
                     return;
@@ -839,30 +854,30 @@ void SplitContainer::applyFromDescriptorRecursively(
                 split->setModerationMode(splitNode.moderationMode_);
                 split->setFilters(splitNode.filters_);
 
-                Node *_node = new Node();
+                auto *node = new Node();
-                _node->parent_ = node;
+                node->parent_ = baseNode;
-                _node->split_ = split;
+                node->split_ = split;
-                _node->type_ = Node::_Split;
+                node->type_ = Node::Type::Split;
 
-                _node->flexH_ = splitNode.flexH_;
+                node->flexH_ = splitNode.flexH_;
-                _node->flexV_ = splitNode.flexV_;
+                node->flexV_ = splitNode.flexV_;
-                node->children_.emplace_back(_node);
+                baseNode->children_.emplace_back(node);
 
                 this->addSplit(split);
             }
             else
             {
-                Node *_node = new Node();
+                auto *node = new Node();
-                _node->parent_ = node;
+                node->parent_ = baseNode;
 
-                if (auto *n = std::get_if<ContainerNodeDescriptor>(&item))
+                if (const auto *n = std::get_if<ContainerNodeDescriptor>(&item))
                 {
-                    _node->flexH_ = n->flexH_;
+                    node->flexH_ = n->flexH_;
-                    _node->flexV_ = n->flexV_;
+                    node->flexV_ = n->flexV_;
                 }
 
-                node->children_.emplace_back(_node);
+                baseNode->children_.emplace_back(node);
-                this->applyFromDescriptorRecursively(item, _node);
+                this->applyFromDescriptorRecursively(item, node);
             }
         }
     }
@@ -928,26 +943,26 @@ void SplitContainer::refreshTabLiveStatus()
 // Node
 //
 
-SplitContainer::Node::Type SplitContainer::Node::getType()
+SplitContainer::Node::Type SplitContainer::Node::getType() const
 {
     return this->type_;
 }
-Split *SplitContainer::Node::getSplit()
+Split *SplitContainer::Node::getSplit() const
 {
     return this->split_;
 }
 
-SplitContainer::Node *SplitContainer::Node::getParent()
+SplitContainer::Node *SplitContainer::Node::getParent() const
 {
     return this->parent_;
 }
 
-qreal SplitContainer::Node::getHorizontalFlex()
+qreal SplitContainer::Node::getHorizontalFlex() const
 {
     return this->flexH_;
 }
 
-qreal SplitContainer::Node::getVerticalFlex()
+qreal SplitContainer::Node::getVerticalFlex() const
 {
     return this->flexV_;
 }
@@ -966,7 +981,7 @@ SplitContainer::Node::Node()
 }
 
 SplitContainer::Node::Node(Split *_split, Node *_parent)
-    : type_(Type::_Split)
+    : type_(Type::Split)
     , split_(_split)
     , parent_(_parent)
 {
@@ -988,7 +1003,7 @@ bool SplitContainer::Node::isOrContainsNode(SplitContainer::Node *_node)
 SplitContainer::Node *SplitContainer::Node::findNodeContainingSplit(
     Split *_split)
 {
-    if (this->type_ == Type::_Split && this->split_ == _split)
+    if (this->type_ == Type::Split && this->split_ == _split)
     {
         return this;
     }
@@ -1012,19 +1027,19 @@ void SplitContainer::Node::insertSplitRelative(Split *_split,
     {
         switch (this->type_)
         {
-            case Node::EmptyRoot: {
+            case Node::Type::EmptyRoot: {
                 this->setSplit(_split);
             }
             break;
-            case Node::_Split: {
+            case Node::Type::Split: {
                 this->nestSplitIntoCollection(_split, _direction);
             }
             break;
-            case Node::HorizontalContainer: {
+            case Node::Type::HorizontalContainer: {
                 this->nestSplitIntoCollection(_split, _direction);
             }
             break;
-            case Node::VerticalContainer: {
+            case Node::Type::VerticalContainer: {
                 this->nestSplitIntoCollection(_split, _direction);
             }
             break;
@@ -1107,15 +1122,15 @@ void SplitContainer::Node::insertNextToThis(Split *_split, Direction _direction)
 void SplitContainer::Node::setSplit(Split *_split)
 {
     assert(this->split_ == nullptr);
-    assert(this->children_.size() == 0);
+    assert(this->children_.empty());
 
     this->split_ = _split;
-    this->type_ = Type::_Split;
+    this->type_ = Type::Split;
 }
 
 SplitContainer::Position SplitContainer::Node::releaseSplit()
 {
-    assert(this->type_ == Type::_Split);
+    assert(this->type_ == Type::Split);
 
     if (parent_ == nullptr)
     {
@@ -1127,69 +1142,66 @@ SplitContainer::Position SplitContainer::Node::releaseSplit()
         pos.direction_ = Direction::Right;
         return pos;
     }
-    else
+
+    auto &siblings = this->parent_->children_;
+
+    auto it = std::find_if(begin(siblings), end(siblings), [this](auto &node) {
+        return this == node.get();
+    });
+    assert(it != siblings.end());
+
+    Position position;
+    if (siblings.size() == 2)
     {
-        auto &siblings = this->parent_->children_;
+        // delete this and move split to parent
-
+        position.relativeNode_ = this->parent_;
-        auto it =
+        if (this->parent_->type_ == Type::VerticalContainer)
-            std::find_if(begin(siblings), end(siblings), [this](auto &node) {
-                return this == node.get();
-            });
-        assert(it != siblings.end());
-
-        Position position;
-        if (siblings.size() == 2)
         {
-            // delete this and move split to parent
+            position.direction_ =
-            position.relativeNode_ = this->parent_;
+                siblings.begin() == it ? Direction::Above : Direction::Below;
-            if (this->parent_->type_ == Type::VerticalContainer)
-            {
-                position.direction_ = siblings.begin() == it ? Direction::Above
-                                                             : Direction::Below;
-            }
-            else
-            {
-                position.direction_ =
-                    siblings.begin() == it ? Direction::Left : Direction::Right;
-            }
-
-            Node *_parent = this->parent_;
-            siblings.erase(it);
-            std::unique_ptr<Node> &sibling = siblings.front();
-            _parent->type_ = sibling->type_;
-            _parent->split_ = sibling->split_;
-            std::vector<std::unique_ptr<Node>> nodes =
-                std::move(sibling->children_);
-            for (auto &node : nodes)
-            {
-                node->parent_ = _parent;
-            }
-            _parent->children_ = std::move(nodes);
         }
         else
         {
-            if (this == siblings.back().get())
+            position.direction_ =
-            {
+                siblings.begin() == it ? Direction::Left : Direction::Right;
-                position.direction_ =
-                    this->parent_->type_ == Type::VerticalContainer
-                        ? Direction::Below
-                        : Direction::Right;
-                siblings.erase(it);
-                position.relativeNode_ = siblings.back().get();
-            }
-            else
-            {
-                position.relativeNode_ = (it + 1)->get();
-                position.direction_ =
-                    this->parent_->type_ == Type::VerticalContainer
-                        ? Direction::Above
-                        : Direction::Left;
-                siblings.erase(it);
-            }
         }
 
-        return position;
+        auto *parent = this->parent_;
+        siblings.erase(it);
+        std::unique_ptr<Node> &sibling = siblings.front();
+        parent->type_ = sibling->type_;
+        parent->split_ = sibling->split_;
+        std::vector<std::unique_ptr<Node>> nodes =
+            std::move(sibling->children_);
+        for (auto &node : nodes)
+        {
+            node->parent_ = parent;
+        }
+        parent->children_ = std::move(nodes);
     }
+    else
+    {
+        if (this == siblings.back().get())
+        {
+            position.direction_ =
+                this->parent_->type_ == Type::VerticalContainer
+                    ? Direction::Below
+                    : Direction::Right;
+            siblings.erase(it);
+            position.relativeNode_ = siblings.back().get();
+        }
+        else
+        {
+            position.relativeNode_ = (it + 1)->get();
+            position.direction_ =
+                this->parent_->type_ == Type::VerticalContainer
+                    ? Direction::Above
+                    : Direction::Left;
+            siblings.erase(it);
+        }
+    }
+
+    return position;
 }
 
 qreal SplitContainer::Node::getFlex(bool isVertical)
@@ -1217,26 +1229,23 @@ void SplitContainer::Node::layout(bool addSpacing, float _scale,
 {
     for (std::unique_ptr<Node> &node : this->children_)
     {
-        if (node->flexH_ <= 0)
+        node->clamp();
-            node->flexH_ = 0;
-        if (node->flexV_ <= 0)
-            node->flexV_ = 0;
     }
 
     switch (this->type_)
     {
-        case Node::_Split: {
+        case Node::Type::Split: {
             QRect rect = this->geometry_.toRect();
             this->split_->setGeometry(
                 rect.marginsRemoved(QMargins(1, 1, 0, 0)));
         }
         break;
-        case Node::VerticalContainer:
+        case Node::Type::VerticalContainer:
-        case Node::HorizontalContainer: {
+        case Node::Type::HorizontalContainer: {
-            bool isVertical = this->type_ == Node::VerticalContainer;
+            bool isVertical = this->type_ == Node::Type::VerticalContainer;
 
             // vars
-            qreal minSize = qreal(48 * _scale);
+            qreal minSize(48 * _scale);
 
             qreal totalFlex = std::max<qreal>(
                 0.0001, this->getChildrensTotalFlex(isVertical));
@@ -1370,6 +1379,12 @@ void SplitContainer::Node::layout(bool addSpacing, float _scale,
     }
 }
 
+void SplitContainer::Node::clamp()
+{
+    this->flexH_ = std::max(0.0, this->flexH_);
+    this->flexV_ = std::max(0.0, this->flexV_);
+}
+
 SplitContainer::Node::Type SplitContainer::Node::toContainerType(Direction _dir)
 {
     return _dir == Direction::Left || _dir == Direction::Right
@@ -1398,7 +1413,7 @@ void SplitContainer::DropOverlay::setRects(
 
 // pajlada::Signals::NoArgSignal dragEnded;
 
-void SplitContainer::DropOverlay::paintEvent(QPaintEvent *)
+void SplitContainer::DropOverlay::paintEvent(QPaintEvent * /*event*/)
 {
     QPainter painter(this);
 
@@ -1438,7 +1453,7 @@ void SplitContainer::DropOverlay::dragMoveEvent(QDragMoveEvent *event)
     this->update();
 }
 
-void SplitContainer::DropOverlay::dragLeaveEvent(QDragLeaveEvent *)
+void SplitContainer::DropOverlay::dragLeaveEvent(QDragLeaveEvent * /*event*/)
 {
     this->mouseOverPoint_ = QPoint(-10000, -10000);
     this->close();
@@ -1457,12 +1472,23 @@ void SplitContainer::DropOverlay::dropEvent(QDropEvent *event)
         }
     }
 
-    if (position != nullptr)
+    if (!position)
     {
-        this->parent_->insertSplit(SplitContainer::draggingSplit, *position);
+        qCDebug(chatterinoWidget) << "No valid drop rectangle under cursor";
-        event->acceptProposedAction();
+        return;
     }
 
+    auto *draggedSplit = dynamic_cast<Split *>(event->source());
+    if (!draggedSplit)
+    {
+        qCDebug(chatterinoWidget)
+            << "Dropped something that wasn't a split onto a split container";
+        return;
+    }
+
+    this->parent_->insertSplit(draggedSplit, {.position = *position});
+    event->acceptProposedAction();
+
     this->mouseOverPoint_ = QPoint(-10000, -10000);
     this->close();
     this->dragEnded.invoke();
@@ -1486,7 +1512,7 @@ SplitContainer::ResizeHandle::ResizeHandle(SplitContainer *_parent)
     this->hide();
 }
 
-void SplitContainer::ResizeHandle::paintEvent(QPaintEvent *)
+void SplitContainer::ResizeHandle::paintEvent(QPaintEvent * /*event*/)
 {
     QPainter painter(this);
     painter.setPen(QPen(getApp()->themes->splits.resizeHandle, 2));
@@ -1516,7 +1542,7 @@ void SplitContainer::ResizeHandle::mousePressEvent(QMouseEvent *event)
     }
 }
 
-void SplitContainer::ResizeHandle::mouseReleaseEvent(QMouseEvent *)
+void SplitContainer::ResizeHandle::mouseReleaseEvent(QMouseEvent * /*event*/)
 {
     this->isMouseDown_ = false;
 }
@@ -1531,7 +1557,7 @@ void SplitContainer::ResizeHandle::mouseMoveEvent(QMouseEvent *event)
     assert(node != nullptr);
     assert(node->parent_ != nullptr);
 
-    auto &siblings = node->parent_->getChildren();
+    const auto &siblings = node->parent_->getChildren();
     auto it = std::find_if(siblings.begin(), siblings.end(),
                            [this](const std::unique_ptr<Node> &n) {
                                return n.get() == this->node;
@@ -1591,7 +1617,7 @@ void SplitContainer::ResizeHandle::mouseDoubleClickEvent(QMouseEvent *event)
 
 void SplitContainer::ResizeHandle::resetFlex()
 {
-    for (auto &sibling : this->node->getParent()->getChildren())
+    for (const auto &sibling : this->node->getParent()->getChildren())
     {
         sibling->flexH_ = 1;
         sibling->flexV_ = 1;

src/widgets/splits/SplitContainer.hpp

@@ -6,14 +6,11 @@
 #include <pajlada/signals/signal.hpp>
 #include <pajlada/signals/signalholder.hpp>
 #include <QDragEnterEvent>
-#include <QHBoxLayout>
 #include <QRect>
-#include <QVBoxLayout>
-#include <QVector>
 #include <QWidget>
 
 #include <algorithm>
-#include <functional>
+#include <optional>
 #include <unordered_map>
 #include <vector>
 
@@ -38,6 +35,7 @@ public:
     struct Node;
 
     // fourtf: !!! preserve the order of left, up, right and down
+    // It's important to preserve since we cast from an int to this enum, so 0 = left, 1 = above etc
     enum Direction { Left, Above, Right, Below };
 
     struct Position final {
@@ -49,8 +47,8 @@ public:
         {
         }
 
-        Node *relativeNode_;
+        Node *relativeNode_{nullptr};
-        Direction direction_;
+        Direction direction_{Direction::Right};
 
         friend struct Node;
         friend class SplitContainer;
@@ -83,13 +81,18 @@ private:
 
 public:
     struct Node final {
-        enum Type { EmptyRoot, _Split, VerticalContainer, HorizontalContainer };
+        enum class Type {
+            EmptyRoot,
+            Split,
+            VerticalContainer,
+            HorizontalContainer,
+        };
 
-        Type getType();
+        Type getType() const;
-        Split *getSplit();
+        Split *getSplit() const;
-        Node *getParent();
+        Node *getParent() const;
-        qreal getHorizontalFlex();
+        qreal getHorizontalFlex() const;
-        qreal getVerticalFlex();
+        qreal getVerticalFlex() const;
         const std::vector<std::unique_ptr<Node>> &getChildren();
 
     private:
@@ -110,6 +113,9 @@ public:
                     std::vector<DropRect> &dropRects_,
                     std::vector<ResizeRect> &resizeRects);
 
+        // Clamps the flex values ensuring they're never below 0
+        void clamp();
+
         static Type toContainerType(Direction _dir);
 
         Type type_;
@@ -174,33 +180,48 @@ public:
     SplitContainer(Notebook *parent);
 
     Split *appendNewSplit(bool openChannelNameDialog);
-    void appendSplit(Split *split);
+
-    void insertSplit(Split *split, const Position &position);
+    struct InsertOptions {
-    void insertSplit(Split *split, Direction direction, Split *relativeTo);
+        /// Position must be set alone, as if it's set it will override direction & relativeNode with its underlying values
-    void insertSplit(Split *split, Direction direction,
+        std::optional<Position> position{};
-                     Node *relativeTo = nullptr);
+
+        /// Will be used to figure out the relative node, so relative node or position must not be set if using this
+        Split *relativeSplit{nullptr};
+
+        Node *relativeNode{nullptr};
+        std::optional<Direction> direction{};
+    };
+
+    // Insert split into the base node of this container
+    // Default values for each field must be specified due to these bugs:
+    //  - https://bugs.llvm.org/show_bug.cgi?id=36684
+    //  - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96645
+    void insertSplit(Split *split, InsertOptions &&options = InsertOptions{
+                                       .position = std::nullopt,
+                                       .relativeSplit = nullptr,
+                                       .relativeNode = nullptr,
+                                       .direction = std::nullopt,
+                                   });
+
+    // Returns a pointer to the selected split
     Split *getSelectedSplit() const;
     Position releaseSplit(Split *split);
     Position deleteSplit(Split *split);
 
     void selectNextSplit(Direction direction);
-    void setSelected(Split *selected_);
+    void setSelected(Split *split);
 
-    int getSplitCount();
+    std::vector<Split *> getSplits() const;
-    const std::vector<Split *> getSplits() const;
 
     void refreshTab();
 
     NotebookTab *getTab() const;
     Node *getBaseNode();
 
-    void setTab(NotebookTab *tab_);
+    void setTab(NotebookTab *tab);
     void hideResizeHandles();
     void resetMouseStatus();
 
-    static bool isDraggingSplit;
-    static Split *draggingSplit;
-
     void applyFromDescriptor(const NodeDescriptor &rootNode);
 
     void popup();
@@ -219,7 +240,7 @@ protected:
 
 private:
     void applyFromDescriptorRecursively(const NodeDescriptor &rootNode,
-                                        Node *node);
+                                        Node *baseNode);
 
     void layout();
     void selectSplitRecursive(Node *node, Direction direction);
@@ -233,19 +254,7 @@ private:
     void refreshTabTitle();
     void refreshTabLiveStatus();
 
-    struct DropRegion {
-        QRect rect;
-        std::pair<int, int> position;
-
-        DropRegion(QRect rect, std::pair<int, int> position)
-        {
-            this->rect = rect;
-            this->position = position;
-        }
-    };
-
     std::vector<DropRect> dropRects_;
-    std::vector<DropRegion> dropRegions_;
     DropOverlay overlay_;
     std::vector<std::unique_ptr<ResizeHandle>> resizeHandles_;
     QPoint mouseOverPoint_;
@@ -263,6 +272,7 @@ private:
 
     pajlada::Signals::SignalHolder signalHolder_;
 
+    // Specifies whether the user is currently dragging something over this container
     bool isDragging_ = false;
 };