[gnome-shell/wip/gdm-shell: 3/16] popupMenu: Hide separators when they aren't separating

[Ray Strode <halfline src gnome org>]

commit 7059a17af340b9bb44b8a6595716392324f2fbb1
Author: Ray Strode <rstrode redhat com>
Date:   Fri Aug 19 14:42:20 2011 -0400

    popupMenu: Hide separators when they aren't separating
    
    A separator only makes sense if there are items on both
    sides of it.  There is quite a lot of code written
    spread throughout the shell that manages the process
    of showing and hiding separators as the items around those
    separators change.
    
    This commit drops all that code in favor of changes to the menu
    implementation to dynamically hide or show separators as
    appropriate, so the callers don't have to deal with it.

 js/ui/popupMenu.js        |   46 +++++++++++++++++++++++++++++++++++++++++++++
 js/ui/status/bluetooth.js |    7 ------
 js/ui/status/network.js   |   10 ++++----
 js/ui/status/power.js     |   13 +-----------
 js/ui/status/volume.js    |    6 +----
 js/ui/statusMenu.js       |   28 ---------------------------
 6 files changed, 53 insertions(+), 57 deletions(-)
---
diff --git a/js/ui/popupMenu.js b/js/ui/popupMenu.js
index 9ee304d..fbf1fc6 100644
--- a/js/ui/popupMenu.js
+++ b/js/ui/popupMenu.js
@@ -860,6 +860,43 @@ PopupMenuBase.prototype = {
         }));
     },
 
+    _updateSeparatorVisibility: function(menuItem) {
+        if (menuItem.actor.get_parent() != this.box)
+            return;
+
+        let childBefore = this.box.get_actor_before(menuItem.actor);
+
+        while (childBefore && !childBefore.visible)
+            childBefore = this.box.get_actor_before(childBefore);
+
+        if (!childBefore) {
+            menuItem.actor.hide();
+            return;
+        }
+
+        if (childBefore._delegate instanceof PopupSeparatorMenuItem) {
+            menuItem.actor.hide();
+            return;
+        }
+
+        let childAfter = this.box.get_actor_after(menuItem.actor);
+
+        while (childAfter && !childAfter.visible)
+            childAfter = this.box.get_actor_after(childAfter);
+
+        if (!childAfter) {
+            menuItem.actor.hide();
+            return;
+        }
+
+        if (childAfter._delegate instanceof PopupSeparatorMenuItem) {
+            menuItem.actor.hide();
+            return;
+        }
+
+        menuItem.actor.show();
+    },
+
     addMenuItem: function(menuItem, position) {
         let before_item = null;
         if (position == undefined) {
@@ -891,6 +928,15 @@ PopupMenuBase.prototype = {
                 if (!open)
                     menuItem.menu.close(false);
             });
+        } else if (menuItem instanceof PopupSeparatorMenuItem) {
+            this._connectItemSignals(menuItem);
+
+            // updateSeparatorVisibility needs to get called any time the
+            // separator's adjacent siblings change visibility or position.
+            // open-state-changed isn't exactly that, but doing it in more
+            // precise ways would require a lot more bookkeeping.
+            this.connect('open-state-changed', Lang.bind(this, function() { this._updateSeparatorVisibility(menuItem); }));
+            this._updateSeparatorVisibility(menuItem);
         } else if (menuItem instanceof PopupBaseMenuItem)
             this._connectItemSignals(menuItem);
         else
diff --git a/js/ui/status/bluetooth.js b/js/ui/status/bluetooth.js
index d506996..2970b2c 100644
--- a/js/ui/status/bluetooth.js
+++ b/js/ui/status/bluetooth.js
@@ -67,7 +67,6 @@ Indicator.prototype = {
                                new PopupMenu.PopupMenuItem(_("Set up a New Device...")),
                                new PopupMenu.PopupSeparatorMenuItem()];
         this._hasDevices = false;
-        this._deviceSep = this._fullMenuItems[0]; // hidden if no device exists
 
         this._fullMenuItems[1].connect('activate', function() {
             GLib.spawn_command_line_async('bluetooth-sendto');
@@ -162,10 +161,6 @@ Indicator.prototype = {
                 this._hasDevices = true;
             }
         }
-        if (this._hasDevices)
-            this._deviceSep.actor.show();
-        else
-            this._deviceSep.actor.hide();
     },
 
     _updateDeviceItem: function(item, device) {
@@ -303,8 +298,6 @@ Indicator.prototype = {
             this._showAll(this._fullMenuItems);
             if (this._hasDevices)
                 this._showAll(this._deviceItems);
-            else
-                this._deviceSep.actor.hide();
         } else {
             this._hideAll(this._fullMenuItems);
             this._hideAll(this._deviceItems);
diff --git a/js/ui/status/network.js b/js/ui/status/network.js
index e2ae3f1..4e988ad 100644
--- a/js/ui/status/network.js
+++ b/js/ui/status/network.js
@@ -1554,9 +1554,9 @@ NMApplet.prototype = {
         this._statusSection.addAction(_("Enable networking"), Lang.bind(this, function() {
             this._client.networking_enabled = true;
         }));
-        this._statusSection.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._statusSection.actor.hide();
         this.menu.addMenuItem(this._statusSection);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._devices = { };
 
@@ -1567,9 +1567,9 @@ NMApplet.prototype = {
         };
 
         this._devices.wired.section.addMenuItem(this._devices.wired.item);
-        this._devices.wired.section.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._devices.wired.section.actor.hide();
         this.menu.addMenuItem(this._devices.wired.section);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._devices.wireless = {
             section: new PopupMenu.PopupMenuSection(),
@@ -1577,9 +1577,9 @@ NMApplet.prototype = {
             item: this._makeToggleItem('wireless', _("Wireless"))
         };
         this._devices.wireless.section.addMenuItem(this._devices.wireless.item);
-        this._devices.wireless.section.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._devices.wireless.section.actor.hide();
         this.menu.addMenuItem(this._devices.wireless.section);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._devices.wwan = {
             section: new PopupMenu.PopupMenuSection(),
@@ -1587,9 +1587,9 @@ NMApplet.prototype = {
             item: this._makeToggleItem('wwan', _("Mobile broadband"))
         };
         this._devices.wwan.section.addMenuItem(this._devices.wwan.item);
-        this._devices.wwan.section.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._devices.wwan.section.actor.hide();
         this.menu.addMenuItem(this._devices.wwan.section);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._devices.vpn = {
             section: new PopupMenu.PopupMenuSection(),
@@ -1602,9 +1602,9 @@ NMApplet.prototype = {
         this._devices.vpn.item.updateForDevice(this._devices.vpn.device);
         this._devices.vpn.section.addMenuItem(this._devices.vpn.item);
         this._devices.vpn.section.addMenuItem(this._devices.vpn.device.section);
-        this._devices.vpn.section.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._devices.vpn.section.actor.hide();
         this.menu.addMenuItem(this._devices.vpn.section);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this.menu.addAction(_("Network Settings"), function() {
             Main.overview.hide();
diff --git a/js/ui/status/power.js b/js/ui/status/power.js
index 482711b..9b88ac3 100644
--- a/js/ui/status/power.js
+++ b/js/ui/status/power.js
@@ -75,8 +75,7 @@ Indicator.prototype = {
         this._batteryItem.addActor(this._primaryPercentage, { align: St.Align.END });
         this.menu.addMenuItem(this._batteryItem);
 
-        this._deviceSep = new PopupMenu.PopupSeparatorMenuItem();
-        this.menu.addMenuItem(this._deviceSep);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._otherDevicePosition = 2;
         this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
@@ -96,7 +95,6 @@ Indicator.prototype = {
                 this._hasPrimary = false;
                 this._primaryDeviceId = null;
                 this._batteryItem.actor.hide();
-                this._deviceSep.actor.hide();
                 return;
             }
             let [device_id, device_type, icon, percentage, state, seconds] = device;
@@ -126,12 +124,9 @@ Indicator.prototype = {
                 }
                 this._primaryPercentage.text = Math.round(percentage) + '%';
                 this._batteryItem.actor.show();
-                if (this._deviceItems.length > 0)
-                    this._deviceSep.actor.show();
             } else {
                 this._hasPrimary = false;
                 this._batteryItem.actor.hide();
-                this._deviceSep.actor.hide();
             }
 
             this._primaryDeviceId = device_id;
@@ -144,7 +139,6 @@ Indicator.prototype = {
             this._deviceItems = [];
 
             if (error) {
-                this._deviceSep.actor.hide();
                 return;
             }
 
@@ -159,11 +153,6 @@ Indicator.prototype = {
                 this.menu.addMenuItem(item, this._otherDevicePosition + position);
                 position++;
             }
-
-            if (this._hasPrimary && position > 0)
-                this._deviceSep.actor.show();
-            else
-                this._deviceSep.actor.hide();
         }));
     },
 
diff --git a/js/ui/status/volume.js b/js/ui/status/volume.js
index 0c02a9e..3e3e8c9 100644
--- a/js/ui/status/volume.js
+++ b/js/ui/status/volume.js
@@ -47,8 +47,7 @@ Indicator.prototype = {
         this.menu.addMenuItem(this._outputTitle);
         this.menu.addMenuItem(this._outputSlider);
 
-        this._separator = new PopupMenu.PopupSeparatorMenuItem();
-        this.menu.addMenuItem(this._separator);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._input = null;
         this._inputVolumeId = 0;
@@ -140,7 +139,6 @@ Indicator.prototype = {
             this._mutedChanged (null, null, '_input');
             this._volumeChanged (null, null, '_input');
         } else {
-            this._separator.actor.hide();
             this._inputTitle.actor.hide();
             this._inputSlider.actor.hide();
         }
@@ -163,11 +161,9 @@ Indicator.prototype = {
             }
         }
         if (showInput) {
-            this._separator.actor.show();
             this._inputTitle.actor.show();
             this._inputSlider.actor.show();
         } else {
-            this._separator.actor.hide();
             this._inputTitle.actor.hide();
             this._inputSlider.actor.hide();
         }
diff --git a/js/ui/statusMenu.js b/js/ui/statusMenu.js
index 21bc2b1..2f16cbc 100644
--- a/js/ui/statusMenu.js
+++ b/js/ui/statusMenu.js
@@ -115,35 +115,12 @@ StatusMenuButton.prototype = {
           this._name.set_text("");
     },
 
-    _updateSessionSeparator: function() {
-        let sessionItemsVisible = this._loginScreenItem.actor.visible ||
-                                  this._logoutItem.actor.visible ||
-                                  this._lockScreenItem.actor.visible;
-
-        let showSessionSeparator = sessionItemsVisible &&
-                                   this._suspendOrPowerOffItem.actor.visible;
-
-        let showSettingsSeparator = sessionItemsVisible ||
-                                    this._suspendOrPowerOffItem.actor.visible;
-
-        if (showSessionSeparator)
-            this._sessionSeparator.actor.show();
-        else
-            this._sessionSeparator.actor.hide();
-
-        if (showSettingsSeparator)
-            this._settingsSeparator.actor.show();
-        else
-            this._settingsSeparator.actor.hide();
-    },
-
     _updateSwitchUser: function() {
         let allowSwitch = !this._lockdownSettings.get_boolean(DISABLE_USER_SWITCH_KEY);
         if (allowSwitch && this._gdm.can_switch ())
             this._loginScreenItem.actor.show();
         else
             this._loginScreenItem.actor.hide();
-        this._updateSessionSeparator();
     },
 
     _updateLogout: function() {
@@ -152,7 +129,6 @@ StatusMenuButton.prototype = {
             this._logoutItem.actor.show();
         else
             this._logoutItem.actor.hide();
-        this._updateSessionSeparator();
     },
 
     _updateLockScreen: function() {
@@ -161,7 +137,6 @@ StatusMenuButton.prototype = {
             this._lockScreenItem.actor.show();
         else
             this._lockScreenItem.actor.hide();
-        this._updateSessionSeparator();
     },
 
     _updateHaveShutdown: function() {
@@ -184,7 +159,6 @@ StatusMenuButton.prototype = {
             this._suspendOrPowerOffItem.actor.hide();
         else
             this._suspendOrPowerOffItem.actor.show();
-         this._updateSessionSeparator();
 
         // If we can't suspend show Power Off... instead
         // and disable the alt key
@@ -237,7 +211,6 @@ StatusMenuButton.prototype = {
 
         item = new PopupMenu.PopupSeparatorMenuItem();
         this.menu.addMenuItem(item);
-        this._settingsSeparator = item;
 
         item = new PopupMenu.PopupMenuItem(_("Lock Screen"));
         item.connect('activate', Lang.bind(this, this._onLockScreenActivate));
@@ -256,7 +229,6 @@ StatusMenuButton.prototype = {
 
         item = new PopupMenu.PopupSeparatorMenuItem();
         this.menu.addMenuItem(item);
-        this._sessionSeparator = item;
 
         item = new PopupMenu.PopupAlternatingMenuItem(_("Suspend"),
                                                       _("Power Off..."));