Merge lp:~uriboni/unity-2d/launcher-shortcuts-on-hold-dash-on-tap into lp:unity-2d/3.0

[Ugo Riboni]

[launcher] Make the Dash appear when the user just "taps" the Super key, and force the Launcher to appear and display the keyboard shortcuts when the Super key is held instead.

[Florian Boucault (fboucault)]

I think it also fixes https://bugs.launchpad.net/unity/+bug/706713

[Olivier Tilloy (osomon)]

In launcherview.cpp, the following include is useless, it is already included in launcherview.h:

+#include <QTimer>

review: Needs Fixing (code)

[Olivier Tilloy (osomon)]

Having two attributes for the state of the super key, m_superKeyPressed and m_superKeyHeld, is rather confusing, even if m_superKeyPressed is private. I would expect 'held' to imply 'press'.
If I read the code correctly, the only place where m_superKeyPressed is used is in LauncherView::updateSuperKeyHoldState(). Can’t you get rid of the attribute and test the state of the modifier using KeyboardModifiersMonitor::keyboardModifiers()?

[Olivier Tilloy (osomon)]

When I tap the super key, the dash shows up, but it is empty, and searching has no effect. I suspect setting the 'active' property of the dash to true is not enough.

review: Needs Fixing (functional)

[Ugo Riboni (uriboni)]

Fixed all the remarks, except for the code simplification that as discussed caused more harm than benefit.

[Olivier Tilloy (osomon)]

In launcher/app/launcherview.cpp, for consistency with the naming scheme you picked, 'DASH_DBUS_HOME' should be renamed to 'DASH_DBUS_METHOD_ACTIVATE_HOME'.

review: Needs Fixing (code)

[Olivier Tilloy (osomon)]

In launcher/Launcher.qml, please add curly braces around the code connected to launcherView.onSuperKeyHeldChanged for readability and consistency with the other handlers that are not one-liners.

review: Needs Fixing (code)

[Olivier Tilloy (osomon)]

Once you have applied those tweaks, please go ahead and mark the request approved.

review: Approve

[Bob The Builder (bobthebuilder-deactivatedaccount)]

No proposals found for merge of lp:~uriboni/unity-2d/clean-up-dash-dbus into lp:unity-2d.

Unmerged revisions

=== modified file 'launcher/Launcher.qml'
--- launcher/Launcher.qml	2011-03-22 11:58:51 +0000
+++ launcher/Launcher.qml	2011-03-22 11:58:51 +0000
@@ -107,14 +107,13 @@
         onDesktopFileDropped: applications.insertFavoriteApplication(path)
         onWebpageUrlDropped: applications.insertWebFavorite(url)
         onAddWebFavoriteRequested: applications.insertWebFavorite(url)
-<<<<<<< TREE
+        onSuperKeyHeldChanged: {
+            if (superKeyHeld) visibilityController.beginForceVisible()
+            else visibilityController.endForceVisible()
+        }
         onFocusChanged: {
             if (focus) visibilityController.beginForceVisible()
             else visibilityController.endForceVisible()
         }
-=======
-        onSuperKeyHeldChanged: if (superKeyHeld) visibilityController.beginForceVisible()
-                               else visibilityController.endForceVisible()
->>>>>>> MERGE-SOURCE
     }
 }

=== modified file 'launcher/app/launcherview.cpp'
--- launcher/app/launcherview.cpp	2011-03-22 11:58:51 +0000
+++ launcher/app/launcherview.cpp	2011-03-22 11:58:51 +0000
@@ -28,8 +28,8 @@
 #include <QDeclarativeEngine>
 #include <QDeclarativeContext>
 #include <QDeclarativeImageProvider>
-#include <QTimer>
 #include <QtDBus/QDBusInterface>
+#include <QtDBus/QDBusPendingCall>
 
 #include <X11/Xlib.h>
 #include <X11/Xatom.h>
@@ -45,6 +45,7 @@
 static const char* DASH_DBUS_PATH = "/Dash";
 static const char* DASH_DBUS_INTERFACE = "com.canonical.Unity2d.Dash";
 static const char* DASH_DBUS_PROPERTY_ACTIVE = "active";
+static const char* DASH_DBUS_METHOD_ACTIVATE_HOME = "activateHome";
 
 LauncherView::LauncherView(QWidget* parent) :
     QDeclarativeView(parent),
@@ -208,9 +209,13 @@
     }
 
     bool dashActive = dashActiveResult.toBool();
-    if (!dashInterface.setProperty(DASH_DBUS_PROPERTY_ACTIVE, !dashActive)) {
-        qWarning() << "Can't set the DBUS Dash property" << DASH_DBUS_PROPERTY_ACTIVE
-                   << "on" << DASH_DBUS_SERVICE << DASH_DBUS_PATH << DASH_DBUS_INTERFACE;
+    if (dashActive) {
+        if (!dashInterface.setProperty(DASH_DBUS_PROPERTY_ACTIVE, false)) {
+            qWarning() << "Can't set the DBUS Dash property" << DASH_DBUS_PROPERTY_ACTIVE
+                       << "on" << DASH_DBUS_SERVICE << DASH_DBUS_PATH << DASH_DBUS_INTERFACE;
+        }
+    } else {
+        dashInterface.asyncCall(DASH_DBUS_METHOD_ACTIVATE_HOME);
     }
 }
 

=== modified file 'launcher/app/launcherview.h'
--- launcher/app/launcherview.h	2011-03-22 11:58:51 +0000
+++ launcher/app/launcherview.h	2011-03-22 11:58:51 +0000
@@ -33,13 +33,8 @@
 class LauncherView : public QDeclarativeView
 {
     Q_OBJECT
-<<<<<<< TREE
-    Q_PROPERTY(bool superKeyPressed READ superKeyPressed
-                                    NOTIFY superKeyPressedChanged)
+    Q_PROPERTY(bool superKeyHeld READ superKeyHeld NOTIFY superKeyHeldChanged)
     Q_PROPERTY(bool focus READ hasFocus NOTIFY focusChanged) // overridden
-=======
-    Q_PROPERTY(bool superKeyHeld READ superKeyHeld NOTIFY superKeyHeldChanged)
->>>>>>> MERGE-SOURCE
 
 public:
     explicit LauncherView(QWidget* parent = NULL);