system: Replace action icons with regular menu items

Besides making the menu a bit less special, it allows us to fit both
shutdown and suspend actions without any hidden alt-key Easter eggs.

https://gitlab.gnome.org/GNOME/gnome-shell/issues/270

data/theme/gnome-shell-sass/_common.scss

@@ -1190,7 +1190,8 @@ StScrollBar {
 
   .aggregate-menu {
     min-width: 21em;
-    .popup-menu-icon { padding: 0 4px; }
+    .popup-menu-icon { padding: 0 4px;
+                       -st-icon-style: symbolic; }
     .popup-sub-menu .popup-menu-item > :first-child {
       &:ltr { /* 12px spacing + 2*4px padding */
               padding-left: 20px; margin-left: 1.09em; }
@@ -1199,27 +1200,6 @@ StScrollBar {
     }
   }
 
-  .system-menu-action {
-    -st-icon-style: symbolic;
-    color: $fg_color;
-    border-radius: 32px; /* wish we could do 50% */
-    padding: 13px;
-    border: 1px solid $_bubble_borders_color;
-
-    &:hover, &:focus {
-      background-color: $_hover_bg_color;
-      color: $fg_color;
-      border: none;
-      padding: 14px;
-    }
-    &:active { 
-      background-color: $selected_bg_color;
-      color: $selected_fg_color;
-    }
-
-    & > StIcon { icon-size: 16px; }
-  }
-
 // Activities Ripples
 .ripple-box {
   width: 52px;

js/ui/panel.js

@@ -760,12 +760,13 @@ class AggregateMenu extends PanelMenu.Button {
         this.menu.addMenuItem(this._rfkill.menu);
         this.menu.addMenuItem(this._power.menu);
         this.menu.addMenuItem(this._nightLight.menu);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this.menu.addMenuItem(this._system.menu);
 
         menuLayout.addSizeChild(this._location.menu.actor);
         menuLayout.addSizeChild(this._rfkill.menu.actor);
         menuLayout.addSizeChild(this._power.menu.actor);
-        menuLayout.addSizeChild(this._system.buttonGroup);
+        menuLayout.addSizeChild(this._system.menu.actor);
     }
 });
 

js/ui/status/system.js

@@ -1,7 +1,7 @@
 // -*- mode: js; js-indent-level: 4; indent-tabs-mode: nil -*-
 /* exported Indicator */
 
-const { AccountsService, Clutter, GLib, GObject, Shell, St } = imports.gi;
+const { GObject, Shell, St } = imports.gi;
 
 const BoxPointer = imports.ui.boxpointer;
 const SystemActions = imports.misc.systemActions;
@@ -10,120 +10,11 @@ const PanelMenu = imports.ui.panelMenu;
 const PopupMenu = imports.ui.popupMenu;
 
 
-var AltSwitcher = GObject.registerClass(
-class AltSwitcher extends St.Bin {
-    _init(standard, alternate) {
-        super._init();
-        this._standard = standard;
-        this._standard.connect('notify::visible', this._sync.bind(this));
-        if (this._standard instanceof St.Button)
-            this._standard.connect('clicked',
-                                   () => this._clickAction.release());
-
-        this._alternate = alternate;
-        this._alternate.connect('notify::visible', this._sync.bind(this));
-        if (this._alternate instanceof St.Button)
-            this._alternate.connect('clicked',
-                                    () => this._clickAction.release());
-
-        this._capturedEventId = global.stage.connect('captured-event', this._onCapturedEvent.bind(this));
-
-        this._flipped = false;
-
-        this._clickAction = new Clutter.ClickAction();
-        this._clickAction.connect('long-press', this._onLongPress.bind(this));
-
-        this.connect('destroy', this._onDestroy.bind(this));
-    }
-
-    vfunc_map() {
-        super.vfunc_map();
-        this._flipped = false;
-    }
-
-    vfunc_unmap() {
-        super.vfunc_unmap();
-        this._flipped = false;
-    }
-
-    _sync() {
-        let childToShow = null;
-
-        if (this._standard.visible && this._alternate.visible) {
-            let [x_, y_, mods] = global.get_pointer();
-            let altPressed = (mods & Clutter.ModifierType.MOD1_MASK) != 0;
-            if (this._flipped)
-                childToShow = altPressed ? this._standard : this._alternate;
-            else
-                childToShow = altPressed ? this._alternate : this._standard;
-        } else if (this._standard.visible) {
-            childToShow = this._standard;
-        } else if (this._alternate.visible) {
-            childToShow = this._alternate;
-        } else {
-            this.hide();
-            return;
-        }
-
-        let childShown = this.get_child();
-        if (childShown != childToShow) {
-            if (childShown) {
-                if (childShown.fake_release)
-                    childShown.fake_release();
-                childShown.remove_action(this._clickAction);
-            }
-            childToShow.add_action(this._clickAction);
-
-            let hasFocus = this.contains(global.stage.get_key_focus());
-            this.set_child(childToShow);
-            if (hasFocus)
-                childToShow.grab_key_focus();
-
-            // The actors might respond to hover, so
-            // sync the pointer to make sure they update.
-            global.sync_pointer();
-        }
-
-        this.show();
-    }
-
-    _onDestroy() {
-        if (this._capturedEventId > 0) {
-            global.stage.disconnect(this._capturedEventId);
-            this._capturedEventId = 0;
-        }
-    }
-
-    _onCapturedEvent(actor, event) {
-        let type = event.type();
-        if (type == Clutter.EventType.KEY_PRESS || type == Clutter.EventType.KEY_RELEASE) {
-            let key = event.get_key_symbol();
-            if (key == Clutter.KEY_Alt_L || key == Clutter.KEY_Alt_R)
-                this._sync();
-        }
-
-        return Clutter.EVENT_PROPAGATE;
-    }
-
-    _onLongPress(action, actor, state) {
-        if (state == Clutter.LongPressState.QUERY ||
-            state == Clutter.LongPressState.CANCEL)
-            return true;
-
-        this._flipped = !this._flipped;
-        this._sync();
-        return true;
-    }
-});
-
 var Indicator = GObject.registerClass(
 class Indicator extends PanelMenu.SystemIndicator {
     _init() {
         super._init();
 
-        let userManager = AccountsService.UserManager.get_default();
-        this._user = userManager.get_user(GLib.get_user_name());
-
         this._systemActions = new SystemActions.getDefault();
 
         this._createSubMenu();
@@ -149,97 +40,36 @@ class Indicator extends PanelMenu.SystemIndicator {
     }
 
     _sessionUpdated() {
-        this._settingsAction.visible = Main.sessionMode.allowSettings;
+        this._settingsItem.visible = Main.sessionMode.allowSettings;
     }
 
     _updateMultiUser() {
         let hasSwitchUser = this._loginScreenItem.visible;
         let hasLogout = this._logoutItem.visible;
 
-        this._switchUserSubMenu.visible = hasSwitchUser || hasLogout;
+        this._sessionSubMenu.visible = hasSwitchUser || hasLogout;
-    }
-
-    _updateSwitchUserSubMenu() {
-        this._switchUserSubMenu.label.text = this._user.get_real_name();
-        let clutterText = this._switchUserSubMenu.label.clutter_text;
-
-        // XXX -- for some reason, the ClutterText's width changes
-        // rapidly unless we force a relayout of the actor. Probably
-        // a size cache issue or something. Moving this to be a layout
-        // manager would be a much better idea.
-        clutterText.get_allocation_box();
-
-        let layout = clutterText.get_layout();
-        if (layout.is_ellipsized())
-            this._switchUserSubMenu.label.text = this._user.get_user_name();
-    }
-
-    _createActionButton(iconName, accessibleName) {
-        return new St.Button({
-            child: new St.Icon({ icon_name: iconName }),
-            reactive: true,
-            can_focus: true,
-            track_hover: true,
-            accessible_name: accessibleName,
-            x_expand: true,
-            x_align: Clutter.ActorAlign.CENTER,
-            style_class: 'system-menu-action',
-        });
     }
 
     _createSubMenu() {
         let bindFlags = GObject.BindingFlags.DEFAULT | GObject.BindingFlags.SYNC_CREATE;
         let item;
 
-        this._switchUserSubMenu = new PopupMenu.PopupSubMenuMenuItem('', true);
+        item = new PopupMenu.PopupImageMenuItem(_('Lock Screen Rotation'),
-        this._switchUserSubMenu.icon.icon_name = 'avatar-default-symbolic';
+            this._systemActions.orientation_lock_icon);
-
-        // Since the label of the switch user submenu depends on the width of
-        // the popup menu, and we can't easily connect on allocation-changed
-        // or notify::width without creating layout cycles, simply update the
-        // label whenever the menu is opened.
-        this.menu.connect('open-state-changed', (menu, isOpen) => {
-            if (isOpen)
-                this._updateSwitchUserSubMenu();
-        });
-
-        item = new PopupMenu.PopupMenuItem(_("Switch User"));
         item.connect('activate', () => {
             this.menu.itemActivated(BoxPointer.PopupAnimation.NONE);
-            this._systemActions.activateSwitchUser();
+            this._systemActions.activateLockOrientation();
         });
-        this._switchUserSubMenu.menu.addMenuItem(item);
+        this.menu.addMenuItem(item);
-        this._loginScreenItem = item;
+        this._orientationLockItem = item;
-        this._systemActions.bind_property('can-switch-user',
+        this._systemActions.bind_property('can-lock-orientation',
-                                          this._loginScreenItem,
+                                          this._orientationLockItem,
                                           'visible',
                                           bindFlags);
-
+        this._systemActions.connect('notify::orientation-lock-icon', () => {
-        item = new PopupMenu.PopupMenuItem(_("Log Out"));
+            let iconName = this._systemActions.orientation_lock_icon;
-        item.connect('activate', () => {
+            this._orientationLockItem.setIcon(iconName);
-            this.menu.itemActivated(BoxPointer.PopupAnimation.NONE);
-            this._systemActions.activateLogout();
         });
-        this._switchUserSubMenu.menu.addMenuItem(item);
-        this._logoutItem = item;
-        this._systemActions.bind_property('can-logout',
-                                          this._logoutItem,
-                                          'visible',
-                                          bindFlags);
-
-        this._switchUserSubMenu.menu.addSettingsAction(_("Account Settings"),
-                                                       'gnome-user-accounts-panel.desktop');
-
-        this._user.connect('notify::is-loaded', this._updateSwitchUserSubMenu.bind(this));
-        this._user.connect('changed', this._updateSwitchUserSubMenu.bind(this));
-
-        this.menu.addMenuItem(this._switchUserSubMenu);
-
-        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
-
-        item = new PopupMenu.PopupBaseMenuItem({ reactive: false,
-                                                 can_focus: false });
-        this.buttonGroup = item;
 
         let app = this._settingsApp = Shell.AppSystem.get_default().lookup_app(
             'gnome-control-center.desktop'
@@ -247,83 +77,85 @@ class Indicator extends PanelMenu.SystemIndicator {
         if (app) {
             let [icon, name] = [app.app_info.get_icon().names[0],
                                 app.get_name()];
-            this._settingsAction = this._createActionButton(icon, name);
+            item = new PopupMenu.PopupImageMenuItem(name, icon);
-            this._settingsAction.connect('clicked',
+            item.connect('activate', () => {
-                                         this._onSettingsClicked.bind(this));
+                this.menu.itemActivated(BoxPointer.PopupAnimation.NONE);
+                Main.overview.hide();
+                this._settingsApp.activate();
+            });
+            this.menu.addMenuItem(item);
+            this._settingsItem = item;
         } else {
             log('Missing required core component Settings, expect trouble…');
-            this._settingsAction = new St.Widget();
+            this._settingsItem = new St.Widget();
         }
-        item.add_child(this._settingsAction);
 
-        this._orientationLockAction = this._createActionButton('', _("Orientation Lock"));
+        item = new PopupMenu.PopupImageMenuItem(_('Lock'), 'changes-prevent-symbolic');
-        this._orientationLockAction.connect('clicked', () => {
+        item.connect('activate', () => {
-            this.menu.itemActivated(BoxPointer.PopupAnimation.NONE);
-            this._systemActions.activateLockOrientation();
-        });
-        item.add_child(this._orientationLockAction);
-        this._systemActions.bind_property('can-lock-orientation',
-                                          this._orientationLockAction,
-                                          'visible',
-                                          bindFlags);
-        this._systemActions.bind_property('orientation-lock-icon',
-                                          this._orientationLockAction.child,
-                                          'icon-name',
-                                          bindFlags);
-
-        this._lockScreenAction = this._createActionButton('changes-prevent', _("Lock"));
-        this._lockScreenAction.connect('clicked', () => {
             this.menu.itemActivated(BoxPointer.PopupAnimation.NONE);
             this._systemActions.activateLockScreen();
         });
-        item.add_child(this._lockScreenAction);
+        this.menu.addMenuItem(item);
+        this._lockScreenItem = item;
         this._systemActions.bind_property('can-lock-screen',
-                                          this._lockScreenAction,
+                                          this._lockScreenItem,
                                           'visible',
                                           bindFlags);
 
-        this._suspendAction = this._createActionButton('media-playback-pause', _("Suspend"));
+        this._sessionSubMenu = new PopupMenu.PopupSubMenuMenuItem(
-        this._suspendAction.connect('clicked', () => {
+            _('Power Off / Log Out'), true);
+        this._sessionSubMenu.icon.icon_name = 'system-shutdown-symbolic';
+
+        item = new PopupMenu.PopupMenuItem(_("Log Out"));
+        item.connect('activate', () => {
+            this.menu.itemActivated(BoxPointer.PopupAnimation.NONE);
+            this._systemActions.activateLogout();
+        });
+        this._sessionSubMenu.menu.addMenuItem(item);
+        this._logoutItem = item;
+        this._systemActions.bind_property('can-logout',
+                                          this._logoutItem,
+                                          'visible',
+                                          bindFlags);
+
+        item = new PopupMenu.PopupMenuItem(_("Switch User…"));
+        item.connect('activate', () => {
+            this.menu.itemActivated(BoxPointer.PopupAnimation.NONE);
+            this._systemActions.activateSwitchUser();
+        });
+        this._sessionSubMenu.menu.addMenuItem(item);
+        this._loginScreenItem = item;
+        this._systemActions.bind_property('can-switch-user',
+                                          this._loginScreenItem,
+                                          'visible',
+                                          bindFlags);
+
+        this._sessionSubMenu.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
+
+        item = new PopupMenu.PopupMenuItem(_("Suspend"));
+        item.connect('activate', () => {
             this.menu.itemActivated(BoxPointer.PopupAnimation.NONE);
             this._systemActions.activateSuspend();
         });
+        this._sessionSubMenu.menu.addMenuItem(item);
+        this._suspendItem = item;
         this._systemActions.bind_property('can-suspend',
-                                          this._suspendAction,
+                                          this._suspendItem,
                                           'visible',
                                           bindFlags);
 
-        this._powerOffAction = this._createActionButton('system-shutdown', _("Power Off"));
+        item = new PopupMenu.PopupMenuItem(_("Power Off…"));
-        this._powerOffAction.connect('clicked', () => {
+        item.connect('activate', () => {
             this.menu.itemActivated(BoxPointer.PopupAnimation.NONE);
             this._systemActions.activatePowerOff();
         });
+        this._sessionSubMenu.menu.addMenuItem(item);
+        this._powerOffItem = item;
         this._systemActions.bind_property('can-power-off',
-                                          this._powerOffAction,
+                                          this._powerOffItem,
                                           'visible',
                                           bindFlags);
 
-        this._altSwitcher = new AltSwitcher(this._powerOffAction, this._suspendAction);
+        this.menu.addMenuItem(this._sessionSubMenu);
-        item.add_child(this._altSwitcher);
-
-        this.menu.addMenuItem(item);
-
-        let visibilityGroup = [
-            this._settingsAction,
-            this._orientationLockAction,
-            this._lockScreenAction,
-            this._altSwitcher,
-        ];
-
-        for (let actor of visibilityGroup) {
-            actor.connect('notify::visible', () => {
-                this.buttonGroup.visible = visibilityGroup.some(a => a.visible);
-            });
-        }
-    }
-
-    _onSettingsClicked() {
-        this.menu.itemActivated();
-        Main.overview.hide();
-        this._settingsApp.activate();
     }
 });