From d48c7be6fa0632254210ee228a182e144efb22f5 Mon Sep 17 00:00:00 2001 From: Martchus Date: Wed, 26 Dec 2018 01:18:28 +0100 Subject: [PATCH] Set the parents of the QActions for Dolphin integration correctly The parent must be the plugin itself. Otherwise the QActions are not destroyed when the menu vanishes. --- .../syncthingfileitemaction.cpp | 44 ++++++++++--------- .../syncthingfileitemaction.h | 2 +- fileitemactionplugin/syncthingmenuaction.cpp | 6 +-- fileitemactionplugin/syncthingmenuaction.h | 2 +- 4 files changed, 28 insertions(+), 26 deletions(-) diff --git a/fileitemactionplugin/syncthingfileitemaction.cpp b/fileitemactionplugin/syncthingfileitemaction.cpp index a4fe334..6b01254 100644 --- a/fileitemactionplugin/syncthingfileitemaction.cpp +++ b/fileitemactionplugin/syncthingfileitemaction.cpp @@ -47,17 +47,22 @@ SyncthingFileItemAction::SyncthingFileItemAction(QObject *parent, const QVariant QList SyncthingFileItemAction::actions(const KFileItemListProperties &fileItemInfo, QWidget *parentWidget) { - const QList actions = createActions(fileItemInfo, parentWidget); + Q_UNUSED(parentWidget) - // don't show anything if no relevant actions could be determined - if (s_data.connection().isConnected() && actions.isEmpty()) { - return QList(); + // create actions + const QList subActions = createActions(fileItemInfo, this); + QList topLevelActions; + + // don't show anything if no relevant actions could be determined but successfully connected + if (s_data.connection().isConnected() && !s_data.hasError() && subActions.isEmpty()) { + return topLevelActions; } - return QList({ new SyncthingMenuAction(fileItemInfo, actions, parentWidget) }); + topLevelActions << new SyncthingMenuAction(fileItemInfo, subActions, this); + return topLevelActions; } -QList SyncthingFileItemAction::createActions(const KFileItemListProperties &fileItemInfo, QWidget *parentWidget) +QList SyncthingFileItemAction::createActions(const KFileItemListProperties &fileItemInfo, QObject *parent) { QList actions; auto &data = s_data; @@ -107,7 +112,7 @@ QList SyncthingFileItemAction::createActions(const KFileItemListPrope } else { rescanLabel = tr("Rescan \"%1\"").arg(detectedItems.front().name); } - actions << new QAction(QIcon::fromTheme(QStringLiteral("view-refresh")), rescanLabel, parentWidget); + actions << new QAction(QIcon::fromTheme(QStringLiteral("view-refresh")), rescanLabel, parent); if (connection.isConnected()) { for (const SyncthingItem &item : detectedItems) { connect(actions.back(), &QAction::triggered, bind(&SyncthingFileItemActionStaticData::rescanDir, &data, item.dir->id, item.path)); @@ -121,8 +126,7 @@ QList SyncthingFileItemAction::createActions(const KFileItemListPrope if (!detectedDirs.isEmpty()) { // rescan item actions << new QAction(QIcon::fromTheme(QStringLiteral("folder-sync")), - detectedDirs.size() == 1 ? tr("Rescan \"%1\"").arg(detectedDirs.front()->displayName()) : tr("Rescan selected directories"), - parentWidget); + detectedDirs.size() == 1 ? tr("Rescan \"%1\"").arg(detectedDirs.front()->displayName()) : tr("Rescan selected directories"), parent); if (connection.isConnected()) { for (const SyncthingDir *dir : detectedDirs) { connect(actions.back(), &QAction::triggered, bind(&SyncthingFileItemActionStaticData::rescanDir, &data, dir->id, QString())); @@ -145,12 +149,10 @@ QList SyncthingFileItemAction::createActions(const KFileItemListPrope } if (isPaused) { actions << new QAction(QIcon::fromTheme(QStringLiteral("media-playback-start")), - detectedDirs.size() == 1 ? tr("Resume \"%1\"").arg(detectedDirs.front()->displayName()) : tr("Resume selected directories"), - parentWidget); + detectedDirs.size() == 1 ? tr("Resume \"%1\"").arg(detectedDirs.front()->displayName()) : tr("Resume selected directories"), parent); } else { actions << new QAction(QIcon::fromTheme(QStringLiteral("media-playback-pause")), - detectedDirs.size() == 1 ? tr("Pause \"%1\"").arg(detectedDirs.front()->displayName()) : tr("Pause selected directories"), - parentWidget); + detectedDirs.size() == 1 ? tr("Pause \"%1\"").arg(detectedDirs.front()->displayName()) : tr("Pause selected directories"), parent); } if (connection.isConnected()) { connect(actions.back(), &QAction::triggered, @@ -165,7 +167,7 @@ QList SyncthingFileItemAction::createActions(const KFileItemListPrope // rescan item actions << new QAction(QIcon::fromTheme(QStringLiteral("folder-sync")), containingDirs.size() == 1 ? tr("Rescan \"%1\"").arg(containingDirs.front()->displayName()) : tr("Rescan containing directories"), - parentWidget); + parent); if (connection.isConnected()) { for (const SyncthingDir *dir : containingDirs) { connect(actions.back(), &QAction::triggered, bind(&SyncthingFileItemActionStaticData::rescanDir, &data, dir->id, QString())); @@ -188,11 +190,11 @@ QList SyncthingFileItemAction::createActions(const KFileItemListPrope if (isPaused) { actions << new QAction(QIcon::fromTheme(QStringLiteral("media-playback-start")), containingDirs.size() == 1 ? tr("Resume \"%1\"").arg(containingDirs.front()->displayName()) : tr("Resume containing directories"), - parentWidget); + parent); } else { actions << new QAction(QIcon::fromTheme(QStringLiteral("media-playback-pause")), containingDirs.size() == 1 ? tr("Pause \"%1\"").arg(containingDirs.front()->displayName()) : tr("Pause containing directories"), - parentWidget); + parent); } if (connection.isConnected()) { connect(actions.back(), &QAction::triggered, @@ -204,7 +206,7 @@ QList SyncthingFileItemAction::createActions(const KFileItemListPrope // add actions to show further information about directory if the selection is only about one particular Syncthing dir if (lastDir && detectedDirs.size() + containingDirs.size() == 1) { - auto *statusActions = new SyncthingDirActions(*lastDir, parentWidget); + auto *statusActions = new SyncthingDirActions(*lastDir, parent); connect(&connection, &SyncthingConnection::newDirs, statusActions, static_cast &)>(&SyncthingDirActions::updateStatus)); connect(&connection, &SyncthingConnection::dirStatusChanged, statusActions, @@ -214,13 +216,13 @@ QList SyncthingFileItemAction::createActions(const KFileItemListPrope // add separator if (!actions.isEmpty()) { - QAction *const separator = new QAction(parentWidget); + QAction *const separator = new QAction(parent); separator->setSeparator(true); actions << separator; } // add error action - QAction *const errorAction = new SyncthingInfoAction(parentWidget); + QAction *const errorAction = new SyncthingInfoAction(parent); errorAction->setText(data.currentError()); errorAction->setIcon(QIcon::fromTheme(QStringLiteral("state-error"))); errorAction->setVisible(data.hasError()); @@ -230,12 +232,12 @@ QList SyncthingFileItemAction::createActions(const KFileItemListPrope actions << errorAction; // add config file selection - QAction *const configFileAction = new QAction(QIcon::fromTheme(QStringLiteral("settings-configure")), tr("Select Syncthing config ...")); + QAction *const configFileAction = new QAction(QIcon::fromTheme(QStringLiteral("settings-configure")), tr("Select Syncthing config ..."), parent); connect(configFileAction, &QAction::triggered, &data, &SyncthingFileItemActionStaticData::selectSyncthingConfig); actions << configFileAction; // about about action - QAction *const aboutAction = new QAction(QIcon::fromTheme(QStringLiteral("help-about")), tr("About")); + QAction *const aboutAction = new QAction(QIcon::fromTheme(QStringLiteral("help-about")), tr("About"), parent); connect(aboutAction, &QAction::triggered, &SyncthingFileItemActionStaticData::showAboutDialog); actions << aboutAction; diff --git a/fileitemactionplugin/syncthingfileitemaction.h b/fileitemactionplugin/syncthingfileitemaction.h index 73a3ee0..df2954e 100644 --- a/fileitemactionplugin/syncthingfileitemaction.h +++ b/fileitemactionplugin/syncthingfileitemaction.h @@ -17,7 +17,7 @@ class SyncthingFileItemAction : public KAbstractFileItemActionPlugin { public: SyncthingFileItemAction(QObject *parent, const QVariantList &args); QList actions(const KFileItemListProperties &fileItemInfo, QWidget *parentWidget) override; - static QList createActions(const KFileItemListProperties &fileItemInfo, QWidget *parentWidget); + static QList createActions(const KFileItemListProperties &fileItemInfo, QObject *parent); static SyncthingFileItemActionStaticData &staticData(); private: diff --git a/fileitemactionplugin/syncthingmenuaction.cpp b/fileitemactionplugin/syncthingmenuaction.cpp index 8be5f03..e315b5f 100644 --- a/fileitemactionplugin/syncthingmenuaction.cpp +++ b/fileitemactionplugin/syncthingmenuaction.cpp @@ -18,8 +18,8 @@ using namespace Data; -SyncthingMenuAction::SyncthingMenuAction(const KFileItemListProperties &properties, const QList &actions, QWidget *parentWidget) - : QAction(parentWidget) +SyncthingMenuAction::SyncthingMenuAction(const KFileItemListProperties &properties, const QList &actions, QObject *parent) + : QAction(parent) , m_properties(properties) , m_notifier(SyncthingFileItemAction::staticData().connection()) { @@ -51,7 +51,7 @@ void SyncthingMenuAction::handleConnectedChanged() menu->deleteLater(); setMenu(nullptr); } - createMenu(SyncthingFileItemAction::createActions(m_properties, parentWidget())); + createMenu(SyncthingFileItemAction::createActions(m_properties, parent())); // update status of action itself updateActionStatus(); diff --git a/fileitemactionplugin/syncthingmenuaction.h b/fileitemactionplugin/syncthingmenuaction.h index d44823b..002aed7 100644 --- a/fileitemactionplugin/syncthingmenuaction.h +++ b/fileitemactionplugin/syncthingmenuaction.h @@ -19,7 +19,7 @@ class SyncthingMenuAction : public QAction { public: explicit SyncthingMenuAction(const KFileItemListProperties &properties = KFileItemListProperties(), - const QList &actions = QList(), QWidget *parentWidget = nullptr); + const QList &actions = QList(), QObject *parent = nullptr); #ifdef DEBUG_BUILD ~SyncthingMenuAction() override; #endif