GNOME Bugzilla – Bug 658185
logged in users are not indicated
Last modified: 2012-08-01 20:29:40 UTC
While trying to fix fast user switching, I notice that the shell greeter does not indicate 'already logged in' in the user list, which makes it a bit surprising when you end up in an existing session.
hmm, i guess we need designer feedback here. Maybe we need show a radial gradient like we do for alreadying running apps in the dash, or put the text already logged in like we do in fallback greeter, or not do anything at all.
I saw mccann asking to be explicitly CCed for his input, so adding him.
*** Bug 660944 has been marked as a duplicate of this bug. ***
*** Bug 658967 has been marked as a duplicate of this bug. ***
bug 658967 points out we need to change the button text as well.
In bug 660913 a possible solution is discussed in dimming/desaturating all accounts and using full opacity/lightness and saturation for accounts with a running session. I would favor not playing with brightness or saturation, but add more focus to the users with a running session using the same 'spotlight' treatment we have in the overview/top bar in the shell. Will attach a mockup.
http://git.gnome.org/browse/gnome-shell-design/plain/mockups/static/user-selector.png The users Angela and Leonard have a running session, while Suzanne does not. In addition to the label color and the 'spotlight' backdrop, the active labels have a text-shadow. This is a similar treatment to running apps in the dash/overview.
So during keynav, how should the currently selected item look ? Right now it's essentially the horizontal line that in the mockup but with no radial gradient above it.
For :focus I would do the same as what we do in the overview for :hover - a semi-opaque white background.
The current function has a more opaque line. I would suggest to make the text for logged in users the same as the users not logged in (less opaque) but keep the black drop gradient. The opaque white text and white line would make a nice distinct keynav.
Created attachment 219255 [details] [review] loginDialog: Indicate whether users are logged in Unlike the fallback gdm UI, we do not indicate in the user list whether a user already has an open session or not. This information is useful, so use a spotlight effect similar to the running-app indicator to mark logged in users.
Its a nice glow. But it gets lost when I go to the password prompt and then back to the list by hitting Cancel. It should probably stay around, right ?
Oh, right - the glow needs to be hidden while the password entry is shown (at least for the selected user), but I'll need to add it back when going back to the user list ...
Created attachment 219567 [details] [review] loginDialog: Indicate whether users are logged in Rebase to master, fix style getting out-of-sync after cancelling password entry
Created attachment 219999 [details] [review] loginDialog: Indicate whether users are logged in Rebased to master
Review of attachment 219999 [details] [review]: This patch is fine, though, I have another idea you can take or leave. You could make the logged-in pseudoclass always reflect the logged-in state regardless of userlist state, add a new pseudoclass for the user list state and just bind it all together in the CSS file. (e.g. .login-dialog-user-list:expanded .login-dialog-user-list-item:logged-in { ... }). The advantage of doing it that way is it makes 'logged-in' mean 'logged-in' and not 'logged-in-and-not-user-list-not-shrunk' and adds a bit more flexibility later on should we end up needing it (You could imagine a :expanded type thing getting used for other future styling changes in the user list too I guess) Anyway, either way seems fine to me, so push away if you're happy with where its at right now. ::: js/gdm/loginDialog.js @@ +187,3 @@ + // actor.can_focus is only true when the user list is shrunk, and it's + // the easiest way for this part of the code to check for that + if (this.actor.can_focus && this.user.is_logged_in()) Just noticed the text is inverted. Sholud be this: "can_focus is only true when the user list is not shrunk,"
Created attachment 220074 [details] [review] loginDialog: Add an :expanded pseudo class to the user list We want to style user list items differently depending on whether the list is expanded or shrunk; instead of manually updating the items' style, we can just expose the :expanded style on the list itself and use that in the CSS.
Created attachment 220075 [details] [review] loginDialog: Indicate whether users are logged in I like that approach :-)
Review of attachment 220074 [details] [review]: We usually don't use too many custom pseudo-classes. Why not a regular style class?
Review of attachment 220075 [details] [review]: OK, yeah, this is not an appropriate use of a pseudo-class.
(In reply to comment #19) > We usually don't use too many custom pseudo-classes. Why not a regular style > class? I like to use regular classes to specify the element and pseudo classes to specify the element's state - using that definition, a pseudo class is appropriate here.
Grepping through, it seems we're half and half. As a quick example, the ActivitiesButton has a "overview" pseudo-class, but the panel itself has an "in-overview" style class. I still think "logged-in" belongs as a separate style class, but I can live with "expanded".
For me, using a pseudoclass seems like the natural "right" choice in this situation (for the reasons mentioned in comment 21, although I hadn't really thought about it before). Jasper, if this isn't an ideal candidate for a pseudoclass in your mind, under what circumstances do you think pseudoclasses are appropriate?
To me, it's about UI state, not semantic, internal state. "logged-in" is semantic state. "hover", "active", etc. are not. I guess "expanded" can be nice. "overview" is skirting both sides, and the votes by the code in the Shell show that.
i kind of see what you're saying, and there's definite logic to it, but the html anchor pseudoclass "visited" is very analagous to "logged-in" so I think using a pseudoclass is actually okay for "logged-in". So to me, classes are about entities and pseudoclasses are about state modifiers for those entities. Normally the state is interface related, but doesn't have to be. Honestly, at the end of the day, though, it doesn't matter much. Both could work equally well. Given we don't have a super precise definition, I say we should probably just leave it up to Florian to pick his favorite.
Sure thing. Other than this, these commits look fine. Treat them as ACN.
Attachment 220074 [details] pushed as a3f4bca - loginDialog: Add an :expanded pseudo class to the user list Attachment 220075 [details] pushed as 96c9f80 - loginDialog: Indicate whether users are logged in I don't care too much either way, but pushing the existing patches allows me to not rebuild a gnome-shell package with an updated patch before falling into post-guadec coma ...