unity-2d

Merge lp://qastaging/~osomon/unity-2d/custom-quicklists-dynamic into lp://qastaging/unity-2d/3.0

  1. custom-quicklists-dynamic
  2. Merge into natty
Proposed by Olivier Tilloy
Status: Merged
Approved by: Ugo Riboni
Approved revision: 560
Merged at revision: 563
Proposed branch: lp://qastaging/~osomon/unity-2d/custom-quicklists-dynamic
Merge into: lp://qastaging/unity-2d/3.0
Diff against target: 296 lines (+93/-13)
6 files modified
launcher/UnityApplications/launcherapplication.cpp (+59/-9)
launcher/UnityApplications/launcherapplication.h (+10/-1)
launcher/UnityApplications/launcherapplicationslist.cpp (+7/-1)
launcher/UnityApplications/launcherapplicationslist.h (+2/-1)
launcher/UnityApplications/launchermenu.cpp (+12/-1)
launcher/UnityApplications/launchermenu.h (+3/-0)
To merge this branch: bzr merge lp://qastaging/~osomon/unity-2d/custom-quicklists-dynamic
Reviewer Review Type Date Requested Status
Ugo Riboni (community) Approve
Florian Boucault Pending
Review via email: mp+59396@code.qastaging.launchpad.net

Commit message

[launcher] Support dynamic shortcuts in the quicklists.

Dynamic shortcuts are exposed over D-Bus by the application through the use of libunity.
See https://wiki.ubuntu.com/Unity/LauncherAPI#Dynamic%20Quicklist%20entries for documentation.

Description of the change

Note: at the moment, it seems the only application in the archive that implements dynamic quicklists is deja-dup.
To test, you will need to install deja-dup, start a backup, and observe the quicklist while backing up.
Alternatively, you can run example programs that pretend to be another application and expose dynamic a quicklist for it, see https://wiki.ubuntu.com/Unity/LauncherAPI#Example%20Code for details.

To post a comment you must log in.
Revision history for this message
Ugo Riboni (uriboni) wrote :

Functionally it works OK. I tested it both with dejadup and with the example code.

The only issue that might be interesting to fix is that if the menu is already open, it's not updated. This leads to having some menu items that are not necessarily valid anymore. Not sure what will happen if you try to activate a menu item that the application has removed already.

review: Approve
Revision history for this message
Ugo Riboni (uriboni) wrote :

208 LauncherApplicationsList::onRemoteEntryUpdated(QString applicationURI, QMap<QString, QVariant> properties)
209 {
210 + QString sender = message().service();

For safety I would would check the result of calledFromDBus() before proceeding to access message()

Other than that, code looks good.

Revision history for this message
Olivier Tilloy (osomon) wrote :

> The only issue that might be interesting to fix is that if the menu is already
> open, it's not updated. This leads to having some menu items that are not
> necessarily valid anymore. Not sure what will happen if you try to activate a
> menu item that the application has removed already.

It is actually updated auto-magically :) (yes, I was surprised myself)
Try with the example code: launch the executable/script, open the contextual menu for evolution, and while it’s open, kill the example application: the menu items it added will disappear.

Revision history for this message
Olivier Tilloy (osomon) wrote :

> 208 LauncherApplicationsList::onRemoteEntryUpdated(QString
> applicationURI, QMap<QString, QVariant> properties)
> 209 {
> 210 + QString sender = message().service();
>
> For safety I would would check the result of calledFromDBus() before
> proceeding to access message()

This is true on principle (better safe than sorry). However onRemoteEntryUpdated is a private slot and it is only connected to D-Bus signals, so we are guaranteed that it is actually called from DBus.

lp://qastaging/~osomon/unity-2d/custom-quicklists-dynamic updated
559. By Olivier Tilloy

For safety, always make sure we are called from DBus.

560. By Olivier Tilloy

Add a FIXME to explain a current limitation of the implementation.

Revision history for this message
Ugo Riboni (uriboni) wrote :

Good to go now. Good job, and please open a bug for the missing feature in the FIXME.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches