From: Felix Ernst Date: Sun, 18 Aug 2024 21:41:34 +0000 (+0000) Subject: Implement "Focus Places Panel" X-Git-Url: https://cloud.milkyroute.net/gitweb/dolphin.git/commitdiff_plain/f220e3b0783a24a6c7195f170297cf4b12a29d85 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 80d8fda1b..d2bf74c5f 100644 --- 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 bf605fa96..f68939f87 100644 --- 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(m_placesPanel->parent())); + disconnect(static_cast(m_placesPanel->parent()), + &DolphinDockWidget::visibilityChanged, + this, + &DolphinMainWindow::slotPlacesPanelVisibilityChanged); } QVector 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.") + 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() " to display it again.") + 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(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 5f2ed20ca..37994b85a 100644 --- 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 2f7ea857d..5c9afa03d 100644 --- a/src/dolphinui.rc +++ b/src/dolphinui.rc @@ -1,6 +1,6 @@ - + @@ -73,7 +73,6 @@ - diff --git a/src/tests/dolphinmainwindowtest.cpp b/src/tests/dolphinmainwindowtest.cpp index 2d90ae52f..94e6d5be4 100644 --- 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(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.