Created attachment 236524 [details] [review]
loginDialog: get rid of title label

the login screen currently says "Sign In" at the top of it.

The latest mock ups don't have that.

This commit drops it.

Created attachment 236525 [details] [review]
loginDialog: drop animations

The latest mockups don't animate between states by
resizing actors. Instead, crossfades are employed.

This commit strips out many of the existing animations
as a first step toward implementing the new ones.

Created attachment 236526 [details] [review]
loginDialog: fade out before starting session

Right now we very abruptly kill the login screen
and start the users session without any transition
out.

This commit introduces a fade out of the dialog and
panels.

Created attachment 236527 [details] [review]
unlockDialog: move user widget into separate file

The user widget is the username and avatar shown on
the unlock dialog.

The login dialog has something very similar.

This commit separates the user widget out to its own
file, so we can use it from the login dialog in a
later commit.

Created attachment 236528 [details] [review]
loginDialog: use user widget when doing verification

Right now, when a user item is clicked we remove all other users from
the list and position the item in the appropriate place on screen.

Ultimately, we're going to want to crossfade from the fully populated
list to the user prompt.  Since we're going to need to show the user
avatar in two different positions we can't simply move it.

This commit leaves the user item for the user list, and instead shows
a UserWidget actor during user verification, in the same way the
unlock dialog shows a UserWidget actor during reauthentication.

Created attachment 236529 [details] [review]
loginDialog: move prompt hiding code to hidePrompt

The sessionList and the prompt hint are all really
part of the prompt, so we should have the code that
hides those things in hidePrompt instead of in
showUserList.

This commit does that.

Created attachment 236530 [details] [review]
loginDialog: put "Not Listed?" button and user list in separate container

The user list and the "Not Listed?" button get shown and hidden at the
same time, so we can simplify the code by putting them in a new
subcontainer.

This commit creates a userSelectionBox container that both actors get
put in, and changes all the code that shows and hides these actors to
show and hide userSelectionBox instead.

Created attachment 236531 [details] [review]
loginDialog: add cross fade animation between states

This commit adds a crossfade between the user selection state
and the user verification state.

Review of attachment 236524 [details] [review]:

OK.

Review of attachment 236525 [details] [review]:

Nice.

::: js/gdm/loginDialog.js
@@ +1067,2 @@
     _onUserListActivated: function(activatedItem) {
         let userName;

Unused?

Review of attachment 236526 [details] [review]:

::: js/gdm/loginDialog.js
@@ +917,3 @@
+                           transition: 'easeOutQuad',
+                           onUpdate: function() {
+                               Main.layoutManager.panelBox.opacity = this.dialogLayout.opacity;

I wonder if you should just fade in a MetaBackgroundActor on top. Seems a little iffy that this is all we're ever going to need.

Like, sometimes there's a notification that comes up from the bottom about some status indication. That might hang on screen as well.

@@ +922,3 @@
+                           onComplete: function() {
+                               Mainloop.idle_add(Lang.bind(this, function() {
+                                   this._greeter.call_start_session_when_ready_sync(serviceName, true, null);

return false;

Review of attachment 236527 [details] [review]:

You forgot to add the file???

Review of attachment 236528 [details] [review]:

It would be nice if we could use the entire UnlockDialog, but that's a cleanup for another day...

::: js/gdm/loginDialog.js
@@ +1089,3 @@
     },
 
+    _setPromptUserFromItem: function(item) {

This is so minor I would just inline it.

@@ -1084,2 @@
         this._userList.actor.reactive = false;
-        this._userList.hideItemsExcept(activatedItem);

You can remove this method now...

Review of attachment 236529 [details] [review]:

OK.

Review of attachment 236530 [details] [review]:

Weren't there keyboard focus issues when we tried this before?

Review of attachment 236531 [details] [review]:

It looks like from the motion mockups that they don't want a cross-fade but a fade out, then fade in sort of thing.

::: js/gdm/util.js
@@ +78,3 @@
+    // and position, but leave a non-reactive clone on-screen,
+    // so from the user's point of view it smoothly fades away
+    // and reveals its sibling.

ShellStack would be a more optimal way of doing this sort of thing.

(In reply to comment #11)
> I wonder if you should just fade in a MetaBackgroundActor on top. Seems a
> little iffy that this is all we're ever going to need.
We don't show a MetaBackgroundActor at the login screen, we show a noise texture. We could fade it over, though.

(In reply to comment #15)
> Review of attachment 236530 [details] [review]:
> 
> Weren't there keyboard focus issues when we tried this before?
I have some vague notion of a problem that Florian fixed.  At any rate, there aren't any focus issues now that i can see.

(In reply to comment #16)
> Review of attachment 236531 [details] [review]:
> 
> It looks like from the motion mockups that they don't want a cross-fade but a
> fade out, then fade in sort of thing.
Which ones?  This one:

http://jimmac.fedorapeople.org/gnome3/login-gray-fadein.webm

and this slightly older one:

http://jimmac.fedorapeople.org/gnome3/login.webm

both do cross fades.
 
> ::: js/gdm/util.js
> @@ +78,3 @@
> +    // and position, but leave a non-reactive clone on-screen,
> +    // so from the user's point of view it smoothly fades away
> +    // and reveals its sibling.
> 
> ShellStack would be a more optimal way of doing this sort of thing.
Can you give me some more details on what you have in mind?

Created attachment 236708 [details] [review]
loginDialog: fade out before starting session

Right now we very abruptly kill the login screen
and start the users session without any transition
out.

This commit introduces a fade out of the dialog and
panels.

Created attachment 236709 [details] [review]
unlockDialog: move user widget into separate file

The user widget is the username and avatar shown on
the unlock dialog.

The login dialog has something very similar.

This commit separates the user widget out to its own
file, so we can use it from the login dialog in a
later commit.

Created attachment 236710 [details] [review]
loginDialog: use user widget when doing verification

Right now, when a user item is clicked we remove all other users from
the list and position the item in the appropriate place on screen.

Ultimately, we're going to want to crossfade from the fully populated
list to the user prompt.  Since we're going to need to show the user
avatar in two different positions we can't simply move it.

This commit leaves the user item for the user list, and instead shows
a UserWidget actor during user verification, in the same way the
unlock dialog shows a UserWidget actor during reauthentication.

Created attachment 236711 [details] [review]
loginDialog: move prompt hiding code to hidePrompt

The sessionList and the prompt hint are all really
part of the prompt, so we should have the code that
hides those things in hidePrompt instead of in
showUserList.

This commit does that.

Created attachment 236712 [details] [review]
loginDialog: put "Not Listed?" button and user list in separate container

The user list and the "Not Listed?" button get shown and hidden at the
same time, so we can simplify the code by putting them in a new
subcontainer.

This commit creates a userSelectionBox container that both actors get
put in, and changes all the code that shows and hides these actors to
show and hide userSelectionBox instead.

Created attachment 236713 [details] [review]
loginDialog: add cross fade animation between states

This commit adds a crossfade between the user selection state
and the user verification state.

(In reply to comment #17)
> Can you give me some more details on what you have in mind?

Well, the page crossfades were removed, but you have a ShellStack with multiple pages (actually it could just be a ClutterBinLayout nowadays) and you tweak the opacity of each one.

Oh, i see what you're saying now.

The problem with doing that is the buttons will have the wrong placement during the cross fade.

does the line it probably makes sense to stop using modal dialog for this, but that's a cleanup for another time.

(In reply to comment #26)
> The problem with doing that is the buttons will have the wrong placement during
> the cross fade.

That doesn't make any sense to me. Why?

Review of attachment 236708 [details] [review]:

::: js/gdm/loginDialog.js
@@ +924,3 @@
+
+                               for (let i = 0; i < children.length; i++) {
+                                   if (children[i] != Main.layoutManager.screenShieldGroup)

Not the biggest fan of this here. We should be able to do better than this.

Review of attachment 236709 [details] [review]:

::: js/ui/unlockDialog.js
@@ -66,3 @@
-                                        vertical: false });
-
-        this._avatar = new UserMenu.UserAvatarWidget(user);

Perhaps you should move UserAvatarWidget over as well?

::: js/ui/userWidget.js
@@ +1,1 @@
+

Remove whitespace at the top.

Review of attachment 236710 [details] [review]:

This is really nice.

Review of attachment 236712 [details] [review]:

I really like this.

Review of attachment 236713 [details] [review]:

This is OK on the condition that we remove ModalDialog and move to a ClutterBinLayout sometime before release.

pushing now.  if we need to do some follow up work we can

Attachment 236524 [details] pushed as f8446f1 - loginDialog: get rid of title label
Attachment 236525 [details] pushed as adf95fb - loginDialog: drop animations
Attachment 236708 [details] pushed as 244121d - loginDialog: fade out before starting session
Attachment 236709 [details] pushed as a3d3d81 - unlockDialog: move user widget into separate file
Attachment 236710 [details] pushed as d124ca3 - loginDialog: use user widget when doing verification
Attachment 236711 [details] pushed as 963e808 - loginDialog: move prompt hiding code to hidePrompt
Attachment 236712 [details] pushed as 96001d8 - loginDialog: put "Not Listed?" button and user list in separate container
Attachment 236713 [details] pushed as 5fa9581 - loginDialog: add cross fade animation between states

(reopening since there's still more we can do here)

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

(In reply to comment #35)
> (reopening since there's still more we can do here)

Do you have a list of what else should be done?

nope.