Implement "Focus Places Panel"

This commit implements an action to move focus to the Places panel
analogous to "Focus Terminal Panel" functionality-wise.
The implementation of the "Focus Terminal Panel" and "Focus Places
Panel" actions is streamlined while improving their code quality.

The "Focus Terminal Panel" action is moved into the "Show Panels"
sub-menu because it makes more sense to be there considering that its
previous location (the "Tools" menu) is meant for external applications
and not for functionality internal to Dolphin.

This commit also makes it so the keyboard focus is moved to and from
the Places panel whenever it is toggled visible or invisible. This is
now consistent with the focus handling when the Terminal panel is shown
or hidden.

The "Focus Places Panel" is one of the actions which was wished for in
KDE's accessibility chat room because people relying on keyboard
controls might need to press the Tab key a lot to move from the view to
the Places panel.

The new default shortcut is Ctrl+P.

diff --git a/src/admin/workerintegration.cpp b/src/admin/workerintegration.cpp
index 80d8fda1b15e72f5865b7df3ed352698398fd44e..d2bf74c5f10b4ad1bad5b01035f7b9489eceb2c4 100644 (file)
--- a/src/admin/workerintegration.cpp
+++ b/src/admin/workerintegration.cpp
@@ -122,7 +122,9 @@ WorkerIntegration::WorkerIntegration(DolphinMainWindow *parent, QAction *actAsAd
 
 void WorkerIntegration::createActAsAdminAction(KActionCollection *actionCollection, DolphinMainWindow *dolphinMainWindow)
 {
-    Q_ASSERT(!instance);
+    Q_ASSERT(!instance /* We never want to construct more than one instance,
+    however in automatic testing sometimes multiple DolphinMainWindows are created, so this assert is diluted to accommodate for that: */
+             || instance->parent() != dolphinMainWindow);
     if (isWorkerInstalled()) {
         QAction *actAsAdminAction = actionCollection->addAction(QStringLiteral("act_as_admin"));
         actAsAdminAction->setText(actionName());

diff --git a/src/dolphinmainwindow.cpp b/src/dolphinmainwindow.cpp
index bf605fa9630e0ee483a5fc18bea443b9d09aa572..f68939f87dcc82ebcc90d809672759a5884f7a03 100644 (file)
--- a/src/dolphinmainwindow.cpp
+++ b/src/dolphinmainwindow.cpp
@@ -241,6 +241,13 @@ DolphinMainWindow::~DolphinMainWindow()
 {
     // This fixes a crash on Wayland when closing the mainwindow while another dialog is open.
     disconnect(QGuiApplication::clipboard(), &QClipboard::dataChanged, this, &DolphinMainWindow::updatePasteAction);
+
+    // This fixes a crash in dolphinmainwindowtest where the connection below fires even though the KMainWindow destructor of this object is already running.
+    Q_CHECK_PTR(qobject_cast<DolphinDockWidget *>(m_placesPanel->parent()));
+    disconnect(static_cast<DolphinDockWidget *>(m_placesPanel->parent()),
+               &DolphinDockWidget::visibilityChanged,
+               this,
+               &DolphinMainWindow::slotPlacesPanelVisibilityChanged);
 }
 
 QVector<DolphinViewContainer *> DolphinMainWindow::viewContainers() const
@@ -1136,11 +1143,21 @@ void DolphinMainWindow::togglePanelLockState()
     GeneralSettings::setLockPanels(newLockState);
 }
 
-void DolphinMainWindow::slotTerminalPanelVisibilityChanged()
+void DolphinMainWindow::slotTerminalPanelVisibilityChanged(bool visible)
 {
-    if (m_terminalPanel->isHiddenInVisibleWindow() && m_activeViewContainer) {
+    if (!visible && m_activeViewContainer) {
         m_activeViewContainer->view()->setFocus();
     }
+    // Putting focus to the Terminal is not handled here but in TerminalPanel::showEvent().
+}
+
+void DolphinMainWindow::slotPlacesPanelVisibilityChanged(bool visible)
+{
+    if (!visible && m_activeViewContainer) {
+        m_activeViewContainer->view()->setFocus();
+        return;
+    }
+    m_placesPanel->setFocus();
 }
 
 void DolphinMainWindow::goBack()
@@ -2046,14 +2063,6 @@ void DolphinMainWindow::setupActions()
         openTerminalHere->setIcon(QIcon::fromTheme(QStringLiteral("utilities-terminal")));
         actionCollection()->setDefaultShortcut(openTerminalHere, Qt::SHIFT | Qt::ALT | Qt::Key_F4);
         connect(openTerminalHere, &QAction::triggered, this, &DolphinMainWindow::openTerminalHere);
-
-#if HAVE_TERMINAL
-        QAction *focusTerminalPanel = actionCollection()->addAction(QStringLiteral("focus_terminal_panel"));
-        focusTerminalPanel->setText(i18nc("@action:inmenu Tools", "Focus Terminal Panel"));
-        focusTerminalPanel->setIcon(QIcon::fromTheme(QStringLiteral("swap-panels")));
-        actionCollection()->setDefaultShortcut(focusTerminalPanel, Qt::CTRL | Qt::SHIFT | Qt::Key_F4);
-        connect(focusTerminalPanel, &QAction::triggered, this, &DolphinMainWindow::focusTerminalPanel);
-#endif
     }
 
     // setup 'Bookmarks' menu
@@ -2309,8 +2318,15 @@ void DolphinMainWindow::setupDockWidgets()
                                           "advanced tasks. To learn more about terminals use the help features in a "
                                           "standalone terminal application like Konsole.</para>")
                                    + panelWhatsThis);
-    }
-#endif
+
+        QAction *focusTerminalPanel = actionCollection()->addAction(QStringLiteral("focus_terminal_panel"));
+        focusTerminalPanel->setText(i18nc("@action:inmenu Tools", "Focus Terminal Panel"));
+        focusTerminalPanel->setToolTip(i18nc("@info:tooltip", "Move keyboard focus to and from the Terminal panel."));
+        focusTerminalPanel->setIcon(QIcon::fromTheme(QStringLiteral("swap-panels")));
+        actionCollection()->setDefaultShortcut(focusTerminalPanel, Qt::CTRL | Qt::SHIFT | Qt::Key_F4);
+        connect(focusTerminalPanel, &QAction::triggered, this, &DolphinMainWindow::toggleTerminalPanelFocus);
+    } // endif "shell_access" allowed
+#endif // HAVE_TERMINAL
 
     if (GeneralSettings::version() < 200) {
         infoDock->hide();
@@ -2340,6 +2356,7 @@ void DolphinMainWindow::setupDockWidgets()
     connect(m_placesPanel, &PlacesPanel::errorMessage, this, &DolphinMainWindow::showErrorMessage);
     connect(this, &DolphinMainWindow::urlChanged, m_placesPanel, &PlacesPanel::setUrl);
     connect(placesDock, &DolphinDockWidget::visibilityChanged, &DolphinUrlNavigatorsController::slotPlacesPanelVisibilityChanged);
+    connect(placesDock, &DolphinDockWidget::visibilityChanged, this, &DolphinMainWindow::slotPlacesPanelVisibilityChanged);
     connect(this, &DolphinMainWindow::settingsChanged, m_placesPanel, &PlacesPanel::readSettings);
     connect(m_placesPanel, &PlacesPanel::storageTearDownRequested, this, &DolphinMainWindow::slotStorageTearDownFromPlacesRequested);
     connect(m_placesPanel, &PlacesPanel::storageTearDownExternallyRequested, this, &DolphinMainWindow::slotStorageTearDownExternallyRequested);
@@ -2381,6 +2398,13 @@ void DolphinMainWindow::setupDockWidgets()
                                     "</interface> to display it again.</para>")
                              + panelWhatsThis);
 
+    QAction *focusPlacesPanel = actionCollection()->addAction(QStringLiteral("focus_places_panel"));
+    focusPlacesPanel->setText(i18nc("@action:inmenu View", "Focus Places Panel"));
+    focusPlacesPanel->setToolTip(i18nc("@info:tooltip", "Move keyboard focus to and from the Places panel."));
+    focusPlacesPanel->setIcon(QIcon::fromTheme(QStringLiteral("swap-panels")));
+    actionCollection()->setDefaultShortcut(focusPlacesPanel, Qt::CTRL | Qt::Key_P);
+    connect(focusPlacesPanel, &QAction::triggered, this, &DolphinMainWindow::togglePlacesPanelFocus);
+
     // Add actions into the "Panels" menu
     KActionMenu *panelsMenu = new KActionMenu(i18nc("@action:inmenu View", "Show Panels"), this);
     actionCollection()->addAction(QStringLiteral("panels"), panelsMenu);
@@ -2394,8 +2418,11 @@ void DolphinMainWindow::setupDockWidgets()
     panelsMenu->addAction(ac->action(QStringLiteral("show_folders_panel")));
     panelsMenu->addAction(ac->action(QStringLiteral("show_terminal_panel")));
     panelsMenu->addSeparator();
-    panelsMenu->addAction(actionShowAllPlaces);
     panelsMenu->addAction(lockLayoutAction);
+    panelsMenu->addSeparator();
+    panelsMenu->addAction(actionShowAllPlaces);
+    panelsMenu->addAction(focusPlacesPanel);
+    panelsMenu->addAction(ac->action(QStringLiteral("focus_terminal_panel")));
 
     connect(panelsMenu->menu(), &QMenu::aboutToShow, this, [actionShowAllPlaces] {
         actionShowAllPlaces->setEnabled(DolphinPlacesModelSingleton::instance().placesModel()->hiddenCount());
@@ -2874,20 +2901,40 @@ void DolphinMainWindow::saveNewToolbarConfig()
     (static_cast<KHamburgerMenu *>(actionCollection()->action(KStandardAction::name(KStandardAction::HamburgerMenu))))->hideActionsOf(toolBar());
 }
 
-void DolphinMainWindow::focusTerminalPanel()
+void DolphinMainWindow::toggleTerminalPanelFocus()
 {
-    if (m_terminalPanel->isVisible()) {
-        if (m_terminalPanel->terminalHasFocus()) {
-            m_activeViewContainer->view()->setFocus(Qt::FocusReason::ShortcutFocusReason);
-            actionCollection()->action(QStringLiteral("focus_terminal_panel"))->setText(i18nc("@action:inmenu Tools", "Focus Terminal Panel"));
-        } else {
-            m_terminalPanel->setFocus(Qt::FocusReason::ShortcutFocusReason);
-            actionCollection()->action(QStringLiteral("focus_terminal_panel"))->setText(i18nc("@action:inmenu Tools", "Defocus Terminal Panel"));
-        }
-    } else {
-        actionCollection()->action(QStringLiteral("show_terminal_panel"))->trigger();
+    if (!m_terminalPanel->isVisible()) {
+        actionCollection()->action(QStringLiteral("show_terminal_panel"))->trigger(); // Also moves focus to the panel.
         actionCollection()->action(QStringLiteral("focus_terminal_panel"))->setText(i18nc("@action:inmenu Tools", "Defocus Terminal Panel"));
+        return;
+    }
+
+    if (m_terminalPanel->terminalHasFocus()) {
+        m_activeViewContainer->view()->setFocus(Qt::FocusReason::ShortcutFocusReason);
+        actionCollection()->action(QStringLiteral("focus_terminal_panel"))->setText(i18nc("@action:inmenu Tools", "Focus Terminal Panel"));
+        return;
     }
+
+    m_terminalPanel->setFocus(Qt::FocusReason::ShortcutFocusReason);
+    actionCollection()->action(QStringLiteral("focus_terminal_panel"))->setText(i18nc("@action:inmenu Tools", "Defocus Terminal Panel"));
+}
+
+void DolphinMainWindow::togglePlacesPanelFocus()
+{
+    if (!m_placesPanel->isVisible()) {
+        actionCollection()->action(QStringLiteral("show_places_panel"))->trigger(); // Also moves focus to the panel.
+        actionCollection()->action(QStringLiteral("focus_places_panel"))->setText(i18nc("@action:inmenu View", "Defocus Terminal Panel"));
+        return;
+    }
+
+    if (m_placesPanel->hasFocus()) {
+        m_activeViewContainer->view()->setFocus(Qt::FocusReason::ShortcutFocusReason);
+        actionCollection()->action(QStringLiteral("focus_places_panel"))->setText(i18nc("@action:inmenu View", "Focus Places Panel"));
+        return;
+    }
+
+    m_placesPanel->setFocus(Qt::FocusReason::ShortcutFocusReason);
+    actionCollection()->action(QStringLiteral("focus_places_panel"))->setText(i18nc("@action:inmenu View", "Defocus Places Panel"));
 }
 
 DolphinMainWindow::UndoUiInterface::UndoUiInterface()

diff --git a/src/dolphinmainwindow.h b/src/dolphinmainwindow.h
index 5f2ed20ca6547c5e78ba6412a1172178e8bba8cc..37994b85ae4d6940f951a863e0ae86cfdda2e35a 100644 (file)
--- a/src/dolphinmainwindow.h
+++ b/src/dolphinmainwindow.h
@@ -403,10 +403,21 @@ private Q_SLOTS:
     void togglePanelLockState();
 
     /**
-     * Is invoked if the Terminal panel got visible/invisible and takes care
-     * that the active view has the focus if the Terminal panel is invisible.
+     * Is invoked whenever the Terminal panel visibility is changed by the user and then moves the focus
+     * to the active view if the panel was hidden.
+     * @note The opposite action (putting focus to the Terminal) is not handled
+     *       here but in TerminalPanel::showEvent().
+     * @param visible the new visibility state of the terminal panel
      */
-    void slotTerminalPanelVisibilityChanged();
+    void slotTerminalPanelVisibilityChanged(bool visible);
+
+    /**
+     * Is invoked whenever the Places panel visibility is changed by the user and then either moves the focus
+     * - to the Places panel if it was made visible, or
+     * - to the active view if the panel was hidden.
+     * @param visible the new visibility state of the Places panel
+     */
+    void slotPlacesPanelVisibilityChanged(bool visible);
 
     /** Goes back one step of the URL history. */
     void goBack();
@@ -456,8 +467,11 @@ private Q_SLOTS:
     /** Opens a terminal window for the URL. */
     void openTerminalJob(const QUrl &url);
 
-    /** Focus a Terminal Panel. */
-    void focusTerminalPanel();
+    /** Toggles focus to/from a Terminal Panel. */
+    void toggleTerminalPanelFocus();
+
+    /** Toggles focus to/from the Places Panel. */
+    void togglePlacesPanelFocus();
 
     /** Opens the settings dialog for Dolphin. */
     void editSettings();

diff --git a/src/dolphinui.rc b/src/dolphinui.rc
index 2f7ea857d1c5f44ea617ae56aac8e369b19fb3f0..5c9afa03d0ec0d876bf5e949601bffff124f3e8f 100644 (file)
--- a/src/dolphinui.rc
+++ b/src/dolphinui.rc
@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 <!DOCTYPE gui SYSTEM "kpartgui.dtd">
-<gui name="dolphin" version="40">
+<gui name="dolphin" version="41">
     <MenuBar>
         <Menu name="file">
             <Action name="new_menu" />
@@ -73,7 +73,6 @@
             <Action name="open_preferred_search_tool" />
             <Action name="open_terminal" />
             <Action name="open_terminal_here" />
-            <Action name="focus_terminal_panel"/>
             <Action name="compare_files" />
             <Action name="change_remote_encoding" />
         </Menu>

diff --git a/src/tests/dolphinmainwindowtest.cpp b/src/tests/dolphinmainwindowtest.cpp
index 2d90ae52f9bda448a3e442c728025d1fd7559f9f..94e6d5be4486c3d282a02de72a4e12350754c26f 100644 (file)
--- a/src/tests/dolphinmainwindowtest.cpp
+++ b/src/tests/dolphinmainwindowtest.cpp
@@ -45,6 +45,7 @@ private Q_SLOTS:
     void testNewFileMenuEnabled();
     void testWindowTitle_data();
     void testWindowTitle();
+    void testFocusPlacesPanel();
     void testPlacesPanelWidthResistance();
     void testGoActions();
     void testOpenFiles();
@@ -282,6 +283,44 @@ void DolphinMainWindowTest::testWindowTitle()
     QCOMPARE(m_mainWindow->windowTitle(), expectedWindowTitle);
 }
 
+void DolphinMainWindowTest::testFocusPlacesPanel()
+{
+    m_mainWindow->openDirectories({QUrl::fromLocalFile(QDir::homePath())}, false);
+    m_mainWindow->show();
+    QVERIFY(QTest::qWaitForWindowExposed(m_mainWindow.data()));
+    QVERIFY(m_mainWindow->isVisible());
+
+    QWidget *placesPanel = reinterpret_cast<QWidget *>(m_mainWindow->m_placesPanel);
+    QVERIFY2(QTest::qWaitFor(
+                 [&]() {
+                     return placesPanel && placesPanel->isVisible() && placesPanel->width() > 0 && placesPanel->height() > 0;
+                 },
+                 5000),
+             "The test couldn't be initialised properly. The places panel should be visible.");
+
+    QAction *focusPlacesPanelAction = m_mainWindow->actionCollection()->action(QStringLiteral("focus_places_panel"));
+    QAction *showPlacesPanelAction = m_mainWindow->actionCollection()->action(QStringLiteral("show_places_panel"));
+
+    focusPlacesPanelAction->trigger();
+    QVERIFY(placesPanel->hasFocus());
+
+    focusPlacesPanelAction->trigger();
+    QVERIFY2(m_mainWindow->activeViewContainer()->isAncestorOf(QApplication::focusWidget()),
+             "Triggering focus_places_panel while the panel already has focus should return the focus to the view.");
+
+    focusPlacesPanelAction->trigger();
+    QVERIFY(placesPanel->hasFocus());
+
+    showPlacesPanelAction->trigger();
+    QVERIFY(!placesPanel->isVisible());
+    QVERIFY2(m_mainWindow->activeViewContainer()->isAncestorOf(QApplication::focusWidget()),
+             "Hiding the Places panel while it has focus should return the focus to the view.");
+
+    showPlacesPanelAction->trigger();
+    QVERIFY(placesPanel->isVisible());
+    QVERIFY2(placesPanel->hasFocus(), "Enabling the Places panel should move keyboard focus there.");
+}
+
 /**
  * The places panel will resize itself if any of the other widgets requires too much horizontal space
  * but a user never wants the size of the places panel to change unless they resized it themselves explicitly.