From ca15f650a1a914bb9a41131109c46c4e52c5ebb1 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Tue, 25 Apr 2023 12:01:36 +0200 Subject: [PATCH] QTabBar: don't scroll when laying out the tabs QTabBar lays out the tabs when the bar's size or content changes, often lazily. This should not change the scroll offset of the tabbar, which is controlled by the user, or at most when a tab needs to be made visible (e.g. when it becomes the current tab). Move the logic of updating the scoll offset into the makeVisible function, so that the scroll is only adjusted to either leave no gap between the last tab and the right edge of the widget of there is still a scroll; or to reset it to 0 if there is enough space for the entire tab bar. Since layoutTabs does show and hide the scroll buttons, we cannot skip this when the buttons are invisible. However, the normalizedScrollRect helper now needs to return the entire widget rect if there are no visible scroll buttons. Add a test case that simulates the previously broken behavior where the scroll got unnecessarily reset to 0 when resizing. Fixes: QTBUG-113140 Pick-to: 6.5 Change-Id: Ic19fbb82821ea09cc5e7646dcbce3aa7607909c2 Reviewed-by: Qt CI Bot Reviewed-by: Santhosh Kumar Reviewed-by: Axel Spoerl --- src/widgets/widgets/qtabbar.cpp | 21 +++++-- .../widgets/widgets/qtabbar/tst_qtabbar.cpp | 62 +++++++++++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/widgets/widgets/qtabbar.cpp b/src/widgets/widgets/qtabbar.cpp index 74e94cac4ae..837874aebfd 100644 --- a/src/widgets/widgets/qtabbar.cpp +++ b/src/widgets/widgets/qtabbar.cpp @@ -534,7 +534,6 @@ void QTabBarPrivate::layoutTabs() if (useScrollButtons && tabList.size() && last > available) { const QRect scrollRect = normalizedScrollRect(0); - scrollOffset = -scrollRect.left(); Q_Q(QTabBar); QStyleOption opt; @@ -571,10 +570,9 @@ void QTabBarPrivate::layoutTabs() leftB->show(); rightB->setGeometry(scrollButtonRightRect); - rightB->setEnabled(last - scrollOffset > scrollRect.x() + scrollRect.width()); + rightB->setEnabled(last + scrollRect.left() > scrollRect.x() + scrollRect.width()); rightB->show(); } else { - scrollOffset = 0; rightB->hide(); leftB->hide(); } @@ -591,6 +589,11 @@ QRect QTabBarPrivate::normalizedScrollRect(int index) // tab bar itself is in a different orientation. Q_Q(QTabBar); + // If scrollbuttons are not visible, then there's no tear either, and + // the entire widget is the scroll rect. + if (leftB->isHidden()) + return verticalTabs(shape) ? q->rect().transposed() : q->rect(); + QStyleOptionTab opt; q->initStyleOption(&opt, currentIndex); opt.rect = q->rect(); @@ -670,16 +673,18 @@ int QTabBarPrivate::hoveredTabIndex() const void QTabBarPrivate::makeVisible(int index) { Q_Q(QTabBar); - if (!validIndex(index) || leftB->isHidden()) + if (!validIndex(index)) return; const QRect tabRect = tabList.at(index)->rect; const int oldScrollOffset = scrollOffset; const bool horiz = !verticalTabs(shape); + const int available = horiz ? q->width() : q->height(); const int tabStart = horiz ? tabRect.left() : tabRect.top(); const int tabEnd = horiz ? tabRect.right() : tabRect.bottom(); const int lastTabEnd = horiz ? tabList.constLast()->rect.right() : tabList.constLast()->rect.bottom(); const QRect scrollRect = normalizedScrollRect(index); + const QRect entireScrollRect = normalizedScrollRect(0); // ignore tears const int scrolledTabBarStart = qMax(1, scrollRect.left() + scrollOffset); const int scrolledTabBarEnd = qMin(lastTabEnd - 1, scrollRect.right() + scrollOffset); @@ -689,6 +694,12 @@ void QTabBarPrivate::makeVisible(int index) } else if (tabEnd > scrolledTabBarEnd) { // Tab is outside on the right, so scroll right. scrollOffset = tabEnd - scrollRect.right(); + } else if (scrollOffset + entireScrollRect.width() > lastTabEnd + 1) { + // there's space on the right + scrollOffset = lastTabEnd - entireScrollRect.width() + 1; + } else if (available >= lastTabEnd) { + // the entire tabbar fits, reset scroll + scrollOffset = 0; } leftB->setEnabled(scrollOffset > -scrollRect.left()); @@ -1778,6 +1789,8 @@ void QTabBar::resizeEvent(QResizeEvent *) Q_D(QTabBar); if (d->layoutDirty) updateGeometry(); + + // when resizing, we want to keep the scroll offset as much as possible d->layoutTabs(); d->makeVisible(d->currentIndex); diff --git a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp index 1d7e999b101..8fd7b5c4b26 100644 --- a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp +++ b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp @@ -94,6 +94,9 @@ private slots: void hoverTab_data(); void hoverTab(); + void resizeKeepsScroll_data(); + void resizeKeepsScroll(); + private: void checkPositions(const TabBar &tabbar, const QList &positions); }; @@ -1346,5 +1349,64 @@ void tst_QTabBar::hoverTab() QCOMPARE(tabbar.styleOptions[1].state & QStyle::State_MouseOver, QStyle::State_None); } + +void tst_QTabBar::resizeKeepsScroll_data() +{ + QTest::addColumn("tabShape"); + + QTest::addRow("North") << QTabBar::RoundedNorth; + QTest::addRow("East") << QTabBar::RoundedEast; + QTest::addRow("South") << QTabBar::RoundedSouth; + QTest::addRow("West") << QTabBar::RoundedWest; +} + +void tst_QTabBar::resizeKeepsScroll() +{ + QFETCH(QTabBar::Shape, tabShape); + + QTabBar tabBar; + TabBarScrollingProxyStyle proxyStyle; + tabBar.setStyle(&proxyStyle); + + for (int i = 0; i < 10; ++i) + tabBar.addTab(u"Tab Number %1"_s.arg(i)); + + tabBar.setShape(tabShape); + tabBar.setUsesScrollButtons(true); + + // resize to half + const QSize fullSize = tabBar.sizeHint(); + const bool horizontal = fullSize.width() > fullSize.height(); + if (horizontal) + tabBar.resize(fullSize.width() / 2, fullSize.height()); + else + tabBar.resize(fullSize.width(), fullSize.height() / 2); + + tabBar.show(); + QVERIFY(QTest::qWaitForWindowExposed(&tabBar)); + + // select a tab outside, this will scroll + tabBar.setCurrentIndex(6); + // the first tab is now scrolled out + const int scrollOffset = horizontal + ? tabBar.tabRect(0).left() + : tabBar.tabRect(0).top(); + QCOMPARE_LT(scrollOffset, 0); + // the current index is now fully visible, with margin on both sides + tabBar.setCurrentIndex(5); + + // make the tab bar a bit larger, by the width of a tab + if (horizontal) + tabBar.resize(tabBar.width() + tabBar.tabRect(5).width(), tabBar.height()); + else + tabBar.resize(tabBar.width(), tabBar.height() + tabBar.tabRect(5).height()); + + // this should not change the scroll + QCOMPARE(scrollOffset, horizontal + ? tabBar.tabRect(0).left() + : tabBar.tabRect(0).top()); + +} + QTEST_MAIN(tst_QTabBar) #include "tst_qtabbar.moc"