From 37327c9b0aae112c5890703cba1f0157043007e0 Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Sun, 20 Sep 2020 18:53:59 +0200 Subject: Make UrlNavigators in the toolbar the only option The UrlNavigators will be automatically added to the toolbar. The Sort By action is removed from the default toolbar to make space. Remove all options to have UrlNavigators outside the toolbar and remove those code paths. Make it so the new NavigatorsWidgetAction contains two UrlNavigators when in split view mode. Spacing was also added to align these UrlNavigators with the ViewContainers when enough space is available. Force the toolbar to be either at the top or bottom of the window. Set a sane sizeHint for DolphinUrlNavigator. It would be better to do this in KUrlNavigator in the future. This commit also contains a changes which should be moved to a separate merge requests before this gets merged: - Add an expansion animation when split view is enabled by the user --- src/dolphintabpage.h | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) (limited to 'src/dolphintabpage.h') diff --git a/src/dolphintabpage.h b/src/dolphintabpage.h index 74344acd1..6a8801edd 100644 --- a/src/dolphintabpage.h +++ b/src/dolphintabpage.h @@ -11,8 +11,10 @@ #include #include -class QSplitter; +class DolphinNavigatorsWidgetAction; class DolphinViewContainer; +class QSplitter; +class QVariantAnimation; class KFileItemList; class DolphinTabPage : public QWidget @@ -66,6 +68,30 @@ public: */ int selectedItemsCount() const; + /** + * Connects a navigatorsWidget to this. It will be connected to the DolphinViewContainers + * managed by this tab. For alignment purposes this will from now on notify the + * navigatorsWidget when this tab or its viewContainers are resized. + */ + void connectNavigators(DolphinNavigatorsWidgetAction *navigatorsWidget); + + /** + * Makes it so this tab and its DolphinViewContainers aren't controlled by any + * UrlNavigators anymore. + */ + void disconnectNavigators(); + + /** + * Calls resizeNavigators() when a watched object is resized. + */ + bool eventFilter(QObject */* watched */, QEvent *event) override; + + /** + * Notify the connected DolphinNavigatorsWidgetAction of geometry changes which it + * needs for visual alignment. + */ + void resizeNavigators() const; + /** * Marks the items indicated by \p urls to get selected after the * directory DolphinView::url() has been loaded. Note that nothing @@ -80,14 +106,6 @@ public: */ void markUrlAsCurrent(const QUrl& url); - /** - * Sets the places selector visible, if \a visible is true. - * The places selector allows to select the places provided - * by the places model passed in the constructor. Per default - * the places selector is visible. - */ - void setPlacesSelectorVisible(bool visible); - /** * Refreshes the views of the main window by recreating them according to * the given Dolphin settings. @@ -125,6 +143,7 @@ public: signals: void activeViewChanged(DolphinViewContainer* viewContainer); void activeViewUrlChanged(const QUrl& url); + void splitterMoved(int pos, int index); private slots: /** @@ -153,8 +172,10 @@ private: private: QSplitter* m_splitter; + QPointer m_navigatorsWidget; QPointer m_primaryViewContainer; QPointer m_secondaryViewContainer; + QPointer m_splitViewAnimation; bool m_primaryViewActive; bool m_splitViewEnabled; -- cgit v1.3 From a418d6229e31dac254660da2a417b4306f066ae3 Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Wed, 28 Oct 2020 17:52:29 +0100 Subject: Fix a crash and extract unrelated changes The secondary UrlNavigator is now created when and only when: - split view mode is activated for the active tab OR - switching to a tab that has split view already enabled. This fixes a crash that occurs when the setting to always start in split view mode is enabled. An animation for activating split view is also removed from this and moved into a separate MR. Another unrelated name change left over from a previous commit (viewContainers() -> activeViewContainers()) is dropped. --- src/dolphinbookmarkhandler.cpp | 2 +- src/dolphinmainwindow.cpp | 3 ++- src/dolphinmainwindow.h | 6 +++--- src/dolphinnavigatorswidgetaction.h | 8 +++++++- src/dolphintabpage.cpp | 41 +------------------------------------ src/dolphintabpage.h | 2 -- src/dolphintabwidget.cpp | 8 ++++---- 7 files changed, 18 insertions(+), 52 deletions(-) (limited to 'src/dolphintabpage.h') diff --git a/src/dolphinbookmarkhandler.cpp b/src/dolphinbookmarkhandler.cpp index 576a9314b..be4f447d8 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->activeViewContainers(); + const auto viewContainers = m_mainWindow->viewContainers(); QList bookmarks; bookmarks.reserve(viewContainers.size()); for (const auto viewContainer : viewContainers) { diff --git a/src/dolphinmainwindow.cpp b/src/dolphinmainwindow.cpp index 6a93de078..8b389ce9b 100644 --- a/src/dolphinmainwindow.cpp +++ b/src/dolphinmainwindow.cpp @@ -213,7 +213,7 @@ DolphinMainWindow::~DolphinMainWindow() { } -QVector DolphinMainWindow::activeViewContainers() const +QVector DolphinMainWindow::viewContainers() const { QVector viewContainers; @@ -2178,6 +2178,7 @@ void DolphinMainWindow::connectViewSignals(DolphinViewContainer* container) auto navigators = static_cast (actionCollection()->action(QStringLiteral("url_navigators"))); + KUrlNavigator *navigator = m_tabWidget->currentTabPage()->primaryViewActive() ? navigators->primaryUrlNavigator() : navigators->secondaryUrlNavigator(); diff --git a/src/dolphinmainwindow.h b/src/dolphinmainwindow.h index 173e017c9..29ab6326d 100644 --- a/src/dolphinmainwindow.h +++ b/src/dolphinmainwindow.h @@ -65,12 +65,12 @@ public: * having a split view setup, the nonactive view * is usually shown in darker colors. */ - DolphinViewContainer *activeViewContainer() const; + DolphinViewContainer* activeViewContainer() const; /** - * Returns the active view containers of all tabs. + * Returns view container for all tabs */ - QVector activeViewContainers() const; + QVector viewContainers() const; /** * Opens each directory in \p dirs in a separate tab. If \a splitView is set, diff --git a/src/dolphinnavigatorswidgetaction.h b/src/dolphinnavigatorswidgetaction.h index c1808d68e..8046ce2dc 100644 --- a/src/dolphinnavigatorswidgetaction.h +++ b/src/dolphinnavigatorswidgetaction.h @@ -57,7 +57,13 @@ public: bool addToToolbarAndSave(KXmlGuiWindow *mainWindow); /** - * Different to the primary UrlNavigator, the secondary UrlNavigator is only created on-demand. + * The secondary UrlNavigator is only created on-demand. Such an action is not necessary + * for the primary UrlNavigator which is created preemptively. + * + * This method should preferably only be called when: + * - Split view is activated in the active tab + * OR + * - A switch to a tab that is already in split view mode is occuring */ void createSecondaryUrlNavigator(); diff --git a/src/dolphintabpage.cpp b/src/dolphintabpage.cpp index d2fd1d143..d196508a8 100644 --- a/src/dolphintabpage.cpp +++ b/src/dolphintabpage.cpp @@ -9,7 +9,6 @@ #include "dolphin_generalsettings.h" #include "dolphinviewcontainer.h" -#include #include #include @@ -70,7 +69,6 @@ void DolphinTabPage::setSplitViewEnabled(bool enabled, const QUrl &secondaryUrl) m_splitViewEnabled = enabled; if (enabled) { - int splitterTotalWidth = m_splitter->width(); const QUrl& url = (secondaryUrl.isEmpty()) ? m_primaryViewContainer->url() : secondaryUrl; m_secondaryViewContainer = createViewContainer(url); @@ -84,33 +82,8 @@ void DolphinTabPage::setSplitViewEnabled(bool enabled, const QUrl &secondaryUrl) m_splitter->addWidget(m_secondaryViewContainer); m_secondaryViewContainer->installEventFilter(this); - m_secondaryViewContainer->setActive(true); - - // opening animation - m_splitter->widget(1)->setMinimumWidth(1); - const QList splitterSizes = {m_splitter->width(), 0}; - m_splitter->setSizes(splitterSizes); - - // TODO: This is only here to test the robustness of DolphinNavigatorsWidgetAction! I still have to move it to another merge request! - m_splitViewAnimation = new QVariantAnimation(m_splitter); - m_splitViewAnimation->setDuration(200); // TODO: where do I get the animation times from again? - m_splitViewAnimation->setStartValue(splitterTotalWidth); - m_splitViewAnimation->setEndValue(splitterTotalWidth / 2); - m_splitViewAnimation->setEasingCurve(QEasingCurve::OutCubic); - - connect(m_splitViewAnimation, &QVariantAnimation::valueChanged, [=]() { - if (m_splitter->count() != 2) { - return; - } - int value = m_splitViewAnimation->currentValue().toInt(); - const QList splitterSizes = {value, m_splitter->width() - value}; - m_splitter->setSizes(splitterSizes); - if (value == m_splitViewAnimation->endValue().toInt()) { - m_splitter->widget(1)->setMinimumWidth(m_splitter->widget(1)->minimumSizeHint().width()); - } - }); - m_splitViewAnimation->start(QAbstractAnimation::DeleteWhenStopped); m_secondaryViewContainer->show(); + m_secondaryViewContainer->setActive(true); } else { m_navigatorsWidget->setSecondaryNavigatorVisible(false); m_secondaryViewContainer->disconnectUrlNavigator(); @@ -144,10 +117,6 @@ void DolphinTabPage::setSplitViewEnabled(bool enabled, const QUrl &secondaryUrl) m_primaryViewContainer->setActive(true); view->close(); view->deleteLater(); - if (m_splitViewAnimation) { - delete m_splitViewAnimation; - m_splitter->widget(0)->setMinimumWidth(m_splitter->widget(0)->minimumSizeHint().width()); - } } } } @@ -194,10 +163,6 @@ void DolphinTabPage::connectNavigators(DolphinNavigatorsWidgetAction *navigators m_primaryViewContainer->connectUrlNavigator(primaryNavigator); if (m_splitViewEnabled) { auto secondaryNavigator = navigatorsWidget->secondaryUrlNavigator(); - if (!secondaryNavigator) { - navigatorsWidget->createSecondaryUrlNavigator(); - secondaryNavigator = navigatorsWidget->secondaryUrlNavigator(); - } secondaryNavigator->setActive(!m_primaryViewActive); m_secondaryViewContainer->connectUrlNavigator(secondaryNavigator); } @@ -334,10 +299,6 @@ void DolphinTabPage::restoreState(const QByteArray& state) m_navigatorsWidget->primaryUrlNavigator()->setActive(false); } - if (m_splitViewAnimation) { - delete m_splitViewAnimation; - m_splitter->widget(0)->setMinimumWidth(m_splitter->widget(0)->minimumSizeHint().width()); - } QByteArray splitterState; stream >> splitterState; m_splitter->restoreState(splitterState); diff --git a/src/dolphintabpage.h b/src/dolphintabpage.h index 6a8801edd..650594214 100644 --- a/src/dolphintabpage.h +++ b/src/dolphintabpage.h @@ -14,7 +14,6 @@ class DolphinNavigatorsWidgetAction; class DolphinViewContainer; class QSplitter; -class QVariantAnimation; class KFileItemList; class DolphinTabPage : public QWidget @@ -175,7 +174,6 @@ private: QPointer m_navigatorsWidget; QPointer m_primaryViewContainer; QPointer m_secondaryViewContainer; - QPointer m_splitViewAnimation; bool m_primaryViewActive; bool m_splitViewEnabled; diff --git a/src/dolphintabwidget.cpp b/src/dolphintabwidget.cpp index a09a769d3..eb3f741ee 100644 --- a/src/dolphintabwidget.cpp +++ b/src/dolphintabwidget.cpp @@ -128,12 +128,9 @@ bool DolphinTabWidget::isUrlOpen(const QUrl &url) const void DolphinTabWidget::openNewActivatedTab() { std::unique_ptr oldNavigatorState; - if (currentTabPage()->primaryViewActive()) { + if (currentTabPage()->primaryViewActive() || !m_navigatorsWidget->secondaryUrlNavigator()) { oldNavigatorState = m_navigatorsWidget->primaryUrlNavigator()->visualState(); } else { - if (!m_navigatorsWidget->secondaryUrlNavigator()) { - m_navigatorsWidget->createSecondaryUrlNavigator(); - } oldNavigatorState = m_navigatorsWidget->secondaryUrlNavigator()->visualState(); } @@ -401,6 +398,9 @@ void DolphinTabWidget::currentTabChanged(int index) m_lastViewedTab->disconnectNavigators(); m_lastViewedTab->setActive(false); } + if (tabPage->splitViewEnabled() && !m_navigatorsWidget->secondaryUrlNavigator()) { + m_navigatorsWidget->createSecondaryUrlNavigator(); + } DolphinViewContainer* viewContainer = tabPage->activeViewContainer(); Q_EMIT activeViewChanged(viewContainer); Q_EMIT currentUrlChanged(viewContainer->url()); -- cgit v1.3 From 63f4981fe01d88b2ef1b27e0577d7f5d4c8cc485 Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Mon, 9 Nov 2020 14:25:15 +0100 Subject: 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(-) (limited to 'src/dolphintabpage.h') diff --git a/src/dolphinnavigatorswidgetaction.cpp b/src/dolphinnavigatorswidgetaction.cpp index 84f52279a..984e1f35c 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 d196508a8..7f945dce2 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 650594214..b874d128f 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 94cdc627b..da8f76d7c 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 f24cf2e06..1dfe5420f 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 a15428799..9bcc32b4d 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 78fecd18a..59e1d6356 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 9761f108f..0fe8ee9d3 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()); }); -- cgit v1.3