From 63f4981fe01d88b2ef1b27e0577d7f5d4c8cc485 Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Mon, 9 Nov 2020 14:25:15 +0100 Subject: [PATCH] Adress the third round of Angelaccio's review comments Additionally remove some redundant code concerning UrlNavigator visuals. --- src/dolphinnavigatorswidgetaction.cpp | 4 ++-- src/dolphintabpage.cpp | 9 +++------ src/dolphintabpage.h | 2 +- src/dolphintabwidget.cpp | 2 +- src/dolphinurlnavigator.cpp | 4 +--- src/dolphinurlnavigator.h | 6 +++++- src/dolphinurlnavigatorscontroller.cpp | 2 ++ src/dolphinviewcontainer.cpp | 5 +++-- 8 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/dolphinnavigatorswidgetaction.cpp b/src/dolphinnavigatorswidgetaction.cpp index 84f52279a5..984e1f35c8 100644 --- a/src/dolphinnavigatorswidgetaction.cpp +++ b/src/dolphinnavigatorswidgetaction.cpp @@ -201,7 +201,7 @@ QWidget *DolphinNavigatorsWidgetAction::createNavigatorWidget(Side side) const auto emptyTrashButton = newEmptyTrashButton(urlNavigator, navigatorWidget); layout->addWidget(emptyTrashButton); - connect(urlNavigator, &KUrlNavigator::urlChanged, [this]() { + connect(urlNavigator, &KUrlNavigator::urlChanged, this, [this]() { // We have to wait for DolphinUrlNavigator::sizeHint() to update which // happens a little bit later than when urlChanged is emitted. this->m_adjustSpacingTimer->start(); @@ -231,7 +231,7 @@ QPushButton *DolphinNavigatorsWidgetAction::newEmptyTrashButton(const DolphinUrl connect(&Trash::instance(), &Trash::emptinessChanged, emptyTrashButton, &QPushButton::setDisabled); emptyTrashButton->hide(); - connect(urlNavigator, &KUrlNavigator::urlChanged, [emptyTrashButton, urlNavigator]() { + connect(urlNavigator, &KUrlNavigator::urlChanged, this, [emptyTrashButton, urlNavigator]() { emptyTrashButton->setVisible(urlNavigator->locationUrl().scheme() == QLatin1String("trash")); }); emptyTrashButton->setDisabled(Trash::isEmpty()); diff --git a/src/dolphintabpage.cpp b/src/dolphintabpage.cpp index d196508a87..7f945dce27 100644 --- a/src/dolphintabpage.cpp +++ b/src/dolphintabpage.cpp @@ -159,11 +159,9 @@ void DolphinTabPage::connectNavigators(DolphinNavigatorsWidgetAction *navigators { m_navigatorsWidget = navigatorsWidget; auto primaryNavigator = navigatorsWidget->primaryUrlNavigator(); - primaryNavigator->setActive(m_primaryViewActive); m_primaryViewContainer->connectUrlNavigator(primaryNavigator); if (m_splitViewEnabled) { auto secondaryNavigator = navigatorsWidget->secondaryUrlNavigator(); - secondaryNavigator->setActive(!m_primaryViewActive); m_secondaryViewContainer->connectUrlNavigator(secondaryNavigator); } resizeNavigators(); @@ -178,12 +176,13 @@ void DolphinTabPage::disconnectNavigators() } } -bool DolphinTabPage::eventFilter(QObject */* watched */, QEvent *event) +bool DolphinTabPage::eventFilter(QObject *watched, QEvent *event) { if (event->type() == QEvent::Resize && m_navigatorsWidget) { resizeNavigators(); + return false; } - return false; + return QWidget::eventFilter(watched, event); } void DolphinTabPage::resizeNavigators() const @@ -292,11 +291,9 @@ void DolphinTabPage::restoreState(const QByteArray& state) stream >> m_primaryViewActive; if (m_primaryViewActive) { m_primaryViewContainer->setActive(true); - m_navigatorsWidget->primaryUrlNavigator()->setActive(true); } else { Q_ASSERT(m_splitViewEnabled); m_secondaryViewContainer->setActive(true); - m_navigatorsWidget->primaryUrlNavigator()->setActive(false); } QByteArray splitterState; diff --git a/src/dolphintabpage.h b/src/dolphintabpage.h index 650594214a..b874d128f6 100644 --- a/src/dolphintabpage.h +++ b/src/dolphintabpage.h @@ -83,7 +83,7 @@ public: /** * Calls resizeNavigators() when a watched object is resized. */ - bool eventFilter(QObject */* watched */, QEvent *event) override; + bool eventFilter(QObject *watched, QEvent *event) override; /** * Notify the connected DolphinNavigatorsWidgetAction of geometry changes which it diff --git a/src/dolphintabwidget.cpp b/src/dolphintabwidget.cpp index 94cdc627b7..da8f76d7c4 100644 --- a/src/dolphintabwidget.cpp +++ b/src/dolphintabwidget.cpp @@ -406,7 +406,7 @@ void DolphinTabWidget::currentTabChanged(int index) tabPage->setActive(true); tabPage->connectNavigators(m_navigatorsWidget); m_navigatorsWidget->setSecondaryNavigatorVisible(tabPage->splitViewEnabled()); - m_lastViewedTab = tabPageAt(index); + m_lastViewedTab = tabPage; } void DolphinTabWidget::tabInserted(int index) diff --git a/src/dolphinurlnavigator.cpp b/src/dolphinurlnavigator.cpp index f24cf2e067..1dfe5420f1 100644 --- a/src/dolphinurlnavigator.cpp +++ b/src/dolphinurlnavigator.cpp @@ -47,10 +47,8 @@ DolphinUrlNavigator::DolphinUrlNavigator(const QUrl &url, QWidget *parent) : DolphinUrlNavigatorsController::registerDolphinUrlNavigator(this); - connect(this, &DolphinUrlNavigator::returnPressed, + connect(this, &KUrlNavigator::returnPressed, this, &DolphinUrlNavigator::slotReturnPressed); - connect(editor(), &KUrlComboBox::completionModeChanged, - DolphinUrlNavigatorsController::setCompletionMode); } DolphinUrlNavigator::~DolphinUrlNavigator() diff --git a/src/dolphinurlnavigator.h b/src/dolphinurlnavigator.h index a154287995..9bcc32b4d1 100644 --- a/src/dolphinurlnavigator.h +++ b/src/dolphinurlnavigator.h @@ -15,8 +15,9 @@ * * Makes sure that Dolphin preferences and settings are * applied to all constructed DolphinUrlNavigators. - * * @see KUrlNavigator + * + * To apply changes to all instances of this class @see DolphinUrlNavigatorsController. */ class DolphinUrlNavigator : public KUrlNavigator { @@ -55,6 +56,9 @@ public: /** * Retrieve the visual state of this DolphinUrlNavigator. * If two DolphinUrlNavigators have the same visual state they should look identical. + * + * @return a copy of the visualState of this object. Ownership of this copy is transferred + * to the caller via std::unique_ptr. */ std::unique_ptr visualState() const; /** diff --git a/src/dolphinurlnavigatorscontroller.cpp b/src/dolphinurlnavigatorscontroller.cpp index 78fecd18a4..59e1d63561 100644 --- a/src/dolphinurlnavigatorscontroller.cpp +++ b/src/dolphinurlnavigatorscontroller.cpp @@ -46,6 +46,8 @@ bool DolphinUrlNavigatorsController::placesSelectorVisible() void DolphinUrlNavigatorsController::registerDolphinUrlNavigator(DolphinUrlNavigator *dolphinUrlNavigator) { s_instances.push_front(dolphinUrlNavigator); + connect(dolphinUrlNavigator->editor(), &KUrlComboBox::completionModeChanged, + DolphinUrlNavigatorsController::setCompletionMode); } void DolphinUrlNavigatorsController::unregisterDolphinUrlNavigator(DolphinUrlNavigator *dolphinUrlNavigator) diff --git a/src/dolphinviewcontainer.cpp b/src/dolphinviewcontainer.cpp index 9761f108fc..0fe8ee9d3d 100644 --- a/src/dolphinviewcontainer.cpp +++ b/src/dolphinviewcontainer.cpp @@ -291,11 +291,11 @@ void DolphinViewContainer::connectUrlNavigator(DolphinUrlNavigator *urlNavigator Q_CHECK_PTR(m_view); urlNavigator->setLocationUrl(m_view->url()); - urlNavigator->setActive(isActive()); if (m_urlNavigatorVisualState) { urlNavigator->setVisualState(*m_urlNavigatorVisualState.get()); m_urlNavigatorVisualState.reset(); } + urlNavigator->setActive(isActive()); connect(m_view, &DolphinView::urlChanged, urlNavigator, &DolphinUrlNavigator::setLocationUrl); @@ -307,7 +307,8 @@ void DolphinViewContainer::connectUrlNavigator(DolphinUrlNavigator *urlNavigator this, &DolphinViewContainer::slotUrlNavigatorLocationAboutToBeChanged); connect(urlNavigator, &DolphinUrlNavigator::urlSelectionRequested, this, &DolphinViewContainer::slotUrlSelectionRequested); - connect(urlNavigator, &DolphinUrlNavigator::urlsDropped, this, [=](const QUrl &destination, QDropEvent *event) { + connect(urlNavigator, &DolphinUrlNavigator::urlsDropped, + this, [=](const QUrl &destination, QDropEvent *event) { m_view->dropUrls(destination, event, urlNavigator->dropWidget()); });