Merge lp:~agateau/unity-2d/keep-launcher-when-menu-is-visible into lp:unity-2d/3.0

[Aurélien Gâteau]

[launcher] Do not hide launcher while a menu is visible

Adds two signals to LauncherContextualMenu to let QML LauncherList know when a menu is shown or hidden. I first tried to use the aboutToShow() and aboutToHide() signals, but aboutToShow() is not emitted due to the way we use QMenu.

Do not hide launcher while a menu is visible

Adds two signals to LauncherContextualMenu to let QML LauncherList know when a menu is shown or hidden. I first tried to use the aboutToShow() and aboutToHide() signals, but aboutToShow() is not emitted due to the way we use QMenu.

Note that this is built on top of this branch: lp:~agateau/unity-2d/launcher-need-attention

[Ugo Riboni (uriboni)]

From a general design point of view, wouldn't it be more consistent with the QML way of doing things to have a bool property visible and then connecting to onVisibleChanged ?

Having it as a property will help for the cases where we don't need to call functions as an effect of the menu visibility change, but we can just bind the property to some other property.

review: Needs Fixing

[Aurélien Gâteau (agateau)]

What worries me is that there is already a "visible" property because LauncherContextualMenu inherits QMenu so I wanted to avoid overlapping with it, but now that I think about it, maybe I can just add the visibleChanged signal and we would be good.

[Ugo Riboni (uriboni)]

Works for me and the code seems ok too.
I have just one comment (not blocking): usually in QML most of us don't follow the C++ guideline for braces around ifs, so code can be leaner like:

if (item.menu.visible) visibilityController.beginForceVisible();
else visibilityController.endForceVisible();

But I don't mind if you leave it that way.

review: Approve

[Aurélien Gâteau (agateau)]

Mmm... I thought only historical code did not follow the guidelines. no-brace-on-single-line-if is as evil in Javascript as it is in C++. I'd prefer to leave it that way.

[Ugo Riboni (uriboni)]

Well, the C++ and QML guidelines ATM are differnet (with the QMl ones being less strict) AFAIK.
But I am not against what you said, so: code approved.

(let's discuss the guidelines in IRC with all the others, or email, if you want to change them)

review: Approve

[Florian Boucault (fboucault)]

The prerequisite lp:~agateau/unity-2d/launcher-need-attention has not yet been merged into lp:unity-2d.

[lp:~agateau/unity-2d/keep-launcher-when-menu-is-visible]

446. By Aurélien Gâteau on 2011-03-14

Merged from trunk

=== modified file 'launcher/LauncherList.qml'
--- launcher/LauncherList.qml	2011-03-14 15:25:50 +0000
+++ launcher/LauncherList.qml	2011-03-14 17:27:08 +0000
@@ -123,6 +123,17 @@
             onDraggedTileIdChanged: if (reorder.draggedTileId != "") item.menu.hide()
         }
 
+        Connections {
+            target: item.menu
+            onVisibleChanged: {
+                if (item.menu.visible) {
+                    visibilityController.beginForceVisible();
+                } else {
+                    visibilityController.endForceVisible();
+                }
+            }
+        }
+
         function setIconGeometry() {
             if (running) {
                 item.setIconGeometry(x + panel.x, y + panel.y, width, height)

=== modified file 'launcher/UnityApplications/launchermenu.cpp'
--- launcher/UnityApplications/launchermenu.cpp	2011-03-11 13:11:12 +0000
+++ launcher/UnityApplications/launchermenu.cpp	2011-03-14 17:27:08 +0000
@@ -245,3 +245,17 @@
 {
     m_launcherItem = launcherItem;
 }
+
+void
+LauncherContextualMenu::showEvent(QShowEvent* event)
+{
+    QMenu::showEvent(event);
+    visibleChanged(true);
+}
+
+void
+LauncherContextualMenu::hideEvent(QHideEvent* event)
+{
+    QMenu::hideEvent(event);
+    visibleChanged(false);
+}

=== modified file 'launcher/UnityApplications/launchermenu.h'
--- launcher/UnityApplications/launchermenu.h	2011-03-03 15:18:08 +0000
+++ launcher/UnityApplications/launchermenu.h	2011-03-14 17:27:08 +0000
@@ -33,6 +33,7 @@
     Q_PROPERTY(bool transparencyAvailable READ transparencyAvailable)
     Q_PROPERTY(bool folded READ folded WRITE setFolded NOTIFY foldedChanged)
     Q_PROPERTY(QString title READ title WRITE setTitle NOTIFY titleChanged)
+    Q_PROPERTY(bool visible READ isVisible WRITE setVisible NOTIFY visibleChanged)
 
 public:
     LauncherContextualMenu();
@@ -52,13 +53,18 @@
     Q_INVOKABLE void show(int x, int y);
     Q_INVOKABLE void hide();
     Q_INVOKABLE void hideWithDelay(int delay);
+
+protected:
     void resizeEvent(QResizeEvent* event);
     void leaveEvent(QEvent* event);
     void enterEvent(QEvent* event);
+    void showEvent(QShowEvent* event);
+    void hideEvent(QHideEvent* event);
 
 Q_SIGNALS:
     void foldedChanged(bool);
     void titleChanged(QString);
+    void visibleChanged(bool);
 
 protected:
     void paintEvent(QPaintEvent* event);