Fix split focusing being broken in certain circumstances when the "Show input when it's empty" setting was disabled (#3838)

Co-authored-by: Kasia <zneix@zneix.eu>

CHANGELOG.md

@@ -38,6 +38,7 @@
 - Bugfix: Fixed links with no thumbnail having previous link's thumbnail. (#3720)
 - Bugfix: Add icon in the CMake macOS bundle. (#3832)
 - Bugfix: Adopt popup windows in order to force floating behavior on some window managers. (#3836)
+- Bugfix: Fix split focusing being broken in certain circumstances when the "Show input when it's empty" setting was disabled. (#3838)
 - Dev: Rewrite LimitedQueue (#3798)
 - Dev: Overhaul highlight system by moving all checks into a Controller allowing for easier tests. (#3399, #3801, #3835)
 - Dev: Use Game Name returned by Get Streams instead of querying it from the Get Games API. (#3662)

src/common/Channel.cpp

@@ -336,7 +336,7 @@ IndirectChannel::IndirectChannel(ChannelPtr channel, Channel::Type type)
 {
 }
 
-ChannelPtr IndirectChannel::get()
+ChannelPtr IndirectChannel::get() const
 {
     return data_->channel;
 }

src/common/Channel.hpp

@@ -125,7 +125,7 @@ public:
     IndirectChannel(ChannelPtr channel,
                     Channel::Type type = Channel::Type::Direct);
 
-    ChannelPtr get();
+    ChannelPtr get() const;
     void reset(ChannelPtr channel);
     pajlada::Signals::NoArgSignal &getChannelChanged();
     Channel::Type getType();

src/widgets/Notebook.cpp

@@ -784,7 +784,7 @@ void SplitNotebook::showEvent(QShowEvent *)
     {
         if (auto split = page->findChild<Split *>())
         {
-            split->giveFocus(Qt::OtherFocusReason);
+            split->setFocus(Qt::FocusReason::OtherFocusReason);
         }
     }
 }

src/widgets/helper/ChannelView.cpp

@@ -155,6 +155,11 @@ ChannelView::ChannelView(BaseWidget *parent)
     QObject::connect(&this->scrollTimer_, &QTimer::timeout, this,
                      &ChannelView::scrollUpdateRequested);
 
+    // TODO: Figure out if we need this, and if so, why
+    // StrongFocus means we can focus this event through clicking it
+    // and tabbing to it from another widget. I don't currently know
+    // of any place where you can, or where it would make sense,
+    // to tab to a ChannelVieChannelView
     this->setFocusPolicy(Qt::FocusPolicy::StrongFocus);
 }
 

src/widgets/helper/ResizingTextEdit.cpp

@@ -96,8 +96,10 @@ QString ResizingTextEdit::textUnderCursor(bool *hadSpace) const
     return lastWord;
 }
 
-bool ResizingTextEdit::eventFilter(QObject *, QEvent *event)
+bool ResizingTextEdit::eventFilter(QObject *obj, QEvent *event)
 {
+    (void)obj;  // unused
+
     // makes QShortcuts work in the ResizingTextEdit
     if (event->type() != QEvent::ShortcutOverride)
     {

src/widgets/helper/ResizingTextEdit.hpp

@@ -43,7 +43,8 @@ private:
     QCompleter *completer_ = nullptr;
     bool completionInProgress_ = false;
 
-    bool eventFilter(QObject *widget, QEvent *event) override;
+    bool eventFilter(QObject *obj, QEvent *event) override;
+
 private slots:
     void insertCompletion(const QString &completion);
 };

src/widgets/splits/Split.cpp

@@ -91,7 +91,8 @@ Split::Split(QWidget *parent)
 {
     this->setMouseTracking(true);
     this->view_->setPausable(true);
-    this->view_->setFocusPolicy(Qt::FocusPolicy::NoFocus);
+    this->view_->setFocusProxy(this->input_->ui_.textEdit);
+    this->setFocusProxy(this->input_->ui_.textEdit);
 
     this->vbox_->setSpacing(0);
     this->vbox_->setMargin(1);
@@ -112,9 +113,6 @@ Split::Split(QWidget *parent)
     });
     this->updateInputPlaceholder();
 
-    this->view_->mouseDown.connect([this](QMouseEvent *) {
-        this->giveFocus(Qt::MouseFocusReason);
-    });
     this->view_->selectionChanged.connect([this]() {
         if (view_->hasSelection())
         {
@@ -150,28 +148,33 @@ Split::Split(QWidget *parent)
     this->input_->textChanged.connect([=](const QString &newText) {
         if (getSettings()->showEmptyInput)
         {
+            // We always show the input regardless of the text, so we can early out here
             return;
         }
 
-        if (newText.length() == 0)
+        if (newText.isEmpty())
         {
             this->input_->hide();
         }
         else if (this->input_->isHidden())
         {
+            // Text updated and the input was previously hidden, show it
             this->input_->show();
         }
     });
 
     getSettings()->showEmptyInput.connect(
         [this](const bool &showEmptyInput, auto) {
-            if (!showEmptyInput && this->input_->getInputText().length() == 0)
+            if (showEmptyInput)
             {
-                this->input_->hide();
+                this->input_->show();
             }
             else
             {
-                this->input_->show();
+                if (this->input_->getInputText().isEmpty())
+                {
+                    this->input_->hide();
+                }
             }
         },
         this->signalHolder_);
@@ -207,9 +210,11 @@ Split::Split(QWidget *parent)
         });
 
     this->input_->ui_.textEdit->focused.connect([this] {
+        // Forward textEdit's focused event
         this->focused.invoke();
     });
     this->input_->ui_.textEdit->focusLost.connect([this] {
+        // Forward textEdit's focusLost event
         this->focusLost.invoke();
     });
     this->input_->ui_.textEdit->imagePasted.connect(
@@ -640,7 +645,7 @@ IndirectChannel Split::getIndirectChannel()
     return this->channel_;
 }
 
-ChannelPtr Split::getChannel()
+ChannelPtr Split::getChannel() const
 {
     return this->channel_.get();
 }
@@ -757,16 +762,6 @@ void Split::updateLastReadMessage()
     this->view_->updateLastReadMessage();
 }
 
-void Split::giveFocus(Qt::FocusReason reason)
-{
-    this->input_->ui_.textEdit->setFocus(reason);
-}
-
-bool Split::hasFocus() const
-{
-    return this->input_->ui_.textEdit->hasFocus();
-}
-
 void Split::paintEvent(QPaintEvent *)
 {
     // color the background of the chat
@@ -828,11 +823,6 @@ void Split::leaveEvent(QEvent *event)
     this->handleModifiers(QGuiApplication::queryKeyboardModifiers());
 }
 
-void Split::focusInEvent(QFocusEvent *event)
-{
-    this->giveFocus(event->reason());
-}
-
 void Split::handleModifiers(Qt::KeyboardModifiers modifiers)
 {
     if (modifierStatus != modifiers)
@@ -1278,3 +1268,31 @@ void Split::drag()
 }
 
 }  // namespace chatterino
+
+QDebug operator<<(QDebug dbg, const chatterino::Split &split)
+{
+    auto channel = split.getChannel();
+    if (channel)
+    {
+        dbg.nospace() << "Split(" << (void *)&split
+                      << ", channel:" << channel->getName() << ")";
+    }
+    else
+    {
+        dbg.nospace() << "Split(" << (void *)&split << ", no channel)";
+    }
+
+    return dbg;
+}
+
+QDebug operator<<(QDebug dbg, const chatterino::Split *split)
+{
+    if (split != nullptr)
+    {
+        return operator<<(dbg, *split);
+    }
+
+    dbg.nospace() << "Split(nullptr)";
+
+    return dbg;
+}

src/widgets/splits/Split.hpp

@@ -51,7 +51,7 @@ public:
     SplitInput &getInput();
 
     IndirectChannel getIndirectChannel();
-    ChannelPtr getChannel();
+    ChannelPtr getChannel() const;
     void setChannel(IndirectChannel newChannel);
 
     void setFilters(const QList<QUuid> ids);
@@ -64,8 +64,6 @@ public:
 
     void showChangeChannelPopup(const char *dialogTitle, bool empty,
                                 std::function<void(bool)> callback);
-    void giveFocus(Qt::FocusReason reason);
-    bool hasFocus() const;
     void updateGifEmotes();
     void updateLastReadMessage();
     void setIsTopRightSplit(bool value);
@@ -106,7 +104,6 @@ protected:
     void resizeEvent(QResizeEvent *event) override;
     void enterEvent(QEvent *event) override;
     void leaveEvent(QEvent *event) override;
-    void focusInEvent(QFocusEvent *event) override;
 
     void dragEnterEvent(QDragEnterEvent *event) override;
     void dropEvent(QDropEvent *event) override;
@@ -138,11 +135,11 @@ private:
     bool isMouseOver_{};
     bool isDragging_{};
 
-    QVBoxLayout *vbox_;
-    SplitHeader *header_;
-    ChannelView *view_;
-    SplitInput *input_;
-    SplitOverlay *overlay_;
+    QVBoxLayout *const vbox_;
+    SplitHeader *const header_;
+    ChannelView *const view_;
+    SplitInput *const input_;
+    SplitOverlay *const overlay_;
 
     NullablePtr<SelectChannelDialog> selectChannelDialog_;
 
@@ -179,3 +176,6 @@ public slots:
 };
 
 }  // namespace chatterino
+
+QDebug operator<<(QDebug dbg, const chatterino::Split &split);
+QDebug operator<<(QDebug dbg, const chatterino::Split *split);

src/widgets/splits/SplitContainer.cpp

@@ -206,7 +206,7 @@ void SplitContainer::addSplit(Split *split)
 
     split->setParent(this);
     split->show();
-    split->giveFocus(Qt::MouseFocusReason);
+    split->setFocus(Qt::FocusReason::MouseFocusReason);
     this->unsetCursor();
     this->splits_.push_back(split);
 
@@ -331,7 +331,7 @@ SplitContainer::Position SplitContainer::releaseSplit(Split *split)
     }
     else
     {
-        this->splits_.front()->giveFocus(Qt::MouseFocusReason);
+        this->splits_.front()->setFocus(Qt::FocusReason::MouseFocusReason);
     }
 
     this->refreshTab();
@@ -414,7 +414,7 @@ void SplitContainer::focusSplitRecursive(Node *node)
     switch (node->type_)
     {
         case Node::_Split: {
-            node->split_->giveFocus(Qt::OtherFocusReason);
+            node->split_->setFocus(Qt::FocusReason::OtherFocusReason);
         }
         break;
 
@@ -727,7 +727,7 @@ void SplitContainer::focusInEvent(QFocusEvent *)
         return;
     }
 
-    if (this->splits_.size() != 0)
+    if (!this->splits_.empty())
     {
         this->splits_.front()->setFocus();
     }

src/widgets/splits/SplitHeader.cpp

@@ -805,7 +805,7 @@ void SplitHeader::mousePressEvent(QMouseEvent *event)
     switch (event->button())
     {
         case Qt::LeftButton: {
-            this->split_->giveFocus(Qt::MouseFocusReason);
+            this->split_->setFocus(Qt::MouseFocusReason);
 
             this->dragging_ = true;
 
@@ -814,7 +814,7 @@ void SplitHeader::mousePressEvent(QMouseEvent *event)
         break;
 
         case Qt::RightButton: {
-            auto menu = this->createMainMenu().release();
+            auto *menu = this->createMainMenu().release();
             menu->setAttribute(Qt::WA_DeleteOnClose);
             menu->popup(this->mapToGlobal(event->pos() + QPoint(0, 4)));
         }
@@ -824,6 +824,10 @@ void SplitHeader::mousePressEvent(QMouseEvent *event)
             this->split_->openInBrowser();
         }
         break;
+
+        default: {
+        }
+        break;
     }
 
     this->doubleClicked_ = false;

src/widgets/splits/SplitInput.cpp

@@ -133,7 +133,10 @@ void SplitInput::scaleChangedEvent(float scale)
     this->updateEmoteButton();
 
     // set maximum height
-    this->setMaximumHeight(int(150 * this->scale()));
+    if (!this->hidden)
+    {
+        this->setMaximumHeight(this->scaledMaxHeight());
+    }
     this->ui_.textEdit->setFont(
         getApp()->fonts->getFont(FontStyle::ChatMedium, this->scale()));
     this->ui_.textEditLength->setFont(
@@ -214,6 +217,11 @@ void SplitInput::openEmotePopup()
     this->emotePopup_->activateWindow();
 }
 
+int SplitInput::scaledMaxHeight() const
+{
+    return int(150 * this->scale());
+}
+
 void SplitInput::addShortcuts()
 {
     HotkeyController::HotkeyMap actions{
@@ -686,6 +694,35 @@ void SplitInput::insertText(const QString &text)
     this->ui_.textEdit->insertPlainText(text);
 }
 
+void SplitInput::hide()
+{
+    if (this->isHidden())
+    {
+        return;
+    }
+
+    this->hidden = true;
+    this->setMaximumHeight(0);
+    this->updateGeometry();
+}
+
+void SplitInput::show()
+{
+    if (!this->isHidden())
+    {
+        return;
+    }
+
+    this->hidden = false;
+    this->setMaximumHeight(this->scaledMaxHeight());
+    this->updateGeometry();
+}
+
+bool SplitInput::isHidden() const
+{
+    return this->hidden;
+}
+
 void SplitInput::editTextChanged()
 {
     auto app = getApp();
@@ -734,7 +771,7 @@ void SplitInput::editTextChanged()
     this->ui_.textEditLength->setText(labelText);
 }
 
-void SplitInput::paintEvent(QPaintEvent *)
+void SplitInput::paintEvent(QPaintEvent * /*event*/)
 {
     QPainter painter(this);
 
@@ -777,9 +814,4 @@ void SplitInput::resizeEvent(QResizeEvent *)
     }
 }
 
-void SplitInput::mousePressEvent(QMouseEvent *)
-{
-    this->split_->giveFocus(Qt::MouseFocusReason);
-}
-
 }  // namespace chatterino

src/widgets/splits/SplitInput.hpp

@@ -32,16 +32,37 @@ public:
     QString getInputText() const;
     void insertText(const QString &text);
 
+    /**
+     * @brief Hide the widget
+     *
+     * This is a no-op if the SplitInput is already hidden
+     **/
+    void hide();
+
+    /**
+     * @brief Show the widget
+     *
+     * This is a no-op if the SplitInput is already shown
+     **/
+    void show();
+
+    /**
+     * @brief Returns the hidden or shown state of the SplitInput
+     *
+     * Hidden in this context means "has 0 height", meaning it won't be visible
+     * but Qt still treats the widget as "technically visible" so we receive events
+     * as if the widget is visible
+     **/
+    bool isHidden() const;
+
     pajlada::Signals::Signal<const QString &> textChanged;
 
 protected:
-    virtual void scaleChangedEvent(float scale_) override;
-    virtual void themeChangedEvent() override;
+    void scaleChangedEvent(float scale_) override;
+    void themeChangedEvent() override;
 
-    virtual void paintEvent(QPaintEvent *) override;
-    virtual void resizeEvent(QResizeEvent *) override;
-
-    virtual void mousePressEvent(QMouseEvent *event) override;
+    void paintEvent(QPaintEvent * /*event*/) override;
+    void resizeEvent(QResizeEvent * /*event*/) override;
 
 private:
     void addShortcuts() override;
@@ -57,6 +78,10 @@ private:
     void insertCompletionText(const QString &text);
     void openEmotePopup();
 
+    // scaledMaxHeight returns the height in pixels that this widget can grow to
+    // This does not take hidden into account, so callers must take hidden into account themselves
+    int scaledMaxHeight() const;
+
     Split *const split_;
     QObjectRef<EmotePopup> emotePopup_;
     QObjectRef<InputCompletionPopup> inputCompletionPopup_;
@@ -74,6 +99,12 @@ private:
     QString currMsg_;
     int prevIndex_ = 0;
 
+    // Hidden denotes whether this split input should be hidden or not
+    // This is used instead of the regular QWidget::hide/show because
+    // focus events don't work as expected, so instead we use this bool and
+    // set the height of the split input to 0 if we're supposed to be hidden instead
+    bool hidden{false};
+
 private slots:
     void editTextChanged();