From 1b9ee939bb919d710bf8417e07fed3ddf6e876e4 Mon Sep 17 00:00:00 2001 From: Daniel Sage <24928223+dnsge@users.noreply.github.com> Date: Sun, 10 Sep 2023 06:38:59 -0400 Subject: [PATCH] Select correct notebook tab after closing while filtered (#4770) --- CHANGELOG.md | 1 + src/widgets/Notebook.cpp | 117 ++++++++++++++++++++++++++++++--------- src/widgets/Notebook.hpp | 43 ++++++++++++++ 3 files changed, 136 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec4c175f6..c8a25d80a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Bugfix: Fixed a performance issue when displaying replies to certain messages. (#4807) - Bugfix: Fixed a data race when disconnecting from Twitch PubSub. (#4771) - Bugfix: Fixed `/shoutout` command not working with usernames starting with @'s (e.g. `/shoutout @forsen`). (#4800) +- Bugfix: Fixed selection of tabs after closing a tab when using "Live Tabs Only". (#4770) - Dev: Fixed UTF16 encoding of `modes` file for the installer. (#4791) - Dev: Temporarily disable High DPI scaling on Qt6 builds on Windows. (#4767) - Dev: Tests now run on Ubuntu 22.04 instead of 20.04 to loosen C++ restrictions in tests. (#4774) diff --git a/src/widgets/Notebook.cpp b/src/widgets/Notebook.cpp index 89f1e202e..3ff3648a7 100644 --- a/src/widgets/Notebook.cpp +++ b/src/widgets/Notebook.cpp @@ -101,35 +101,47 @@ void Notebook::removePage(QWidget *page) // Queue up save because: Tab removed getApp()->windows->queueSave(); - for (int i = 0; i < this->items_.count(); i++) + int removingIndex = this->indexOf(page); + assert(removingIndex != -1); + + if (this->selectedPage_ == page) { - if (this->items_[i].page == page) + // The page that we are removing is currently selected. We need to determine + // the best tab to select before we remove this one. We follow a strategy used + // by many web browsers: select the next tab. If there is no next tab, select + // the previous tab. + int countVisible = this->getVisibleTabCount(); + int visibleIndex = this->visibleIndexOf(page); + assert(visibleIndex != -1); // A selected page should always be visible + + if (this->items_.count() == 1) { - if (this->items_.count() == 1) - { - this->select(nullptr); - } - else if (i == this->items_.count() - 1) - { - this->select(this->items_[i - 1].page); - } - else - { - this->select(this->items_[i + 1].page); - } - - this->items_[i].page->deleteLater(); - this->items_[i].tab->deleteLater(); - - // if (this->items.empty()) { - // this->addNewPage(); - // } - - this->items_.removeAt(i); - break; + // Deleting only tab, select nothing + this->select(nullptr); + } + else if (countVisible == 1) + { + // Closing the only visible tab, try to select any tab (even if not visible) + int nextIndex = (removingIndex + 1) % this->items_.count(); + this->select(this->items_[nextIndex].page); + } + else if (visibleIndex == countVisible - 1) + { + // Closing last visible tab, select the previous visible tab + this->selectPreviousTab(); + } + else + { + // Otherwise, select the next visible tab + this->selectNextTab(); } } + // Remove page and delete resources + this->items_[removingIndex].page->deleteLater(); + this->items_[removingIndex].tab->deleteLater(); + this->items_.removeAt(removingIndex); + this->performLayout(true); } @@ -154,6 +166,48 @@ int Notebook::indexOf(QWidget *page) const return -1; } +int Notebook::visibleIndexOf(QWidget *page) const +{ + if (!this->tabVisibilityFilter_) + { + return this->indexOf(page); + } + + int i = 0; + for (const auto &item : this->items_) + { + if (item.page == page) + { + assert(this->tabVisibilityFilter_(item.tab)); + return i; + } + if (this->tabVisibilityFilter_(item.tab)) + { + ++i; + } + } + + return -1; +} + +int Notebook::getVisibleTabCount() const +{ + if (!this->tabVisibilityFilter_) + { + return this->items_.count(); + } + + int i = 0; + for (const auto &item : this->items_) + { + if (this->tabVisibilityFilter_(item.tab)) + { + ++i; + } + } + return i; +} + void Notebook::select(QWidget *page, bool focusPage) { if (page == this->selectedPage_) @@ -1162,6 +1216,15 @@ size_t Notebook::visibleButtonCount() const void Notebook::setTabVisibilityFilter(TabVisibilityFilter filter) { + if (filter) + { + // Wrap tab filter to always accept selected tabs. This prevents confusion + // when jumping to hidden tabs with the quick switcher, for example. + filter = [originalFilter = std::move(filter)](const NotebookTab *tab) { + return tab->isSelected() || originalFilter(tab); + }; + } + this->tabVisibilityFilter_ = std::move(filter); this->performLayout(); this->updateTabVisibility(); @@ -1200,11 +1263,15 @@ SplitNotebook::SplitNotebook(Window *parent) getSettings()->tabVisibility.connect( [this](int val, auto) { auto visibility = NotebookTabVisibility(val); + // Set the correct TabVisibilityFilter for the given visiblity setting. + // Note that selected tabs are always shown regardless of what the tab + // filter returns, so no need to include `tab->isSelected()` in the + // predicate. See Notebook::setTabVisibilityFilter. switch (visibility) { case NotebookTabVisibility::LiveOnly: this->setTabVisibilityFilter([](const NotebookTab *tab) { - return tab->isLive() || tab->isSelected(); + return tab->isLive(); }); break; case NotebookTabVisibility::AllTabs: diff --git a/src/widgets/Notebook.hpp b/src/widgets/Notebook.hpp index eb41af63a..e6cd7d62f 100644 --- a/src/widgets/Notebook.hpp +++ b/src/widgets/Notebook.hpp @@ -45,12 +45,55 @@ public: void removePage(QWidget *page); void removeCurrentPage(); + /** + * @brief Returns index of page in Notebook, or -1 if not found. + **/ int indexOf(QWidget *page) const; + + /** + * @brief Returns the visible index of page in Notebook, or -1 if not found. + * Given page should be visible according to the set TabVisibilityFilter. + **/ + int visibleIndexOf(QWidget *page) const; + + /** + * @brief Returns the number of visible tabs in Notebook. + **/ + int getVisibleTabCount() const; + + /** + * @brief Selects the Notebook tab containing the given page. + **/ virtual void select(QWidget *page, bool focusPage = true); + + /** + * @brief Selects the Notebook tab at the given index. Ignores whether tabs + * are visible or not. + **/ void selectIndex(int index, bool focusPage = true); + + /** + * @brief Selects the index'th visible tab in the Notebook. + * + * For example, selecting the 0th visible tab selects the first tab in this + * Notebook that is visible according to the TabVisibilityFilter. If no filter + * is set, equivalent to Notebook::selectIndex. + **/ void selectVisibleIndex(int index, bool focusPage = true); + + /** + * @brief Selects the next visible tab. Wraps to the start if required. + **/ void selectNextTab(bool focusPage = true); + + /** + * @brief Selects the previous visible tab. Wraps to the end if required. + **/ void selectPreviousTab(bool focusPage = true); + + /** + * @brief Selects the last visible tab. + **/ void selectLastTab(bool focusPage = true); int getPageCount() const;