Merge lp:~unity-2d-team/unity-2d/unity-2d-shell-panel-dash-buttons into lp:~unity-2d-team/unity-2d/unity-2d-shell

[Ugo Riboni]

[dash][panel] Make the panel (un)maximize and close buttons work again with the dash

[Michał Sawicz (saviq)]

Any reason why we're based off of activePage and not active?

review: Needs Information

[Ugo Riboni (uriboni)]

I think basing on active is ok too. Can't think of any particular reason why I did it that way, TBH

[Michał Sawicz (saviq)]

Seems better to me to check for the value of "active" on dash instead of activePage.

review: Needs Fixing

[lp:~unity-2d-team/unity-2d/unity-2d-shell-panel-dash-buttons]

947. By Albert Astals Cid on 2012-01-31

Use dashActive instead of dashActivePage to determine if the dash is active or not as requested in the MR

948. By Albert Astals Cid on 2012-01-31

Use setActive(false) instead of setActivePage("")

[Albert Astals Cid (aacid)]

Requested fixes done

[lp:~unity-2d-team/unity-2d/unity-2d-shell-panel-dash-buttons]

949. By Albert Astals Cid on 2012-01-31

merge

[Michał Sawicz (saviq)]

review: Approve

=== modified file 'libunity-2d-private/src/dashclient.cpp'
--- libunity-2d-private/src/dashclient.cpp	2012-01-27 11:34:36 +0000
+++ libunity-2d-private/src/dashclient.cpp	2012-01-31 15:27:35 +0000
@@ -92,7 +92,7 @@
 
     QVariant value = m_dashDbusIface->property("active");
     if (value.isValid()) {
-        m_dashActive = value.toBool();
+        slotDashActiveChanged(value.toBool());
     } else {
         UQ_WARNING << "Fetching Dash.active property failed";
     }
@@ -117,6 +117,7 @@
     if (m_dashActive != value) {
         m_dashActive = value;
         updateActivePage();
+        Q_EMIT activeChanged(value);
     }
 }
 
@@ -128,6 +129,24 @@
     }
 }
 
+bool DashClient::active() const
+{
+    return m_dashActive;
+}
+
+void DashClient::setActive(bool active)
+{
+    if (!active) {
+        // Use m_dashDbusIface to close the dash, but only if it is running
+        if (m_dashDbusIface) {
+            m_dashDbusIface->setProperty("active", false);
+        }
+    } else {
+        QDBusInterface iface(DASH_DBUS_SERVICE, DASH_DBUS_PATH, DASH_DBUS_INTERFACE);
+        iface.setProperty("active", true);
+    }
+}
+
 QString DashClient::activePage() const
 {
     return m_activePage;
@@ -139,10 +158,7 @@
         return;
     }
     if (page.isEmpty()) {
-        // Use m_dashDbusIface to close the dash, but only if it is running
-        if (m_dashDbusIface) {
-            m_dashDbusIface->setProperty("active", false);
-        }
+        setActive(false);
         return;
     }
     // Use a separate QDBusInterface so that the dash is started if it is not

=== modified file 'libunity-2d-private/src/dashclient.h'
--- libunity-2d-private/src/dashclient.h	2012-01-24 16:46:52 +0000
+++ libunity-2d-private/src/dashclient.h	2012-01-31 15:27:35 +0000
@@ -40,6 +40,8 @@
 public:
     static DashClient* instance();
 
+    bool active() const;
+    void setActive(bool active);
     /**
      * Returns the active page. This is either:
      * - The lens id of the active lens
@@ -52,6 +54,7 @@
     static QSize minimumSizeForDesktop();
 
 Q_SIGNALS:
+    void activeChanged(bool active);
     void activePageChanged(const QString&);
     void alwaysFullScreenChanged();
 

=== modified file 'panel/applets/appname/windowhelper.cpp'
--- panel/applets/appname/windowhelper.cpp	2012-01-24 13:03:34 +0000
+++ panel/applets/appname/windowhelper.cpp	2012-01-31 15:27:35 +0000
@@ -30,6 +30,7 @@
 #include <debug_p.h>
 #include <gconnector.h>
 #include <screeninfo.h>
+#include <dashclient.h>
 
 // Bamf
 #include <bamf-matcher.h>
@@ -57,7 +58,9 @@
 {
     WnckWindow* m_window;
     GConnector m_connector;
-    bool m_activeWindowIsDash;
+    bool m_activeWindowIsShell;
+    bool m_dashIsVisible;
+    bool m_dashIsFullScreen;
 };
 
 WindowHelper::WindowHelper(QObject* parent)
@@ -82,6 +85,15 @@
     // last window is closed. Should be removed when this bug is fixed.
     connect(&BamfMatcher::get_default(), SIGNAL(ViewClosed(BamfView*)),
         SLOT(update()));
+
+    connect(DashClient::instance(), SIGNAL(activeChanged(bool)), SLOT(updateDashVisible(bool)));
+    d->m_dashIsVisible = DashClient::instance()->active();
+
+    // FIXME: the queued connection should not be needed, however if it's not used when
+    // (un)maximizing the dash, the panel will deadlock for some reason.
+    connect(&dash2dConfiguration(), SIGNAL(fullScreenChanged(bool)), SLOT(updateDashFullScreen(bool)),
+            Qt::QueuedConnection);
+    d->m_dashIsFullScreen = dash2dConfiguration().property("fullScreen").toBool();
 }
 
 WindowHelper::~WindowHelper()
@@ -103,6 +115,18 @@
     QMetaObject::invokeMethod(watcher, "nameChanged");
 }
 
+void WindowHelper::updateDashVisible(bool visible)
+{
+    d->m_dashIsVisible = visible;
+    update();
+}
+
+void WindowHelper::updateDashFullScreen(bool fullScreen)
+{
+    d->m_dashIsFullScreen = fullScreen;
+    update();
+}
+
 void WindowHelper::update()
 {
     BamfWindow* bamfWindow = BamfMatcher::get_default().active_window();
@@ -116,8 +140,7 @@
         d->m_window = wnck_window_get(xid);
 
         const char *name = wnck_window_get_name(d->m_window);
-        d->m_activeWindowIsDash = qstrcmp(name, "unity-2d-places") == 0;
-
+        d->m_activeWindowIsShell = qstrcmp(name, "unity-2d-shell") == 0;
         d->m_connector.connect(G_OBJECT(d->m_window), "name-changed", G_CALLBACK(nameChangedCB), this);
         d->m_connector.connect(G_OBJECT(d->m_window), "state-changed", G_CALLBACK(stateChangedCB), this);
     }
@@ -130,8 +153,9 @@
     if (!d->m_window) {
         return false;
     }
-    if (d->m_activeWindowIsDash) {
-        return dash2dConfiguration().property("fullScreen").toBool();
+
+    if (d->m_activeWindowIsShell) {
+        return d->m_dashIsVisible && d->m_dashIsFullScreen;
     } else {
         return wnck_window_is_maximized(d->m_window);
     }
@@ -162,40 +186,46 @@
 
 bool WindowHelper::dashIsVisible() const
 {
-    return d->m_window != 0 && d->m_activeWindowIsDash;
+    return d->m_window != 0 && d->m_activeWindowIsShell && d->m_dashIsVisible;
 }
 
 void WindowHelper::close()
 {
-    guint32 timestamp = QDateTime::currentDateTime().toTime_t();
-    wnck_window_close(d->m_window, timestamp);
+    if (d->m_window != 0 && !d->m_activeWindowIsShell) {
+        guint32 timestamp = QDateTime::currentDateTime().toTime_t();
+        wnck_window_close(d->m_window, timestamp);
+    } else {
+        DashClient::instance()->setActive(false);
+    }
 }
 
 void WindowHelper::minimize()
 {
-    if (!d->m_activeWindowIsDash) {
+    if (d->m_window != 0 && !d->m_activeWindowIsShell) {
         wnck_window_minimize(d->m_window);
     }
+    // If the dash is up, we can't get here, since the minimize button
+    // should be greyed out or hidden in that case.
 }
 
 void WindowHelper::maximize()
 {
-    if (d->m_activeWindowIsDash) {
-        dash2dConfiguration().setProperty("fullScreen", QVariant(true));
-    } else {
+    if (d->m_window != 0 && !d->m_activeWindowIsShell) {
         /* This currently cannot happen, because the window buttons are not
          * shown in the panel for non maximized windows. It's here just for
          * completeness. */
         wnck_window_maximize(d->m_window);
+    } else {
+        dash2dConfiguration().setProperty("fullScreen", QVariant(true));
     }
 }
 
 void WindowHelper::unmaximize()
 {
-    if (d->m_activeWindowIsDash) {
+    if (d->m_window != 0 && !d->m_activeWindowIsShell) {
+        wnck_window_unmaximize(d->m_window);
+    } else {
         dash2dConfiguration().setProperty("fullScreen", QVariant(false));
-    } else {
-        wnck_window_unmaximize(d->m_window);
     }
 }
 

=== modified file 'panel/applets/appname/windowhelper.h'
--- panel/applets/appname/windowhelper.h	2012-01-24 13:03:34 +0000
+++ panel/applets/appname/windowhelper.h	2012-01-31 15:27:35 +0000
@@ -53,6 +53,8 @@
 
 private Q_SLOTS:
     void update();
+    void updateDashVisible(bool);
+    void updateDashFullScreen(bool);
 
 Q_SIGNALS:
     void nameChanged();

=== modified file 'tests/places/fullscreen.rb'
--- tests/places/fullscreen.rb	2012-01-27 12:39:09 +0000
+++ tests/places/fullscreen.rb	2012-01-31 15:27:35 +0000
@@ -116,9 +116,7 @@
     }
   end
 
-  # FIXME: this test is temporarily disabled until we add back the panel buttons for the dash in
-  # the shell branch.
-  xtest "Dash reacts correctly to panel buttons" do
+  test "Dash reacts correctly to panel buttons" do
     %x{dconf write #{DASH_FULLSCREEN_KEY} false}
     %x{dconf write #{DASH_FORMFACTOR_KEY} #{'\"desktop\"'}}
     XDo::Keyboard.super
@@ -128,7 +126,7 @@
     }
 
     maxbutton = nil
-    verify(TIMEOUT, 'The "maximize" button did not appear when the dash was visible' ) {
+    verify(1, 'The "maximize" button did not appear when the dash was visible' ) {
         maxbutton = @panel.AppNameApplet().children( :type => 'QAbstractButton' )[2]
     }