From d21984ba5bcd7b90f0ecdd22f4d31176d9c2ce2d Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Wed, 29 May 2024 13:23:08 +0200 Subject: [PATCH] Improve Filelight installation UX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit pressing the free space button when Filelight is not installed would show a singular action called "Install Filelight to View Disk Usage Statistics…". Pressing this button would open the store page for Filelight. This is an okay user experience, but we can do better. This commit makes it so pressing the free space button when Filelight is not installed shows an attractive UI that makes clear that freeing up disk space can be accomplished nicely by installing Filelight. The "Install Filelight" button on this UI is connected to PackageKit directly, so we do not need to show a separate store like Discover and instead trigger ann installation right then and there. Installation failure or success is then shown within Dolphin as a passive notification. CCBUG: 477739 --- CMakeLists.txt | 2 +- src/CMakeLists.txt | 10 +++ src/config-dolphin.h.cmake | 4 ++ src/dolphinpackagemanager.cpp | 98 ++++++++++++++++++++++++++++ src/dolphinpackagemanager.h | 71 ++++++++++++++++++++ src/dolphinviewcontainer.cpp | 1 + src/global.h | 4 +- src/statusbar/dolphinstatusbar.cpp | 1 + src/statusbar/dolphinstatusbar.h | 7 ++ src/statusbar/statusbarspaceinfo.cpp | 69 ++++++++++++++++++-- src/statusbar/statusbarspaceinfo.h | 10 +++ 11 files changed, 269 insertions(+), 8 deletions(-) create mode 100644 src/dolphinpackagemanager.cpp create mode 100644 src/dolphinpackagemanager.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a4018351b..d7f1cb57af 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -103,7 +103,7 @@ find_package(Phonon4Qt6 CONFIG REQUIRED) find_package(PackageKitQt6) set_package_properties(PackageKitQt6 PROPERTIES DESCRIPTION "Software Manager integration" - TYPE OPTIONAL + TYPE RECOMMENDED PURPOSE "Used in the service menu installer" ) if(PackageKitQt6_FOUND) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a94da9d5a5..4baaf52993 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,5 +1,6 @@ include(ECMAddAppIcon) +set(FILELIGHT_PACKAGE_NAME "filelight") configure_file(config-dolphin.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/config-dolphin.h) add_definitions( @@ -273,6 +274,7 @@ target_sources(dolphinstatic PRIVATE dolphincontextmenu.cpp dolphinnavigatorswidgetaction.cpp dolphintabbar.cpp + dolphinpackagemanager.cpp dolphinplacesmodelsingleton.cpp dolphinrecenttabsmenu.cpp dolphintabpage.cpp @@ -333,6 +335,7 @@ target_sources(dolphinstatic PRIVATE dolphincontextmenu.h dolphinnavigatorswidgetaction.h dolphintabbar.h + dolphinpackagemanager.h dolphinplacesmodelsingleton.h dolphinrecenttabsmenu.h dolphintabpage.h @@ -458,6 +461,13 @@ if (HAVE_PLASMA_ACTIVITIES) ) endif() +if(HAVE_PACKAGEKIT) + target_link_libraries( + dolphinstatic + PK::packagekitqt6 + ) +endif() + if (HAVE_KUSERFEEDBACK) target_link_libraries( dolphinstatic diff --git a/src/config-dolphin.h.cmake b/src/config-dolphin.h.cmake index 797ea38c59..78fce45c23 100644 --- a/src/config-dolphin.h.cmake +++ b/src/config-dolphin.h.cmake @@ -1,6 +1,10 @@ +/** Set whether to build Dolphin with support for these technologies or not. */ #cmakedefine01 HAVE_BALOO #cmakedefine01 HAVE_PLASMA_ACTIVITIES #cmakedefine01 HAVE_KUSERFEEDBACK #cmakedefine01 HAVE_PACKAGEKIT #cmakedefine01 HAVE_TERMINAL #cmakedefine01 HAVE_X11 + +/** The name of the KDE Filelight package. */ +#cmakedefine FILELIGHT_PACKAGE_NAME "@FILELIGHT_PACKAGE_NAME@" diff --git a/src/dolphinpackagemanager.cpp b/src/dolphinpackagemanager.cpp new file mode 100644 index 0000000000..0768748ec9 --- /dev/null +++ b/src/dolphinpackagemanager.cpp @@ -0,0 +1,98 @@ +/* + This file is part of the KDE project + SPDX-FileCopyrightText: 2024 Felix Ernst + + SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL +*/ + +#include "dolphinpackagemanager.h" + +#include + +#if HAVE_PACKAGEKIT +#include +#else +#include +#endif + +#include +#include + +DolphinPackageManager::DolphinPackageManager(QObject *parent) + : QObject(parent) +{ +} + +void DolphinPackageManager::install(const QString &packageName, const QUrl &fallBackInstallationPageUrl, std::function isPackageInstalledCheck) +{ + Q_ASSERT_X(m_packageName.isEmpty(), "install", "Reusing a DolphinPackageManager object has not been implemented and can lead to conflicts."); + if (isPackageInstalledCheck()) { + Q_EMIT success(); + return; + } + + m_packageName = packageName; +#if HAVE_PACKAGEKIT + Q_UNUSED(fallBackInstallationPageUrl) + PackageKit::Daemon::setHints(PackageKit::Daemon::hints() + QStringList{QStringLiteral("interactive=true")}); + const PackageKit::Transaction *resolveTransaction = PackageKit::Daemon::resolve(packageName); + + connect(resolveTransaction, &PackageKit::Transaction::errorCode, this, &DolphinPackageManager::slotInstallationFailed); + connect(resolveTransaction, &PackageKit::Transaction::finished, this, [this, packageName]() { // Will be disconnected if we find a package. + slotInstallationFailed(PackageKit::Transaction::ErrorPackageNotFound, + i18nc("@info:shell about system packages", "Could not find package %1.", packageName)); + }); + connect(resolveTransaction, + &PackageKit::Transaction::package, + this, + [this, resolveTransaction](PackageKit::Transaction::Info /* info */, const QString &packageId) { + disconnect(resolveTransaction, nullptr, this, nullptr); // We only care about the first package. + PackageKit::Transaction *installTransaction = PackageKit::Daemon::installPackage(packageId); + connect(installTransaction, + &PackageKit::Transaction::errorCode, + this, + [installTransaction, this](PackageKit::Transaction::Error error, const QString &details) { + disconnect(installTransaction, nullptr, this, nullptr); // We only want to emit failure() or success() once. + slotInstallationFailed(error, details); + }); + connect(installTransaction, + &PackageKit::Transaction::finished, + this, + [installTransaction, this](const PackageKit::Transaction::Exit status, uint /* runtime */) { + disconnect(installTransaction, nullptr, this, nullptr); // We only want to emit failure() or success() once. + if (status == PackageKit::Transaction::ExitSuccess) { + Q_EMIT success(); + deleteLater(); + } else { + slotInstallationFailed(PackageKit::Transaction::ErrorUnknown, + i18nc("@info %1 is error code", + "Installation exited without reporting success. (%1)", + QMetaEnum::fromType().valueToKey(status))); + } + }); + }); +#else + Q_UNUSED(packageName) + QDesktopServices::openUrl(fallBackInstallationPageUrl); + auto waitForSuccess = new QTimer(this); + connect(waitForSuccess, &QTimer::timeout, this, [isPackageInstalledCheck, this, waitForSuccess]() { + if (isPackageInstalledCheck()) { + Q_EMIT success(); + deleteLater(); + } + }); + waitForSuccess->start(3000); +#endif +} + +#if HAVE_PACKAGEKIT +void DolphinPackageManager::slotInstallationFailed(PackageKit::Transaction::Error error, const QString &details) +{ + Q_EMIT failure(xi18nc("@info:shell %1 is package name, %2 is error message, %3 is error e.g. 'ErrorNoNetwork'", + "Installing %1 failed: %2 (%3)Please try installing %1 manually instead.", + m_packageName, + details, + QMetaEnum::fromType().valueToKey(error))); + deleteLater(); +} +#endif diff --git a/src/dolphinpackagemanager.h b/src/dolphinpackagemanager.h new file mode 100644 index 0000000000..0431c12c07 --- /dev/null +++ b/src/dolphinpackagemanager.h @@ -0,0 +1,71 @@ +/* + This file is part of the KDE project + SPDX-FileCopyrightText: 2024 Felix Ernst + + SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL +*/ + +#ifndef DOLPHINPACKAGEMANAGER_H +#define DOLPHINPACKAGEMANAGER_H + +#include "config-dolphin.h" + +#if HAVE_PACKAGEKIT +#include +#endif + +#include + +/** + * @brief A class providing a simple API to install packages. + */ +class DolphinPackageManager : public QObject +{ + Q_OBJECT + +public: + explicit DolphinPackageManager(QObject *parent = nullptr); + + /** + * @brief Installs a system package. + * + * @param packageName A name that can be resolved to a package. + * @param fallBackInstallationPageUrl This url will be opened if Dolphin was installed without the PackageKit library. A good choice for this parameter + * is an appstream url that will be opened in a software store (like Discover) e.g. "appstream://org.kde.kio.admin". + * The user is then expected to install the package themselves and success() will be emitted once that finished. + * @param isPackageInstalledCheck A function that can be regularly checked to determine if the installation was already successful. + * + * Calling this method will lead to either emitting the success() or the failure() signal. Once either is emitted this object will delete itself. + * This generally happens asynchronously with rare exceptions, so connect to the signals before calling this method. + */ + void install(const QString &packageName, const QUrl &fallBackInstallationPageUrl, std::function isPackageInstalledCheck); + +Q_SIGNALS: + /** + * Emitted when this object assumes that the package was successfully installed. + * This object will delete itself after emitting this signal. + * + * @note This does not always mean that the isPackageInstalledCheck of DolphinPackageManager::install() also passes. + */ + void success(); + + /** + * Emitted when the installation finally failed. + * This object will delete itself after emitting this signal. + * + * Be sure to show the @p userFacingErrorMessage to the user or they won't know what happened and can't write useful bug reports. + */ + void failure(const QString &userFacingErrorMessage); + +#if HAVE_PACKAGEKIT +private Q_SLOTS: + /** Creates a nice user-facing error message from its parameters and then emits failure() and deletes this object. */ + void slotInstallationFailed(PackageKit::Transaction::Error error, const QString &details); +#endif + +private: + /** The name of the package that is currently being installed. */ + QString m_packageName; +}; + +#endif // DOLPHINPACKAGEMANAGER_H diff --git a/src/dolphinviewcontainer.cpp b/src/dolphinviewcontainer.cpp index fbad258ace..0828c8d205 100644 --- a/src/dolphinviewcontainer.cpp +++ b/src/dolphinviewcontainer.cpp @@ -168,6 +168,7 @@ DolphinViewContainer::DolphinViewContainer(const QUrl &url, QWidget *parent) }); connect(m_statusBar, &DolphinStatusBar::stopPressed, this, &DolphinViewContainer::stopDirectoryLoading); connect(m_statusBar, &DolphinStatusBar::zoomLevelChanged, this, &DolphinViewContainer::slotStatusBarZoomLevelChanged); + connect(m_statusBar, &DolphinStatusBar::showMessage, this, &DolphinViewContainer::showMessage); m_statusBarTimer = new QTimer(this); m_statusBarTimer->setSingleShot(true); diff --git a/src/global.h b/src/global.h index dd07f9170f..9161ed877a 100644 --- a/src/global.h +++ b/src/global.h @@ -49,8 +49,8 @@ QPair sortOrderForUrl(QUrl &url); /** * TODO: Move this somewhere global to all KDE apps, not just Dolphin */ -const int VERTICAL_SPACER_HEIGHT = 12; -const int LAYOUT_SPACING_SMALL = 2; +constexpr int VERTICAL_SPACER_HEIGHT = 12; +constexpr int LAYOUT_SPACING_SMALL = 2; } enum Animated { WithAnimation, WithoutAnimation }; diff --git a/src/statusbar/dolphinstatusbar.cpp b/src/statusbar/dolphinstatusbar.cpp index 5a3aa58577..cb8c422456 100644 --- a/src/statusbar/dolphinstatusbar.cpp +++ b/src/statusbar/dolphinstatusbar.cpp @@ -70,6 +70,7 @@ DolphinStatusBar::DolphinStatusBar(QWidget *parent) // Initialize space information m_spaceInfo = new StatusBarSpaceInfo(contentsContainer); + connect(m_spaceInfo, &StatusBarSpaceInfo::showMessage, this, &DolphinStatusBar::showMessage); // Initialize progress information m_stopButton = new QToolButton(contentsContainer); diff --git a/src/statusbar/dolphinstatusbar.h b/src/statusbar/dolphinstatusbar.h index 8abbed66be..a620a01178 100644 --- a/src/statusbar/dolphinstatusbar.h +++ b/src/statusbar/dolphinstatusbar.h @@ -9,6 +9,8 @@ #include "animatedheightwidget.h" +#include + #include class QUrl; @@ -96,6 +98,11 @@ Q_SIGNALS: void zoomLevelChanged(int zoomLevel); + /** + * Requests for @p message with the given @p messageType to be shown to the user in a non-modal way. + */ + void showMessage(const QString &message, KMessageWidget::MessageType messageType); + protected: void contextMenuEvent(QContextMenuEvent *event) override; void paintEvent(QPaintEvent *paintEvent) override; diff --git a/src/statusbar/statusbarspaceinfo.cpp b/src/statusbar/statusbarspaceinfo.cpp index 9df4164676..5ea62357a8 100644 --- a/src/statusbar/statusbarspaceinfo.cpp +++ b/src/statusbar/statusbarspaceinfo.cpp @@ -6,6 +6,9 @@ #include "statusbarspaceinfo.h" +#include "config-dolphin.h" +#include "dolphinpackagemanager.h" +#include "global.h" #include "spaceinfoobserver.h" #include @@ -16,14 +19,19 @@ #include #include +#include #include #include +#include #include #include +#include +#include StatusBarSpaceInfo::StatusBarSpaceInfo(QWidget *parent) : QWidget(parent) , m_observer(nullptr) + , m_installFilelightWidgetAction{nullptr} { m_capacityBar = new KCapacityBar(KCapacityBar::DrawTextInline, this); m_textInfoButton = new QToolButton(this); @@ -116,17 +124,68 @@ void StatusBarSpaceInfo::updateMenu() const KService::Ptr kdiskfree = KService::serviceByDesktopName(QStringLiteral("org.kde.kdf")); if (!filelight && !kdiskfree) { - QAction *installFilelight = - m_buttonMenu->addAction(QIcon::fromTheme(QStringLiteral("filelight")), i18n("Install Filelight to View Disk Usage Statistics…")); + // Show an UI to install a tool to free up disk space because this is what a user pressing on a "free space" button would want. + if (!m_installFilelightWidgetAction) { + auto containerWidget = new QWidget{this}; + containerWidget->setContentsMargins(Dolphin::VERTICAL_SPACER_HEIGHT, + Dolphin::VERTICAL_SPACER_HEIGHT, + Dolphin::VERTICAL_SPACER_HEIGHT, + Dolphin::VERTICAL_SPACER_HEIGHT); + auto vLayout = new QVBoxLayout(containerWidget); - connect(installFilelight, &QAction::triggered, this, [] { + auto installFilelightTitle = new QLabel(i18nc("@title", "Free Up Disk Space"), containerWidget); + installFilelightTitle->setAlignment(Qt::AlignCenter); + installFilelightTitle->setTextInteractionFlags(Qt::TextSelectableByMouse | Qt::TextSelectableByKeyboard | Qt::LinksAccessibleByKeyboard); + QFont titleFont{installFilelightTitle->font()}; + titleFont.setPointSize(titleFont.pointSize() + 2); + installFilelightTitle->setFont(titleFont); + vLayout->addWidget(installFilelightTitle); + + vLayout->addSpacing(Dolphin::VERTICAL_SPACER_HEIGHT); + + auto installFilelightBody = + // i18n: The new line ("") tag is only there to format this text visually pleasing, i.e. to avoid having one very long line. + new QLabel(xi18nc("@title", "Install additional software to view disk usage statisticsand identify big files and folders."), + containerWidget); + installFilelightBody->setAlignment(Qt::AlignCenter); + installFilelightBody->setTextInteractionFlags(Qt::TextSelectableByMouse | Qt::TextSelectableByKeyboard | Qt::LinksAccessibleByKeyboard); + vLayout->addWidget(installFilelightBody); + + vLayout->addSpacing(Dolphin::VERTICAL_SPACER_HEIGHT); + + auto installFilelightButton = + new QPushButton(QIcon::fromTheme(QStringLiteral("filelight")), i18nc("@action:button", "Install Filelight"), containerWidget); + installFilelightButton->setFixedWidth(std::max(installFilelightButton->sizeHint().width(), installFilelightTitle->sizeHint().width())); + auto buttonLayout = new QHBoxLayout{containerWidget}; + buttonLayout->addWidget(installFilelightButton, 0, Qt::AlignHCenter); + vLayout->addLayout(buttonLayout); + + // Make sure one Tab press focuses the button after the UI opened. + m_buttonMenu->setFocusProxy(installFilelightButton); + containerWidget->setFocusPolicy(Qt::TabFocus); + containerWidget->setFocusProxy(installFilelightButton); + installFilelightButton->setAccessibleDescription(installFilelightBody->text()); + + connect(installFilelightButton, &QAbstractButton::clicked, this, [this] { #ifdef Q_OS_WIN QDesktopServices::openUrl(QUrl("https://apps.kde.org/filelight")); #else - QDesktopServices::openUrl(QUrl("appstream://org.kde.filelight.desktop")); + auto packageInstaller = new DolphinPackageManager(this); + connect(packageInstaller, &DolphinPackageManager::failure, this, [this, packageInstaller](const QString &userFacingErrorMessage) { + Q_EMIT showMessage(userFacingErrorMessage, KMessageWidget::Error); + }); + connect(packageInstaller, &DolphinPackageManager::success, this, [this, packageInstaller]() { + Q_EMIT showMessage(xi18nc("@info", "Filelight installed successfully."), KMessageWidget::Positive); + }); + packageInstaller->install(FILELIGHT_PACKAGE_NAME, QUrl("appstream://org.kde.filelight.desktop"), [](){ return KService::serviceByDesktopName(QStringLiteral("org.kde.filelight")); + }); #endif - }); + }); + m_installFilelightWidgetAction = new QWidgetAction{this}; + m_installFilelightWidgetAction->setDefaultWidget(containerWidget); // transfers ownership of containerWidget + } + m_buttonMenu->addAction(m_installFilelightWidgetAction); return; } diff --git a/src/statusbar/statusbarspaceinfo.h b/src/statusbar/statusbarspaceinfo.h index 23a77d045e..02e4e02b13 100644 --- a/src/statusbar/statusbarspaceinfo.h +++ b/src/statusbar/statusbarspaceinfo.h @@ -6,6 +6,8 @@ #ifndef STATUSBARSPACEINFO_H #define STATUSBARSPACEINFO_H +#include + #include #include @@ -14,6 +16,7 @@ class QShowEvent; class QMenu; class QMouseEvent; class QToolButton; +class QWidgetAction; class KCapacityBar; @@ -40,6 +43,12 @@ public: void update(); +Q_SIGNALS: + /** + * Requests for @p message with the given @p messageType to be shown to the user in a non-modal way. + */ + void showMessage(const QString &message, KMessageWidget::MessageType messageType); + protected: void showEvent(QShowEvent *event) override; void hideEvent(QHideEvent *event) override; @@ -55,6 +64,7 @@ private: KCapacityBar *m_capacityBar; QToolButton *m_textInfoButton; QMenu *m_buttonMenu; + QWidgetAction *m_installFilelightWidgetAction; QUrl m_url; bool m_ready; bool m_shown;