From 8fc4a796ba28ba982e5642ecffdddfd059c6e6a6 Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Wed, 6 Dec 2023 19:57:30 +0100 Subject: [PATCH] QTabBar: properly calc tab positions when changing tab position The previous attempt to fix this bug did not work out well since it tried too hard to avoid double calculations. Therefore restore the old behavior and fix makeVisible() instead. This amends 1082038bd89d0bbba4df0d9b3f8af3cd0a2f96f2 Pick-to: 6.5 Fixes: QTBUG-119366 Change-Id: I738a3cd3537ecf0983480d2f961f45f81d101bd9 Reviewed-by: Axel Spoerl Reviewed-by: Qt CI Bot (cherry picked from commit 4e6f7ad26c31fe484421e873cf0de0fad0c878ee) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/widgets/qtabbar.cpp | 17 +++--- .../widgets/widgets/qtabbar/tst_qtabbar.cpp | 53 +++++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/widgets/widgets/qtabbar.cpp b/src/widgets/widgets/qtabbar.cpp index e8e70dc61e0..a9d58bedb6b 100644 --- a/src/widgets/widgets/qtabbar.cpp +++ b/src/widgets/widgets/qtabbar.cpp @@ -680,7 +680,10 @@ void QTabBarPrivate::makeVisible(int index) const int scrolledTabBarStart = qMax(1, scrollRect.left() + scrollOffset); const int scrolledTabBarEnd = qMin(lastTabEnd - 1, scrollRect.right() + scrollOffset); - if (tabStart < scrolledTabBarStart) { + if (available >= lastTabEnd) { + // the entire tabbar fits, reset scroll + scrollOffset = 0; + } else if (tabStart < scrolledTabBarStart) { // Tab is outside on the left, so scroll left. scrollOffset = tabStart - scrollRect.left(); } else if (tabEnd > scrolledTabBarEnd) { @@ -689,9 +692,6 @@ void QTabBarPrivate::makeVisible(int index) } else if (scrollOffset + entireScrollRect.width() > lastTabEnd + 1) { // fill any free space on the right without overshooting scrollOffset = qMax(0, lastTabEnd - entireScrollRect.width() + 1); - } else if (available >= lastTabEnd) { - // the entire tabbar fits, reset scroll - scrollOffset = 0; } leftB->setEnabled(scrollOffset > -scrollRect.left()); @@ -831,9 +831,14 @@ void QTabBarPrivate::refresh() pressedIndex = -1; } - layoutDirty = true; - if (q->isVisible()) + if (!q->isVisible()) { + layoutDirty = true; + } else { + layoutTabs(); + makeVisible(currentIndex); q->update(); + q->updateGeometry(); + } } /*! diff --git a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp index 2e6ae0c4cd2..08bfdb442e2 100644 --- a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp +++ b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp @@ -102,6 +102,7 @@ private slots: void changeTabTextKeepsScroll(); void settingCurrentTabBeforeShowDoesntScroll(); void checkPositionsAfterShapeChange(); + void checkScrollOffsetAfterTabRemoval(); private: void checkPositions(const TabBar &tabbar, const QList &positions); @@ -1543,5 +1544,57 @@ void tst_QTabBar::checkPositionsAfterShapeChange() QVERIFY(opt.rect.top() > 0); } +void tst_QTabBar::checkScrollOffsetAfterTabRemoval() +{ + QTabWidget tabWidget; + QTabBar *tabBar = tabWidget.tabBar(); + for (int i = 0; i < 10; ++i) + tabWidget.addTab(new QWidget, u"Tab %1"_s.arg(i)); + tabWidget.setTabPosition(QTabWidget::North); + tabWidget.resize(300, 300); + tabWidget.setCurrentIndex(0); + tabWidget.show(); + QVERIFY(QTest::qWaitForWindowExposed(&tabWidget)); + + auto *rightButton = tabBar->findChild(u"ScrollRightButton"_s); + auto *leftButton = tabBar->findChild(u"ScrollLeftButton"_s); + QVERIFY(leftButton); + QVERIFY(rightButton); + QVERIFY(rightButton->isEnabled()); + QVERIFY(!leftButton->isEnabled()); + // scroll to the right + tabBar->setCurrentIndex(9); + QVERIFY(!rightButton->isEnabled()); + QVERIFY(leftButton->isEnabled()); + // scroll to the center + tabBar->setCurrentIndex(2); + QVERIFY(rightButton->isEnabled()); + QVERIFY(leftButton->isEnabled()); + + const auto getScrollOffset = [&]() -> int { + return static_cast(QObjectPrivate::get(tabBar))->scrollOffset; + }; + // the scroll offset should not change when a tab right outside + // the scroll rect is removed + auto oldOffset = getScrollOffset(); + tabWidget.removeTab(9); + QCOMPARE(getScrollOffset(), oldOffset); + // the scroll offset must change when a tab left outside + // the scroll rect is removed + oldOffset = getScrollOffset(); + tabWidget.removeTab(0); + QVERIFY(getScrollOffset() < oldOffset); + + // the scroll offset must change when there is empty + // place in the right after tab removal + oldOffset = getScrollOffset(); + QVERIFY(oldOffset > 0); + for (int i : { 7, 6, 5, 4, 3 }) + tabWidget.removeTab(i); + QCOMPARE(getScrollOffset(), 0); + QVERIFY(!rightButton->isVisible()); + QVERIFY(!leftButton->isVisible()); +} + QTEST_MAIN(tst_QTabBar) #include "tst_qtabbar.moc"