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
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.

https://bugzilla.gnome.org/show_bug.cgi?id=657082

js/ui/popupMenu.js

@@ -860,6 +860,39 @@ PopupMenuBase.prototype = {
         }));
     },
 
+    _updateSeparatorVisibility: function(menuItem) {
+        let children = this.box.get_children();
+
+        let index = children.indexOf(menuItem.actor);
+
+        if (index < 0)
+            return;
+
+        let childBeforeIndex = index - 1;
+
+        while (childBeforeIndex >= 0 && !children[childBeforeIndex].visible)
+            childBeforeIndex--;
+
+        if (childBeforeIndex < 0
+            || children[childBeforeIndex]._delegate instanceof PopupSeparatorMenuItem) {
+            menuItem.actor.hide();
+            return;
+        }
+
+        let childAfterIndex = index + 1;
+
+        while (childAfterIndex < children.length && !children[childAfterIndex].visible)
+            childAfterIndex++;
+
+        if (childAfterIndex >= children.length
+            || children[childAfterIndex]._delegate instanceof PopupSeparatorMenuItem) {
+            menuItem.actor.hide();
+            return;
+        }
+
+        menuItem.actor.show();
+    },
+
     addMenuItem: function(menuItem, position) {
         let before_item = null;
         if (position == undefined) {
@@ -891,6 +924,14 @@ 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); }));
         } else if (menuItem instanceof PopupBaseMenuItem)
             this._connectItemSignals(menuItem);
         else

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);

js/ui/status/network.js

@@ -1558,9 +1558,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 = { };
 
@@ -1571,9 +1571,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(),
@@ -1581,9 +1581,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(),
@@ -1591,9 +1591,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(),
@@ -1606,9 +1606,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();

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(new PopupMenu.PopupSeparatorMenuItem());
-        this.menu.addMenuItem(this._deviceSep);
         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();
         }));
     },
 

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(new PopupMenu.PopupSeparatorMenuItem());
-        this.menu.addMenuItem(this._separator);
 
         this._input = null;
         this._inputVolumeId = 0;
@@ -145,7 +144,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();
         }
@@ -168,11 +166,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();
         }

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..."));