Differential D13252

Update KIPI toolbar button after installation of kipi-plugins
ClosedPublic
Actions

Authored by rkflx on May 31 2018, 7:11 PM.

Details

Reviewers
faridb
Group Reviewers
Gwenview
Commits
R260:69ffc4038bfd: Update KIPI toolbar button after installation of kipi-plugins
Summary

In case there are no KIPI plugins installed, Gwenview can offer to
Install Plugins. After installation completed, the Plugins
entry in the menubar would update itself, however the KIPI toolbar
button would still show No Plugin found.

By always listening to KIPIInterface::loadingFinished, we are notified
through mPluginLoadTimer when the menu needs to be updated in reaction
to the plugin installation finishing.

To avoid showing Install Plugins as a Last Used Plugin in
case the installation was aborted, that section is only added when it
makes sense, i.e. when there is actually more than one plugin to show.

While mExportActionList does not contain Install Plugins anymore
after installation completed successfully, it is still contained in
mDefaultAction, which cannot be prevented easily due to not having
access to KIPIInterfacePrivate::mInstallPluginAction. Therefore we
only ever add actions from mExportActionList, which is the right thing
to do in any case.

Test Plan

Install Discover, remove kipi-plugins, open Gwenview and
click on Plugins → Install Plugins. Keep Gwenview open while
installing kipi-plugins and observe both the menu entry and the
toolbar button populating themselves after installation completed.

Restarting Gwenview afterwards should populate the toolbar button like
normal.

Diff Detail

Repository
R260 Gwenview
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rkflx requested review of this revision.May 31 2018, 7:11 PM
rkflx created this revision.
faridb accepted this revision.Jun 1 2018, 9:45 AM

Thanks! This will make D13197 much cleaner.

app/kipiexportaction.cpp
49–52

Is mExportActionList.contains(mDefaultAction) necessary? I think it is already checked in KIPIExportAction::init() on line 99.

This revision is now accepted and ready to land.Jun 1 2018, 9:45 AM
rkflx marked an inline comment as done.Jun 1 2018, 8:37 PM

@faridb Thanks for the review ;)

Does my reply make sense to you, or should I add an additional comment to the code?

app/kipiexportaction.cpp
49–52

I think it is necessary, as this is the whole trick of the patch ;)

If you click on Install Plugins, setDefaultAction will be called and mDefaultAction will be set. Before my patch, this resulted in executing the if, i.e. this action being added even after installation finished.

However, we only want to have Install Plugins if it is coming from KIPIInterfacePrivate through mExportActionList, not if it is a left-over. Thus, I added the check to prevent executing the if in the latter case.

faridb added a comment.Jun 1 2018, 9:55 PM

@rkflx Thanks for the clarification. Now, I see why it's necessary.

Closed by commit R260:69ffc4038bfd: Update KIPI toolbar button after installation of kipi-plugins (authored by rkflx). · Explain WhyJun 2 2018, 6:15 AM
This revision was automatically updated to reflect the committed changes.
rkflx marked an inline comment as done.

Revision Contents
Changeset List

PathPackages
Mapp/kipiexportaction.cpp (10 lines)

Diff 35386

View Options

app/kipiexportaction.cpp

Loading...