From f96b2ee8580b9a72e48ba7c823e2f0b2a2fca574 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Fri, 19 Aug 2011 14:42:20 -0400 Subject: [PATCH] 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 | 41 +++++++++++++++++++++++++++++++++++++++ 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, 48 insertions(+), 57 deletions(-) diff --git a/js/ui/popupMenu.js b/js/ui/popupMenu.js index 9ee304dab..228fc76e2 100644 --- a/js/ui/popupMenu.js +++ b/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 diff --git a/js/ui/status/bluetooth.js b/js/ui/status/bluetooth.js index d5069963f..2970b2c08 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 2f65d1a3a..447efffcf 100644 --- a/js/ui/status/network.js +++ b/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(); diff --git a/js/ui/status/power.js b/js/ui/status/power.js index 482711bf5..9b88ac371 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 98e8d30f6..406ac6641 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; @@ -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(); } diff --git a/js/ui/statusMenu.js b/js/ui/statusMenu.js index 21bc2b128..2f16cbc42 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..."));