From 6599009e795639f1643baed63eee9ad42114495c Mon Sep 17 00:00:00 2001 From: pajlada Date: Sun, 26 Jun 2022 18:53:09 +0200 Subject: [PATCH] Fix split focusing being broken in certain circumstances when the "Show input when it's empty" setting was disabled (#3838) Co-authored-by: Kasia --- CHANGELOG.md | 1 + src/common/Channel.cpp | 2 +- src/common/Channel.hpp | 2 +- src/widgets/Notebook.cpp | 2 +- src/widgets/helper/ChannelView.cpp | 5 ++ src/widgets/helper/ResizingTextEdit.cpp | 4 +- src/widgets/helper/ResizingTextEdit.hpp | 3 +- src/widgets/splits/Split.cpp | 66 ++++++++++++++++--------- src/widgets/splits/Split.hpp | 18 +++---- src/widgets/splits/SplitContainer.cpp | 8 +-- src/widgets/splits/SplitHeader.cpp | 8 ++- src/widgets/splits/SplitInput.cpp | 46 ++++++++++++++--- src/widgets/splits/SplitInput.hpp | 43 +++++++++++++--- 13 files changed, 151 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f841ac855..c22fdf9e7 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/common/Channel.cpp b/src/common/Channel.cpp index b11e2cb31..fda32f4f4 100644 --- a/src/common/Channel.cpp +++ b/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; } diff --git a/src/common/Channel.hpp b/src/common/Channel.hpp index 6f60ddc71..0b7c81653 100644 --- a/src/common/Channel.hpp +++ b/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(); diff --git a/src/widgets/Notebook.cpp b/src/widgets/Notebook.cpp index f7c9ad414..4a6f1256a 100644 --- a/src/widgets/Notebook.cpp +++ b/src/widgets/Notebook.cpp @@ -784,7 +784,7 @@ void SplitNotebook::showEvent(QShowEvent *) { if (auto split = page->findChild()) { - split->giveFocus(Qt::OtherFocusReason); + split->setFocus(Qt::FocusReason::OtherFocusReason); } } } diff --git a/src/widgets/helper/ChannelView.cpp b/src/widgets/helper/ChannelView.cpp index 8fdc3ba92..96fef73cb 100644 --- a/src/widgets/helper/ChannelView.cpp +++ b/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); } diff --git a/src/widgets/helper/ResizingTextEdit.cpp b/src/widgets/helper/ResizingTextEdit.cpp index da718868d..63444f5bf 100644 --- a/src/widgets/helper/ResizingTextEdit.cpp +++ b/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) { diff --git a/src/widgets/helper/ResizingTextEdit.hpp b/src/widgets/helper/ResizingTextEdit.hpp index 1ee423f48..301c00a84 100644 --- a/src/widgets/helper/ResizingTextEdit.hpp +++ b/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); }; diff --git a/src/widgets/splits/Split.cpp b/src/widgets/splits/Split.cpp index dcc9691aa..ab5af96cb 100644 --- a/src/widgets/splits/Split.cpp +++ b/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; +} diff --git a/src/widgets/splits/Split.hpp b/src/widgets/splits/Split.hpp index 7a1eb350e..06d36fcd2 100644 --- a/src/widgets/splits/Split.hpp +++ b/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 ids); @@ -64,8 +64,6 @@ public: void showChangeChannelPopup(const char *dialogTitle, bool empty, std::function 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_; @@ -179,3 +176,6 @@ public slots: }; } // namespace chatterino + +QDebug operator<<(QDebug dbg, const chatterino::Split &split); +QDebug operator<<(QDebug dbg, const chatterino::Split *split); diff --git a/src/widgets/splits/SplitContainer.cpp b/src/widgets/splits/SplitContainer.cpp index 4bb4ca74f..42437b300 100644 --- a/src/widgets/splits/SplitContainer.cpp +++ b/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(); } diff --git a/src/widgets/splits/SplitHeader.cpp b/src/widgets/splits/SplitHeader.cpp index f62aaf33e..d8c02530b 100644 --- a/src/widgets/splits/SplitHeader.cpp +++ b/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; diff --git a/src/widgets/splits/SplitInput.cpp b/src/widgets/splits/SplitInput.cpp index 94669148e..0188f2ced 100644 --- a/src/widgets/splits/SplitInput.cpp +++ b/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 diff --git a/src/widgets/splits/SplitInput.hpp b/src/widgets/splits/SplitInput.hpp index 0518cc9c5..7296dc8b3 100644 --- a/src/widgets/splits/SplitInput.hpp +++ b/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 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_; QObjectRef 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();