gnome-shell-3.2.0-2.fc16.x86_64

do you mean the avatars themselves, or just the borders around the avatars?

So I just tried making the border of the active item turn bright white like the text and it looks really strange.

I'm worried desaturating the user's face might look strange too.

I guess we need designer feedback here to know the right way to proceed.

The case I was using is that my avatar has a lot of bright white in it already. Maybe it's specific to that avatar that it looks out of place.

oh so desaturating wouldn't even be enough for you, you'd need it actually dimmed (since the color white is already fully desaturated)

Bug 658185 is worth noting as it seems a viable solution to distinguishing
running sessions from people not logged in. There are however drawbacks to it,
as people can have desaturated avatars and the default "no avatar" image is
greyscale too.

I lean towards not desaturating nor dimming the avatar as it makes it harder to identify it among a set and rather bring more focus/attention to the users with a running session (prelight+underline).

generalizing and retitling

*** Bug 662713 has been marked as a duplicate of this bug. ***

note that the glow that is supposed to appear under the name is not glowing but is a solid 2-3px horizontal rule that looks a bit odd, especially if you only have one suer ...

Created attachment 219252 [details] [review]
loginDialog: Update user list style

Rather than changing the text color to indicate hover and an underline
to mark the focused item, use the same semi-transparent white background
as in the overview.

Created attachment 219253 [details] [review]
gdm: Adjust timed-login indicator

Until the recent style changes, the same element was used to indicate
both item focus and progress for timed logins. As focus is now indicated
by the item's background style, rename the indicator from focusBin to
timedLoginIndicator and make some minor adjustments to better fit the
new style:
  - move it next to the icon below the text
  - give it a white color and a shadow
  - update animation to grow from the left instead of the center

Created attachment 219254 [details]
Screenshot with patches applied

I'm not aware of any mockups for timed logins, so here's a screenshot with what I came up with ...

Can't test this currently, since timed login is not working in gdm 3.5.4.x

see bug 680348 for the timed login saga.  I've pushed a stopgap to get it functioning again.

Created attachment 219565 [details] [review]
loginDialog: Update user list style

Rebase to master, fix style getting out-of-sync when cancelling password entry

Created attachment 219566 [details] [review]
gdm: Adjust timed-login indicator

Rebase to master

(In reply to comment #13)
> Can't test this currently, since timed login is not working in gdm 3.5.4.x

There's not much to test to be honest - there shouldn't be a functional difference, just a style update (which is why I attached a screenshot ;-)

Right - but testing it revealed timed login broken, so it was good for something...

Review of attachment 219566 [details] [review]:

This looks pretty good (though for unrelated reasons the styling in master is halfway between 3.4 login screen and 3.6 design).

One thing I noticed: Since we used to use the focus indicator for both focus and timed login, if the user was focused you couldn't see the results of timed login, now you can which is definitely a good thing, but it does expose a bug.  We don't stop the indicator animation tween in resetTimedLogin, so if the user arrows down and arrows back up they'll see a timed login indicator that's wrong.

Created attachment 219628 [details] [review]
gdm: Adjust timed-login indicator

(In reply to comment #19)
> One thing I noticed: [...] it does expose a bug. 
> We don't stop the indicator animation tween in resetTimedLogin, so if the user
> arrows down and arrows back up they'll see a timed login indicator that's
> wrong.

Oh - I did encounter this during testing, but assumed it was the actual timeout. Should be fixed now ...

Created attachment 219645 [details] [review]
gdm: Adjust timed-login indicator

I ran the changes by Jakub and he suggested a slightly different style ...

Created attachment 219646 [details] [review]
loginDialog: Tweak automatic scroll animation

The current animation time of two seconds may result in some
confusion, as the reason of the behavior only becomes apprent
when the auto-activating item becomes visible; make the animation
a lot faster and ease it out a bit.

(Only marginally related to this bug, but it doesn't warrant its own)

Review of attachment 219645 [details] [review]:

it's a little faint now, but I guess it doesn't need to be super in-your-face anyway.

Review of attachment 219646 [details] [review]:

Probably fine, though I wonder if easing out is right for the other case we call scrollToItem (nudging the selected item front-and-center). Seems to look okay, so I guess it's good.

Comment on attachment 219254 [details]
Screenshot with patches applied

Screenshot is obsolete, current patch has the jimmac-seal-of-approval, so not attaching another one

Thanks for the review - could you take a look at the first patch as well?

Review of attachment 219565 [details] [review]:

Nothing obviously wrong that I could see (I actually thought I already changed the status of this, weird)

::: js/gdm/loginDialog.js
@@ +311,2 @@
             item.actor.can_focus = false;
+            item.actor.remove_style_pseudo_class('focus');

you could make syncStyleClasses check for can_focus and then call syncStyleClasses here to make the code more symetrical with the hunk below.

Hard to say which way is better, it's probably subjective.

Attachment 219565 [details] pushed as bfea41b - loginDialog: Update user list style
Attachment 219645 [details] pushed as 6a615f3 - gdm: Adjust timed-login indicator
Attachment 219646 [details] pushed as 5c7992b - loginDialog: Tweak automatic scroll animation

Pushed with some tweaks discussed on IRC ...