After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 682546 - Lock screen - add the user name to the left-hand side of the top bar
Lock screen - add the user name to the left-hand side of the top bar
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: lock-screen
3.5.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on: 682542
Blocks:
 
 
Reported: 2012-08-23 13:50 UTC by Allan Day
Modified: 2012-09-04 22:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Panel: clean up button construction code (13.79 KB, patch)
2012-08-28 21:50 UTC, Giovanni Campagna
needs-work Details | Review
UserMenu: move to left and show a padlock icon in the screenshield (4.71 KB, patch)
2012-08-28 21:51 UTC, Giovanni Campagna
rejected Details | Review
Panel: clean up button construction code (13.76 KB, patch)
2012-08-29 00:07 UTC, Giovanni Campagna
committed Details | Review

Description Allan Day 2012-08-23 13:50:20 UTC
Also, show a lock icon to indicate if authentication is required.

See: https://raw.github.com/gnome-design-team/gnome-mockups/master/system-lock-login-boot/lock.png
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-08-23 15:54:11 UTC
Why the left-hand side?
Comment 2 Allan Day 2012-08-23 16:08:03 UTC
(In reply to comment #1)
> Why the left-hand side?

To distinguish it from the user menu, and to make the top bar visually balanced (I presume).
Comment 3 Giovanni Campagna 2012-08-28 21:50:56 UTC
Created attachment 222684 [details] [review]
Panel: clean up button construction code

We already could build the right part of the panel declaratively according
to the session mode. Extend that to handle the left and center parts.
Also, move the mapping from the roles to the classes in panel.js, as it shared
by all modes.
Comment 4 Giovanni Campagna 2012-08-28 21:51:19 UTC
Created attachment 222685 [details] [review]
UserMenu: move to left and show a padlock icon in the screenshield

When the screen is locked, the IM presence is forced to idle (away), so
it's not helpful. Show a padlock instead, to indicate that password
or other authentication method is required.
Also, move the user menu to the left of the panel when in the lock screen.

https://bugzilla.gnome.org/show_bug.cgi?id=681143
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-08-28 23:22:50 UTC
Review of attachment 222684 [details] [review]:

This is nice. Few nits, otherwise, cool.

I wonder if we could make this configurable with a GSetting :)

::: js/ui/panel.js
@@ +912,3 @@
+    'lockScreen': imports.ui.status.lockScreenMenu.Indicator,
+    'keyboard': imports.ui.status.keyboard.InputSourceIndicator,
+    'userMenu': imports.ui.userMenu.UserMenuButton

You forgot PowerMenu, used by GDM?

@@ +1126,3 @@
 
+    _addToPanelBox: function(role, indicator, position, box) {
+        // XXX: why this loop to find the right position?

Have you tried removing it? It seems really sketchy, you're right.

@@ +1160,3 @@
+
+        position = position || 0;
+        boxContainer = this['_' + (box || 'right') + 'Box'];

First of all, missing a "let". Second of all, ugh.

const boxes = {
    left: this._leftBox,
    center: this._centerBox,
    right: this._rightBox
};
let boxContainer = boxes[box] || this._rightBox;
Comment 6 Jasper St. Pierre (not reading bugmail) 2012-08-28 23:24:23 UTC
Review of attachment 222685 [details] [review]:

Add yet another tally to the "I'm quite sure we want a session mode". Not a fan.
Comment 7 Giovanni Campagna 2012-08-28 23:37:37 UTC
(In reply to comment #6)
> Review of attachment 222685 [details] [review]:
> 
> Add yet another tally to the "I'm quite sure we want a session mode". Not a
> fan.

Having a clean dynamic session mode support means much more than just hiding some actors.
What if the new session mode changes hasOverview? Do you make the current overview object a dummy?
What about hasExtensions? Do you unload them? What if the change was from an extension?
What about createSession? Do you want to recreate the session and register network, polkit, mountoperation, automount, etc. again?

ScreenShield.State is the "session sub-mode", in all respects. I can move the signals and getters to SessionMode, if you want.
Comment 8 Giovanni Campagna 2012-08-28 23:39:46 UTC
Forgot to add, this depends on bug 682542 for the state handling.
Comment 9 Jasper St. Pierre (not reading bugmail) 2012-08-28 23:48:38 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > Review of attachment 222685 [details] [review] [details]:
> > 
> > Add yet another tally to the "I'm quite sure we want a session mode". Not a
> > fan.
> 
> Having a clean dynamic session mode support means much more than just hiding
> some actors.
> What if the new session mode changes hasOverview? Do you make the current
> overview object a dummy?
> What about hasExtensions? Do you unload them? What if the change was from an
> extension?

I don't think we need to implement everything out of the gate, just not having 10 different ways to track the current settings of the sessions would be better. If we run into a transition we don't need to handle right now, we'll just log it and move on.

> What about createSession? Do you want to recreate the session and register
> network, polkit, mountoperation, automount, etc. again?

I've never liked the createSession approach.

> ScreenShield.State is the "session sub-mode", in all respects. I can move the
> signals and getters to SessionMode, if you want.
Comment 10 Giovanni Campagna 2012-08-29 00:07:34 UTC
Created attachment 222693 [details] [review]
Panel: clean up button construction code

We already could build the right part of the panel declaratively according
to the session mode. Extend that to handle the left and center parts.
Also, move the mapping from the roles to the classes in panel.js, as it shared
by all modes.
Comment 11 Jasper St. Pierre (not reading bugmail) 2012-08-29 00:42:17 UTC
Review of attachment 222693 [details] [review]:

Yes.
Comment 12 Jasper St. Pierre (not reading bugmail) 2012-08-30 21:56:06 UTC
Review of attachment 222693 [details] [review]:

::: js/ui/panel.js
@@ +1108,3 @@
+        this._initBox(panel.left, this._leftBox);
+        this._initBox(panel.center, this._centerBox);
+        this._initBox(panel.right, this._rightBox);

this._initBox('left');
this._initBox('center');
this._initBox('right');

@@ +1112,3 @@
+
+    _initBox: function(elements, box) {
+        for (let i = 0; i < elements.length; i++) {

let elements = Main.sessionMode.panel[box], boxActor = boxes[box];

@@ +1150,3 @@
+            right: this._rightBox
+        };
+        let boxContainer = boxes[box] || this._rightBox;

Wait, no, what you pass as the 'box' parameter is the actor. That's not going to work here.
Comment 13 Jasper St. Pierre (not reading bugmail) 2012-08-30 22:10:39 UTC
Review of attachment 222693 [details] [review]:

Nevermind, I misread. Good to go.
Comment 14 Giovanni Campagna 2012-09-03 21:09:19 UTC
Comment on attachment 222693 [details] [review]
Panel: clean up button construction code

Attachment 222693 [details] pushed as e5534e8 - Panel: clean up button construction code
Comment 15 Giovanni Campagna 2012-09-04 19:03:15 UTC
Comment on attachment 222685 [details] [review]
UserMenu: move to left and show a padlock icon in the screenshield

This is obsoleted by bug 683156.
Comment 16 Giovanni Campagna 2012-09-04 22:05:16 UTC
And this is fixed!