From d21984ba5bcd7b90f0ecdd22f4d31176d9c2ce2d Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Wed, 29 May 2024 13:23:08 +0200 Subject: [PATCH 1/4] 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; From fb651f0cbcae1ec7dc3a5b7bacefd588b7ebc84b Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Thu, 6 Jun 2024 19:05:22 +0200 Subject: [PATCH 2/4] Make DolphinPackageInstaller a KJob This means developers can use a familiar pattern here. --- src/CMakeLists.txt | 4 +- ...anager.cpp => dolphinpackageinstaller.cpp} | 66 ++++++++------ src/dolphinpackageinstaller.h | 85 +++++++++++++++++++ src/dolphinpackagemanager.h | 71 ---------------- src/statusbar/statusbarspaceinfo.cpp | 28 +++--- 5 files changed, 143 insertions(+), 111 deletions(-) rename src/{dolphinpackagemanager.cpp => dolphinpackageinstaller.cpp} (60%) create mode 100644 src/dolphinpackageinstaller.h delete mode 100644 src/dolphinpackagemanager.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 4baaf52993..73d7eaeb28 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -274,7 +274,7 @@ target_sources(dolphinstatic PRIVATE dolphincontextmenu.cpp dolphinnavigatorswidgetaction.cpp dolphintabbar.cpp - dolphinpackagemanager.cpp + dolphinpackageinstaller.cpp dolphinplacesmodelsingleton.cpp dolphinrecenttabsmenu.cpp dolphintabpage.cpp @@ -335,7 +335,7 @@ target_sources(dolphinstatic PRIVATE dolphincontextmenu.h dolphinnavigatorswidgetaction.h dolphintabbar.h - dolphinpackagemanager.h + dolphinpackageinstaller.h dolphinplacesmodelsingleton.h dolphinrecenttabsmenu.h dolphintabpage.h diff --git a/src/dolphinpackagemanager.cpp b/src/dolphinpackageinstaller.cpp similarity index 60% rename from src/dolphinpackagemanager.cpp rename to src/dolphinpackageinstaller.cpp index 0768748ec9..18af237e73 100644 --- a/src/dolphinpackagemanager.cpp +++ b/src/dolphinpackageinstaller.cpp @@ -5,7 +5,7 @@ SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL */ -#include "dolphinpackagemanager.h" +#include "dolphinpackageinstaller.h" #include @@ -18,51 +18,54 @@ #include #include -DolphinPackageManager::DolphinPackageManager(QObject *parent) - : QObject(parent) +DolphinPackageInstaller::DolphinPackageInstaller(const QString &packageName, + const QUrl &fallBackInstallationPageUrl, + std::function isPackageInstalledCheck, + QObject *parent) + : KJob(parent) + , m_packageName{packageName} + , m_fallBackInstallationPageUrl{fallBackInstallationPageUrl} + , m_isPackageInstalledCheck{isPackageInstalledCheck} { } -void DolphinPackageManager::install(const QString &packageName, const QUrl &fallBackInstallationPageUrl, std::function isPackageInstalledCheck) +void DolphinPackageInstaller::start() { - 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(); + if (m_isPackageInstalledCheck()) { + emitResult(); 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); + const PackageKit::Transaction *resolveTransaction = PackageKit::Daemon::resolve(m_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. + connect(resolveTransaction, &PackageKit::Transaction::errorCode, this, &DolphinPackageInstaller::slotInstallationFailed); + connect(resolveTransaction, &PackageKit::Transaction::finished, this, [this]() { // Will be disconnected if we find a package. slotInstallationFailed(PackageKit::Transaction::ErrorPackageNotFound, - i18nc("@info:shell about system packages", "Could not find package %1.", packageName)); + i18nc("@info:shell about system packages", "Could not find package %1.", m_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); + const PackageKit::Transaction *installTransaction = PackageKit::Daemon::installPackage(packageId); + connectTransactionToJobProgress(*installTransaction); 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. + disconnect(installTransaction, nullptr, this, nullptr); // We only want to emit a result 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. + disconnect(installTransaction, nullptr, this, nullptr); // We only want to emit a result once. if (status == PackageKit::Transaction::ExitSuccess) { - Q_EMIT success(); - deleteLater(); + emitResult(); } else { slotInstallationFailed(PackageKit::Transaction::ErrorUnknown, i18nc("@info %1 is error code", @@ -72,13 +75,11 @@ void DolphinPackageManager::install(const QString &packageName, const QUrl &fall }); }); #else - Q_UNUSED(packageName) - QDesktopServices::openUrl(fallBackInstallationPageUrl); + QDesktopServices::openUrl(m_fallBackInstallationPageUrl); auto waitForSuccess = new QTimer(this); - connect(waitForSuccess, &QTimer::timeout, this, [isPackageInstalledCheck, this, waitForSuccess]() { - if (isPackageInstalledCheck()) { - Q_EMIT success(); - deleteLater(); + connect(waitForSuccess, &QTimer::timeout, this, [this]() { + if (m_isPackageInstalledCheck()) { + emitResult(); } }); waitForSuccess->start(3000); @@ -86,13 +87,24 @@ void DolphinPackageManager::install(const QString &packageName, const QUrl &fall } #if HAVE_PACKAGEKIT -void DolphinPackageManager::slotInstallationFailed(PackageKit::Transaction::Error error, const QString &details) +void DolphinPackageInstaller::connectTransactionToJobProgress(const PackageKit::Transaction &transaction) { - Q_EMIT failure(xi18nc("@info:shell %1 is package name, %2 is error message, %3 is error e.g. 'ErrorNoNetwork'", + connect(&transaction, &PackageKit::Transaction::speedChanged, this, [this, &transaction]() { + emitSpeed(transaction.speed()); + }); + connect(&transaction, &PackageKit::Transaction::percentageChanged, this, [this, &transaction]() { + setPercent(transaction.percentage()); + }); +} + +void DolphinPackageInstaller::slotInstallationFailed(PackageKit::Transaction::Error error, const QString &details) +{ + setErrorString(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(); + setError(error); + emitResult(); } #endif diff --git a/src/dolphinpackageinstaller.h b/src/dolphinpackageinstaller.h new file mode 100644 index 0000000000..2a67111284 --- /dev/null +++ b/src/dolphinpackageinstaller.h @@ -0,0 +1,85 @@ +/* + 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 dolphinpackageinstaller_H +#define dolphinpackageinstaller_H + +#include "config-dolphin.h" + +#if HAVE_PACKAGEKIT +#include +#endif +#include + +#include + +/** + * @brief A KJob providing simple means to install a package. + */ +class DolphinPackageInstaller : public KJob +{ +public: + /** + * @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.filelight.desktop". The user is then expected to install the package themselves and + * KJob::result() will be emitted when it is detected that the installation finished successfully. + * @param isPackageInstalledCheck A function that can be regularly checked to determine if the installation was already successful. + */ + explicit DolphinPackageInstaller(const QString &packageName, + const QUrl &fallBackInstallationPageUrl, + std::function isPackageInstalledCheck, + QObject *parent = nullptr); + + /** + * @see KJob::start(). + * Make sure to connect to the KJob::result() signal and show the KJob::errorString() to users there before calling this. + */ + void start() override; + + /** @see KJob::errorString(). */ + inline QString errorString() const override + { + return m_errorString; + }; + +private: + /** @see KJob::errorString(). */ + inline void setErrorString(const QString &errorString) + { + m_errorString = errorString; + }; + +#if HAVE_PACKAGEKIT + /** + * Makes sure progress signals of @p transaction are forwarded to KJob's progress signals. + */ + void connectTransactionToJobProgress(const PackageKit::Transaction &transaction); + +private Q_SLOTS: + /** Creates a nice user-facing error message from its parameters and then finishes this job with an @p error. */ + void slotInstallationFailed(PackageKit::Transaction::Error error, const QString &details); +#endif + +private: + /** The name of the package that is supposed to be installed. */ + const QString m_packageName; + + /** @see DolphinPackageInstaller::DolphinPackageInstaller(). */ + const QUrl m_fallBackInstallationPageUrl; + + /** @see DolphinPackageInstaller::DolphinPackageInstaller(). */ + const std::function m_isPackageInstalledCheck; + + /** @see KJob::errorString(). */ + QString m_errorString; +}; + +#endif // dolphinpackageinstaller_H diff --git a/src/dolphinpackagemanager.h b/src/dolphinpackagemanager.h deleted file mode 100644 index 0431c12c07..0000000000 --- a/src/dolphinpackagemanager.h +++ /dev/null @@ -1,71 +0,0 @@ -/* - 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/statusbar/statusbarspaceinfo.cpp b/src/statusbar/statusbarspaceinfo.cpp index 5ea62357a8..e1d22c0566 100644 --- a/src/statusbar/statusbarspaceinfo.cpp +++ b/src/statusbar/statusbarspaceinfo.cpp @@ -7,7 +7,7 @@ #include "statusbarspaceinfo.h" #include "config-dolphin.h" -#include "dolphinpackagemanager.h" +#include "dolphinpackageinstaller.h" #include "global.h" #include "spaceinfoobserver.h" @@ -154,7 +154,7 @@ void StatusBarSpaceInfo::updateMenu() vLayout->addSpacing(Dolphin::VERTICAL_SPACER_HEIGHT); auto installFilelightButton = - new QPushButton(QIcon::fromTheme(QStringLiteral("filelight")), i18nc("@action:button", "Install Filelight"), containerWidget); + 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); @@ -170,15 +170,21 @@ void StatusBarSpaceInfo::updateMenu() #ifdef Q_OS_WIN QDesktopServices::openUrl(QUrl("https://apps.kde.org/filelight")); #else - 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")); - }); + auto packageInstaller = new DolphinPackageInstaller( + FILELIGHT_PACKAGE_NAME, + QUrl("appstream://org.kde.filelight.desktop"), + []() { + return KService::serviceByDesktopName(QStringLiteral("org.kde.filelight")); + }, + this); + connect(packageInstaller, &KJob::result, this, [this](KJob *job) { + if (job->error()) { + Q_EMIT showMessage(job->errorString(), KMessageWidget::Error); + } else { + Q_EMIT showMessage(xi18nc("@info", "Filelight installed successfully."), KMessageWidget::Positive); + } + }); + packageInstaller->start(); #endif }); From 982cb7e58c1c281752709508f28b8dad8b5fce83 Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Sat, 8 Jun 2024 15:05:40 +0200 Subject: [PATCH 3/4] Show installation progress on install button Also reopen the menu if it is currently open while the installation finishes. --- src/statusbar/statusbarspaceinfo.cpp | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/statusbar/statusbarspaceinfo.cpp b/src/statusbar/statusbarspaceinfo.cpp index e1d22c0566..1febd4caf4 100644 --- a/src/statusbar/statusbarspaceinfo.cpp +++ b/src/statusbar/statusbarspaceinfo.cpp @@ -153,9 +153,9 @@ void StatusBarSpaceInfo::updateMenu() 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())); + const QString installFilelightButtonDefaultText{i18nc("@action:button", "Install Filelight…")}; + auto installFilelightButton = new QPushButton(QIcon::fromTheme(QStringLiteral("filelight")), installFilelightButtonDefaultText, containerWidget); + installFilelightButton->setMinimumWidth(std::max(installFilelightButton->sizeHint().width(), installFilelightTitle->sizeHint().width())); auto buttonLayout = new QHBoxLayout{containerWidget}; buttonLayout->addWidget(installFilelightButton, 0, Qt::AlignHCenter); vLayout->addLayout(buttonLayout); @@ -166,9 +166,9 @@ void StatusBarSpaceInfo::updateMenu() containerWidget->setFocusProxy(installFilelightButton); installFilelightButton->setAccessibleDescription(installFilelightBody->text()); - connect(installFilelightButton, &QAbstractButton::clicked, this, [this] { + connect(installFilelightButton, &QAbstractButton::clicked, this, [this, installFilelightButton, installFilelightButtonDefaultText] { #ifdef Q_OS_WIN - QDesktopServices::openUrl(QUrl("https://apps.kde.org/filelight")); + QDesktopServices::openUrl(QUrl("https://apps.kde.org/filelight")); #else auto packageInstaller = new DolphinPackageInstaller( FILELIGHT_PACKAGE_NAME, @@ -177,13 +177,25 @@ void StatusBarSpaceInfo::updateMenu() return KService::serviceByDesktopName(QStringLiteral("org.kde.filelight")); }, this); - connect(packageInstaller, &KJob::result, this, [this](KJob *job) { + connect(packageInstaller, &KJob::result, this, [this, installFilelightButton, installFilelightButtonDefaultText](KJob *job) { + installFilelightButton->setText(installFilelightButtonDefaultText); if (job->error()) { Q_EMIT showMessage(job->errorString(), KMessageWidget::Error); } else { Q_EMIT showMessage(xi18nc("@info", "Filelight installed successfully."), KMessageWidget::Positive); + if (m_textInfoButton->menu()->isVisible()) { + m_textInfoButton->menu()->hide(); + updateMenu(); + m_textInfoButton->menu()->show(); + } } }); + connect(packageInstaller, &KJob::percentChanged, installFilelightButton, [installFilelightButton](KJob */* job */, long unsigned int percent) { + if (percent > 100) { + return; // For some reason it instantly reports 101% completion for me when it starts. + } + installFilelightButton->setText(i18nc("@action:button which also shows progress %1 in percent", "Install Filelight… (%1%)", percent)); + }); packageInstaller->start(); #endif }); From 242b1b73a67fdb2a74af0250d142d8d0b0c0fd50 Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Thu, 13 Jun 2024 17:38:52 +0200 Subject: [PATCH 4/4] Show installation progress in the status bar This commit reuses the progress reporting of the status bar for the Filelight installation progress. --- src/statusbar/dolphinstatusbar.cpp | 6 ++++++ src/statusbar/statusbarspaceinfo.cpp | 19 ++++++++++--------- src/statusbar/statusbarspaceinfo.h | 2 ++ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/statusbar/dolphinstatusbar.cpp b/src/statusbar/dolphinstatusbar.cpp index cb8c422456..c8369febcf 100644 --- a/src/statusbar/dolphinstatusbar.cpp +++ b/src/statusbar/dolphinstatusbar.cpp @@ -71,6 +71,12 @@ DolphinStatusBar::DolphinStatusBar(QWidget *parent) // Initialize space information m_spaceInfo = new StatusBarSpaceInfo(contentsContainer); connect(m_spaceInfo, &StatusBarSpaceInfo::showMessage, this, &DolphinStatusBar::showMessage); + connect(m_spaceInfo, + &StatusBarSpaceInfo::showInstallationProgress, + this, + [this](const QString ¤tlyRunningTaskTitle, int installationProgressPercent) { + showProgress(currentlyRunningTaskTitle, installationProgressPercent, CancelLoading::Disallowed); + }); // Initialize progress information m_stopButton = new QToolButton(contentsContainer); diff --git a/src/statusbar/statusbarspaceinfo.cpp b/src/statusbar/statusbarspaceinfo.cpp index 1febd4caf4..e464b9d9f5 100644 --- a/src/statusbar/statusbarspaceinfo.cpp +++ b/src/statusbar/statusbarspaceinfo.cpp @@ -153,8 +153,8 @@ void StatusBarSpaceInfo::updateMenu() vLayout->addSpacing(Dolphin::VERTICAL_SPACER_HEIGHT); - const QString installFilelightButtonDefaultText{i18nc("@action:button", "Install Filelight…")}; - auto installFilelightButton = new QPushButton(QIcon::fromTheme(QStringLiteral("filelight")), installFilelightButtonDefaultText, containerWidget); + auto installFilelightButton = + new QPushButton(QIcon::fromTheme(QStringLiteral("filelight")), i18nc("@action:button", "Install Filelight…"), containerWidget); installFilelightButton->setMinimumWidth(std::max(installFilelightButton->sizeHint().width(), installFilelightTitle->sizeHint().width())); auto buttonLayout = new QHBoxLayout{containerWidget}; buttonLayout->addWidget(installFilelightButton, 0, Qt::AlignHCenter); @@ -166,7 +166,7 @@ void StatusBarSpaceInfo::updateMenu() containerWidget->setFocusProxy(installFilelightButton); installFilelightButton->setAccessibleDescription(installFilelightBody->text()); - connect(installFilelightButton, &QAbstractButton::clicked, this, [this, installFilelightButton, installFilelightButtonDefaultText] { + connect(installFilelightButton, &QAbstractButton::clicked, this, [this] { #ifdef Q_OS_WIN QDesktopServices::openUrl(QUrl("https://apps.kde.org/filelight")); #else @@ -177,8 +177,8 @@ void StatusBarSpaceInfo::updateMenu() return KService::serviceByDesktopName(QStringLiteral("org.kde.filelight")); }, this); - connect(packageInstaller, &KJob::result, this, [this, installFilelightButton, installFilelightButtonDefaultText](KJob *job) { - installFilelightButton->setText(installFilelightButtonDefaultText); + connect(packageInstaller, &KJob::result, this, [this](KJob *job) { + Q_EMIT showInstallationProgress(QString(), 100); // Hides the progress information in the status bar. if (job->error()) { Q_EMIT showMessage(job->errorString(), KMessageWidget::Error); } else { @@ -190,11 +190,12 @@ void StatusBarSpaceInfo::updateMenu() } } }); - connect(packageInstaller, &KJob::percentChanged, installFilelightButton, [installFilelightButton](KJob */* job */, long unsigned int percent) { - if (percent > 100) { - return; // For some reason it instantly reports 101% completion for me when it starts. + const auto installationTaskText{i18nc("@info:status", "Installing Filelight…")}; + Q_EMIT showInstallationProgress(installationTaskText, -1); + connect(packageInstaller, &KJob::percentChanged, this, [this, installationTaskText](KJob */* job */, long unsigned int percent) { + if (percent < 100) { // Ignore some weird reported values. + Q_EMIT showInstallationProgress(installationTaskText, percent); } - installFilelightButton->setText(i18nc("@action:button which also shows progress %1 in percent", "Install Filelight… (%1%)", percent)); }); packageInstaller->start(); #endif diff --git a/src/statusbar/statusbarspaceinfo.h b/src/statusbar/statusbarspaceinfo.h index 02e4e02b13..e78a839349 100644 --- a/src/statusbar/statusbarspaceinfo.h +++ b/src/statusbar/statusbarspaceinfo.h @@ -49,6 +49,8 @@ Q_SIGNALS: */ void showMessage(const QString &message, KMessageWidget::MessageType messageType); + void showInstallationProgress(const QString ¤tlyRunningTaskTitle, int installationProgressPercent); + protected: void showEvent(QShowEvent *event) override; void hideEvent(QHideEvent *event) override;