From b6fc58c3c32b03f504a5f697b62c4834dc3f650a Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Wed, 12 Aug 2020 20:45:17 +0200 Subject: [PATCH] Adress the first round of Angelaccio's review comments - Split the viewContainers(bool includeInActive) into two methods without parameters - Prevent users from accidently hiding all Url Navigators by preventing the dangerous action and then displaying a helpful message instead Unrelated to review comments: Remove a useless line of code --- src/dolphinbookmarkhandler.cpp | 2 +- src/dolphinmainwindow.cpp | 25 ++++++++++++++++++++----- src/dolphinmainwindow.h | 18 ++++++++++-------- src/dolphinviewcontainer.cpp | 8 ++++++-- src/dolphinviewcontainer.h | 3 ++- 5 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/dolphinbookmarkhandler.cpp b/src/dolphinbookmarkhandler.cpp index efcb41692f..576a9314b1 100644 --- a/src/dolphinbookmarkhandler.cpp +++ b/src/dolphinbookmarkhandler.cpp @@ -68,7 +68,7 @@ bool DolphinBookmarkHandler::supportsTabs() const QList DolphinBookmarkHandler::currentBookmarkList() const { - const auto viewContainers = m_mainWindow->viewContainers(false); + const auto viewContainers = m_mainWindow->activeViewContainers(); QList bookmarks; bookmarks.reserve(viewContainers.size()); for (const auto viewContainer : viewContainers) { diff --git a/src/dolphinmainwindow.cpp b/src/dolphinmainwindow.cpp index 6d47afae16..5e6b6e94a2 100644 --- a/src/dolphinmainwindow.cpp +++ b/src/dolphinmainwindow.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -202,7 +203,7 @@ DolphinMainWindow::~DolphinMainWindow() { } -QVector DolphinMainWindow::viewContainers(bool includeInactive) const +QVector DolphinMainWindow::activeViewContainers() const { QVector viewContainers; @@ -845,7 +846,8 @@ void DolphinMainWindow::showFilterBar() void DolphinMainWindow::toggleLocationInToolbar() { // collect needed variables - const bool locationInToolbar = actionCollection()->action(QStringLiteral("location_in_toolbar"))->isChecked(); + QAction *locationInToolbarAction = actionCollection()->action(QStringLiteral("location_in_toolbar")); + const bool locationInToolbar = locationInToolbarAction->isChecked(); auto viewContainers = this->viewContainers(); auto urlNavigatorWidgetAction = static_cast (actionCollection()->action(QStringLiteral("url_navigator"))); @@ -856,6 +858,21 @@ void DolphinMainWindow::toggleLocationInToolbar() const int selectionStart = lineEdit->selectionStart(); const int selectionLength = lineEdit->selectionLength(); + // prevent the switching if it would leave the user without a visible UrlNavigator + if (locationInToolbar && !toolBar()->actions().contains(urlNavigatorWidgetAction)) { + QAction *configureToolbars = actionCollection()->action(KStandardAction::name(KStandardAction::ConfigureToolbars)); + KMessageWidget *messageWidget = m_activeViewContainer->showMessage( + xi18nc("@info 2 is the visible text on a button just below the message", + "The location could not be moved onto the toolbar because there is currently " + "no \"%1\" item on the toolbar. Select %2 and add the " + "\"%1\" item. Then this will work.", urlNavigatorWidgetAction->iconText(), + configureToolbars->iconText()), DolphinViewContainer::Information); + messageWidget->addAction(configureToolbars); + messageWidget->addAction(locationInToolbarAction); + locationInToolbarAction->setChecked(false); + return; + } + // do the switching GeneralSettings::setLocationInToolbar(locationInToolbar); if (locationInToolbar) { @@ -870,7 +887,7 @@ void DolphinMainWindow::toggleLocationInToolbar() } } - urlNavigatorWidgetAction->setUrlNavigatorVisible(!locationInToolbar); + urlNavigatorWidgetAction->setUrlNavigatorVisible(locationInToolbar); m_activeViewContainer->urlNavigator()->setUrlEditable(isEditable); if (hasFocus) { // the rest of this method is unneeded perfectionism m_activeViewContainer->urlNavigator()->editor()->lineEdit()->setText(lineEdit->text()); @@ -1756,8 +1773,6 @@ void DolphinMainWindow::setupActions() "Depending on the settings this Widget is blank/invisible.", "Url Navigator (auto-hide)")); actionCollection()->addAction(QStringLiteral("url_navigator"), urlNavigatorWidgetAction); - connect(locationInToolbar, &KToggleAction::triggered, - urlNavigatorWidgetAction, &DolphinUrlNavigatorWidgetAction::setUrlNavigatorVisible); // for context menu QAction* showTarget = actionCollection()->addAction(QStringLiteral("show_target")); diff --git a/src/dolphinmainwindow.h b/src/dolphinmainwindow.h index 529319e2ad..251f50d8d0 100644 --- a/src/dolphinmainwindow.h +++ b/src/dolphinmainwindow.h @@ -65,17 +65,19 @@ public: * having a split view setup, the nonactive view * is usually shown in darker colors. */ - DolphinViewContainer* activeViewContainer() const; + DolphinViewContainer *activeViewContainer() const; /** - * Returns view containers for all tabs - * @param includeInactive When true all view containers available in - * this window are returned. When false the - * view containers of split views that are not - * currently active are ignored. - * Default is true. + * Returns the active view containers of all tabs. + * @see activeViewContainer() + * Use viewContainers() to also include the inactive ones. */ - QVector viewContainers(bool includeInactive = true) const; + QVector activeViewContainers() const; + + /** + * Returns all view containers. + */ + QVector viewContainers() const; /** * Opens each directory in \p dirs in a separate tab. If \a splitView is set, diff --git a/src/dolphinviewcontainer.cpp b/src/dolphinviewcontainer.cpp index da35041878..3cdad13fb7 100644 --- a/src/dolphinviewcontainer.cpp +++ b/src/dolphinviewcontainer.cpp @@ -367,10 +367,13 @@ void DolphinViewContainer::disconnectUrlNavigator() updateNavigatorWidgetVisibility(); } -void DolphinViewContainer::showMessage(const QString& msg, MessageType type) +KMessageWidget *DolphinViewContainer::showMessage(const QString& msg, MessageType type) { if (msg.isEmpty()) { - return; + return m_messageWidget; + } + for (auto action : m_messageWidget->actions()) { + m_messageWidget->removeAction(action); } m_messageWidget->setText(msg); @@ -396,6 +399,7 @@ void DolphinViewContainer::showMessage(const QString& msg, MessageType type) m_messageWidget->hide(); } m_messageWidget->animatedShow(); + return m_messageWidget; } void DolphinViewContainer::readSettings() diff --git a/src/dolphinviewcontainer.h b/src/dolphinviewcontainer.h index 822d8072d1..77c734949e 100644 --- a/src/dolphinviewcontainer.h +++ b/src/dolphinviewcontainer.h @@ -140,8 +140,9 @@ public: /** * Shows the message \msg with the given type non-modal above * the view-content. + * @return the KMessageWidget used to show the message */ - void showMessage(const QString& msg, MessageType type); + KMessageWidget *showMessage(const QString& msg, MessageType type); /** * Refreshes the view container to get synchronized with the (updated) Dolphin settings.