From d6f2d8dcb6dbee89fc55bd439ab4a837f4f9fb4c Mon Sep 17 00:00:00 2001 From: Andreas Aardal Hanssen Date: Sat, 24 Nov 2012 21:50:05 +0100 Subject: [PATCH] Make sure panels always gain focus when activated or tab is pressed. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changes behavior, but I would argue it's a good change. If you create a panel and activate it (e.g., by simply showing it or reparenting it onto an already-visible item), you expect the panel to gain focus / which is sort of the whole point of activating it. Prior to this change, you had to have explicitly called setFocus() on one of the panel's children, which would give that item subfocus, for that item to auto- gain focus when the panel was activated. This change makes it more automatic. If the panel itself or any of the widgets in its focus chain can gain focus, they will gain focus when the panel is activated. So the new logic is - if the panel already has a focus item, so if someone explicitly set subfocus on an item before the panel is shown, that item gets focus. Otherwise, if the panel itself can gain focus (e.g., someone makes a line edit / text edit panel), it gains focus when activated. Otherwise, we search the focus chain until we find the first item that can gain focus. This last case is the file dialog case, where the dialog itself can't gain focus but typically the first item in the focus chain is the primary focus widget, such as the search field or the directory list view. The change also fixes this for the first Tab. If you clear focus on a panel, the user expects to be able to press Tab to regain focus. Prior to this change that didn't happen. Now, the panel or the first in the focus chain that can get focus gets focus on first tab. Task-number: QTBUG-28194 Change-Id: Id7ec1741d0d5eb4ea845469909c8d684e14017f1 Reviewed-by: Jan Arve Sæther --- src/widgets/graphicsview/qgraphicsscene.cpp | 33 +++++++++++++++++-- .../qgraphicsitem/tst_qgraphicsitem.cpp | 32 +++++++++--------- .../qgraphicsscene/tst_qgraphicsscene.cpp | 4 +-- .../qgraphicswidget/tst_qgraphicswidget.cpp | 6 +--- 4 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/widgets/graphicsview/qgraphicsscene.cpp b/src/widgets/graphicsview/qgraphicsscene.cpp index 769b488f39..139fad0163 100644 --- a/src/widgets/graphicsview/qgraphicsscene.cpp +++ b/src/widgets/graphicsview/qgraphicsscene.cpp @@ -771,9 +771,23 @@ void QGraphicsScenePrivate::setActivePanelHelper(QGraphicsItem *item, bool durin QEvent event(QEvent::WindowActivate); q->sendEvent(panel, &event); - // Set focus on the panel's focus item. - if (QGraphicsItem *focusItem = panel->focusItem()) + // Set focus on the panel's focus item, or itself if it's + // focusable, or on the first focusable item in the panel's + // focus chain as a last resort. + if (QGraphicsItem *focusItem = panel->focusItem()) { focusItem->setFocus(Qt::ActiveWindowFocusReason); + } else if (panel->flags() & QGraphicsItem::ItemIsFocusable) { + panel->setFocus(Qt::ActiveWindowFocusReason); + } else if (panel->isWidget()) { + QGraphicsWidget *fw = static_cast(panel)->d_func()->focusNext; + do { + if (fw->focusPolicy() & Qt::TabFocus) { + fw->setFocus(Qt::ActiveWindowFocusReason); + break; + } + fw = fw->d_func()->focusNext; + } while (fw != panel); + } } else if (q->isActive()) { // Activate the scene QEvent event(QEvent::WindowActivate); @@ -5328,6 +5342,21 @@ bool QGraphicsScene::focusNextPrevChild(bool next) setFocusItem(d->lastFocusItem, next ? Qt::TabFocusReason : Qt::BacktabFocusReason); return true; } + if (d->activePanel) { + if (d->activePanel->flags() & QGraphicsItem::ItemIsFocusable) { + setFocusItem(d->activePanel, next ? Qt::TabFocusReason : Qt::BacktabFocusReason); + return true; + } + if (d->activePanel->isWidget()) { + QGraphicsWidget *fw = static_cast(d->activePanel)->d_func()->focusNext; + do { + if (fw->focusPolicy() & Qt::TabFocus) { + setFocusItem(fw, next ? Qt::TabFocusReason : Qt::BacktabFocusReason); + return true; + } + } while (fw != d->activePanel); + } + } } if (!item && !d->tabFocusFirst) { // No widgets... diff --git a/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp index 9a996cd0c6..d5a40d0743 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicsitem/tst_qgraphicsitem.cpp @@ -10238,23 +10238,24 @@ void tst_QGraphicsItem::modality_clickFocus() EventSpy2 rect1Spy(&scene, rect1); EventSpy2 rect2Spy(&scene, rect2); - // activate rect1, it should not get focus + // activate rect1, it should get focus rect1->setActive(true); - QCOMPARE(scene.focusItem(), (QGraphicsItem *) 0); + QCOMPARE(scene.focusItem(), (QGraphicsItem *) rect1); - // focus stays unset when rect2 becomes modal + // focus stays when rect2 becomes modal rect2->setPanelModality(QGraphicsItem::SceneModal); - QCOMPARE(scene.focusItem(), (QGraphicsItem *) 0); - QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 0); + QCOMPARE(scene.focusItem(), (QGraphicsItem *) rect1); + QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 1); QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 0); QCOMPARE(rect2Spy.counts[QEvent::FocusIn], 0); QCOMPARE(rect2Spy.counts[QEvent::FocusOut], 0); // clicking on rect1 should not set it's focus item + rect1->clearFocus(); sendMouseClick(&scene, QPointF(-25, -25)); QCOMPARE(rect1->focusItem(), (QGraphicsItem *) 0); - QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 0); - QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 0); + QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 1); + QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 1); QCOMPARE(rect2Spy.counts[QEvent::FocusIn], 0); QCOMPARE(rect2Spy.counts[QEvent::FocusOut], 0); @@ -10262,33 +10263,34 @@ void tst_QGraphicsItem::modality_clickFocus() rect2->setActive(true); sendMouseClick(&scene, QPointF(75, 75)); QCOMPARE(scene.focusItem(), (QGraphicsItem *) rect2); - QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 0); - QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 0); + QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 1); + QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 1); QCOMPARE(rect2Spy.counts[QEvent::FocusIn], 1); QCOMPARE(rect2Spy.counts[QEvent::FocusOut], 0); // clicking on rect1 does *not* give it focus rect1->setActive(true); + rect1->clearFocus(); sendMouseClick(&scene, QPointF(-25, -25)); QCOMPARE(scene.focusItem(), (QGraphicsItem *) 0); - QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 0); - QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 0); + QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 2); + QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 2); QCOMPARE(rect2Spy.counts[QEvent::FocusIn], 1); QCOMPARE(rect2Spy.counts[QEvent::FocusOut], 1); // focus doesn't change when leaving modality either rect2->setPanelModality(QGraphicsItem::NonModal); QCOMPARE(scene.focusItem(), (QGraphicsItem *) 0); - QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 0); - QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 0); + QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 2); + QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 2); QCOMPARE(rect2Spy.counts[QEvent::FocusIn], 1); QCOMPARE(rect2Spy.counts[QEvent::FocusOut], 1); // click on rect1, it should get focus now sendMouseClick(&scene, QPointF(-25, -25)); QCOMPARE(scene.focusItem(), (QGraphicsItem *) rect1); - QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 1); - QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 0); + QCOMPARE(rect1Spy.counts[QEvent::FocusIn], 3); + QCOMPARE(rect1Spy.counts[QEvent::FocusOut], 2); QCOMPARE(rect2Spy.counts[QEvent::FocusIn], 1); QCOMPARE(rect2Spy.counts[QEvent::FocusOut], 1); } diff --git a/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp b/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp index 586dfedbf9..69e687d633 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp @@ -3887,11 +3887,11 @@ void tst_QGraphicsScene::initialFocus_data() QTest::addColumn("shouldHaveFocus"); QTest::newRow("inactive scene, normal item") << false << false << false << false; - QTest::newRow("inactive scene, panel item") << false << false << true << false; + QTest::newRow("inactive scene, panel item") << false << false << true << true; QTest::newRow("inactive scene, normal item, explicit focus") << false << true << false << true; QTest::newRow("inactive scene, panel, explicit focus") << false << true << true << true; QTest::newRow("active scene, normal item") << true << false << false << false; - QTest::newRow("active scene, panel item") << true << false << true << false; + QTest::newRow("active scene, panel item") << true << false << true << true; QTest::newRow("active scene, normal item, explicit focus") << true << true << false << true; QTest::newRow("active scene, panel, explicit focus") << true << true << true << true; } diff --git a/tests/auto/widgets/graphicsview/qgraphicswidget/tst_qgraphicswidget.cpp b/tests/auto/widgets/graphicsview/qgraphicswidget/tst_qgraphicswidget.cpp index ad8fa3f241..b7a9885b2a 100644 --- a/tests/auto/widgets/graphicsview/qgraphicswidget/tst_qgraphicswidget.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicswidget/tst_qgraphicswidget.cpp @@ -3323,10 +3323,7 @@ void verifyTabFocus(QGraphicsScene *scene, const QList &chain int n = chain.size() * (wrapsAround ? 3 : 1); for (int i = 0; i < n; ++i) { - if (i == 0) - chain.at(0)->setFocus(); - else - qApp->sendEvent(scene, &tabEvent); + qApp->sendEvent(scene, &tabEvent); QVERIFY(chain.at(i % chain.size())->hasFocus()); QCOMPARE(scene->focusItem(), chain.at(i % chain.size())); } @@ -3364,7 +3361,6 @@ void tst_QGraphicsWidget::tabFocus() verifyTabFocus(&scene, QList() << widget << widget2, false); scene.setActivePanel(widget3); - widget3->setFocus(); QCOMPARE(scene.focusItem(), (QGraphicsItem *)widget3); verifyTabFocus(&scene, QList() << widget3, true);