I'd rather make the switch connect/disconnect from IM. "Do Not disturb" is kind of weak when you really don't want to use IM or be contacted by people. IMHO the solution from bug 652718 is much better: show a notification explaining notifications won't be shown as long as status is set to Busy.

That said, I think the mockup looks nice, but once you change "Do Not Disturb" to "Online"... ;-)

Originally we used the radio because we had a Hidden/Invisible state. We removed that because it wasn't really thought through. I agree that using a radio for two state things like this isn't very nice. It makes a lot of sense here to not use mode names and just communicate the effect. So yeah.

The most important issue of all with busy or do not disturb is to make it clear when it is on. So that it isn't set and forgotten. Wonder if it should expire... the reason I mention it here is that if it does expire then a switch may not be right either. Might be more like a button.

(In reply to comment #1)
> That said, I think the mockup looks nice, but once you change "Do Not Disturb"
> to "Online"... ;-)

Except that you _are_ "online" in the sense that you can view web pages etc. "Do not disturb" sounds fine for me. The more we can keep the technical jargon away the better IMHO.

Bug 652718 is orthogonal to this I think and can/should be done regardless.

I really like the design Allan!

I'm not particularly opposed to the label "Do Not Disturb" by itself, but more by the consequences of the switch: does that mean it sets the online/offline IM status? Makes you visible/invisible? Or just changes the Busy status? IMO the latter wouldn't be enough, that's merely what I meant (or tried).

Isn't a combobox in a popup menu kind of eeky?

Created attachment 190211 [details]
Redesigned user menu mockup

(In reply to comment #5)
> Isn't a combobox in a popup menu kind of eeky?

I don't see why; but maybe I haven't thought it through properly. :)

(In reply to comment #4)
> I'm not particularly opposed to the label "Do Not Disturb" by itself, but more
> by the consequences of the switch: does that mean it sets the online/offline IM
> status? Makes you visible/invisible? Or just changes the Busy status? IMO the
> latter wouldn't be enough, that's merely what I meant (or tried).

If status is available activating dnd would switch it to busy. If status is set to unavailable dnd would leave it unaffected. (This second case is potentially problematic since the top bar would not contain an indication that dnd is active. We might need to find a solution to that.)

Available would remain an option in both cases. Switching to it deactivates dnd.

See the attached mockup for more detail.

Created attachment 190732 [details]
Mockup - tweaked avatar presentation

Word up. I've tweaked the design slightly - the user picture needs to be bigger (64px by 64px).

I think we should merge this bug with bug #652716.

We discussed this at the hackfest and solved this problem in a different way: https://live.gnome.org/Hackfests/IMContacts%20Social2011/Tasks/ShellDesignPresence

I like having a simple switch changing IM presence from Busy/Available to Hidden actually.

I'd really like if we could reach a consensus on this topic, especially if we
go for the solution proposed in bug #652716 or not.

(In reply to comment #0)
> 1. Busy status appears as if it purely relates to online/IM status. This means
> that the wider notification inhibit behaviour of this option is not apparent.

This will be fixed with bug #652718 and so won't be an issue any more.

> 2. User testing has demonstrated that people are often very concerned about how
> they appear to others via the web. The user menu allows a user to affect this
> presentation, but it does not follow the standard contact layout. This means
> that it doesn't give the user a clear idea of how they appear to others.
> 
> I've attached a mockup of how we could improve this situation. The new design
> does a few things:
> 
>  * Adds a separate (and obviously different) control for the notification
> inhibit option (labelled 'Do not disturb'). This makes it clear that something
> else will happen other than changing the online status (though it does do
> that).

I don't think that's longer needed once bug #652718 will be fixed.

>  * Presents the user in the same way that contacts are displayed in the
> contacts app (and will be displayed elsewhere in the future). This includes an
> avatar image.

Agreed, having the avatar would be good.


I'm not convince by this combobox tbh. Furthemore the semantic of "available"
isn't that clear I think. I'd prefer the solution presented in bug #652837,
"Visible on chat" being pretty self explanatory.

What about http://bugzilla-attachments.gnome.org/attachment.cgi?id=190859 +
the avatar as in your mockup?

(In reply to comment #9)
> I'd really like if we could reach a consensus on this topic, especially if we
> go for the solution proposed in bug #652716 or not.

Jon seemed to like both this proposal and the one in bug 652718. I don't really like the approach in bug #652716 - I've added some comments there.

> (In reply to comment #0)
> > 1. Busy status appears as if it purely relates to online/IM status. This means
> > that the wider notification inhibit behaviour of this option is not apparent.
> 
> This will be fixed with bug #652718 and so won't be an issue any more.

An explanatory notification will help, but it doesn't change the confusing nature of this piece of UI. It still needs fixing.
 
...

> I'm not convince by this combobox tbh.

Why?

> Furthemore the semantic of "available"
> isn't that clear I think. I'd prefer the solution presented in bug #652837,
> "Visible on chat" being pretty self explanatory.

I'm pretty surprised that you're taking issue with this, since 'Available' is the string that is used by Empathy. We can bike shed it if you'd really like though. ;)

'Available' is pretty common for IM, so I would imagine that the meaning is clear (particularly in combination with the user status icon). It's used by Google Talk, for instance (and I'm pretty sure Google will have done user testing on it).

(In reply to comment #10)
> > (In reply to comment #0)
> > > 1. Busy status appears as if it purely relates to online/IM status. This means
> > > that the wider notification inhibit behaviour of this option is not apparent.
> > 
> > This will be fixed with bug #652718 and so won't be an issue any more.
> 
> An explanatory notification will help, but it doesn't change the confusing
> nature of this piece of UI. It still needs fixing.

I think the big question here is "Do we want to keep this idea of 'desktop
presence' or not". If we do, we need to keep the Available/Busy switch. If not
a "Don't annoy me" switch is enough.

I was under the impression that the desktop presence was something important
for GNOME 3, controlling your whole desktop experience: notifications, IM,
probably more later, etc.

Bastien could probably tell use more about it, he was the main pro desktop
presence during our session at the hackfest. Personally I use to think it was
bullshit but Bastien almost convinced me it was good; so I'm open to
discussion. :)


> > I'm not convince by this combobox tbh.

> > Furthemore the semantic of "available"
> > isn't that clear I think. I'd prefer the solution presented in bug #652837,
> > "Visible on chat" being pretty self explanatory.
> 
> I'm pretty surprised that you're taking issue with this, since 'Available' is
> the string that is used by Empathy. We can bike shed it if you'd really like
> though. ;)
> 
> 'Available' is pretty common for IM, so I would imagine that the meaning is
> clear (particularly in combination with the user status icon). It's used by
> Google Talk, for instance (and I'm pretty sure Google will have done user
> testing on it).

Right, but Empathy is an IM client while the Shell is not.

From my experience, if we offer an IM-client-like presence chooser in the
Shell, people will exepect the full set of features offered by IM presence
chooser:
- available, busy, away, invisible, offline.
- Set and re-use custom presence messages
- Pretend they are away even if they are not (session isn't idling)
- Per account presence (we don't support it in Empathy)

I assumed that our goal was to keep the user menu as simple as possible, which
is why I liked the "Visible on Chat" approach: a simple switch to control your
IM visiblity; people wanting the full control having just to Empathy.

(In reply to comment #11)
> I think the big question here is "Do we want to keep this idea of 'desktop
> presence' or not". If we do, we need to keep the Available/Busy switch. If not
> a "Don't annoy me" switch is enough.

I don't agree that Allan's proposal is incompatible with the concept of 'desktop presence'.


> I was under the impression that the desktop presence was something important
> for GNOME 3, controlling your whole desktop experience: notifications, IM,
> probably more later, etc.

Yes, that's correct. But it has turned out that people simply don't understand that the current controls set 'desktop presence' rather than 'Chat presence' - and honestly, it's difficult to blame them. It quacks like IM presence, it waddles like IM presence, it must be IM presence ...

So I'd describe the proposal as untangling desktop presence and IM presence. What looks like IM presence actually controls IM presence, and desktop presence is moved to a simple switch. Given that desktop presence only has three states - "Available", "Busy" and "Idle", with the latter only being set automatically - a simple switch is enough.

(In reply to comment #0)
> I've attached a mockup of how we could improve this situation. The new design
> does a few things

It also leaves a few things unspecified ;-)

I've started an implementation, and would like some feedback if the behavior makes sense:

 - if "do not disturb" is activated, the IM presence is set to busy if it is
   "more present" (e.g. available - I think away/extended away are less present
   than busy, but feel free to correct me

 - if "do not disturb" is deactivated, the IM presence is set to the state it
   had when activating "do not disturb" (kind of fails when logging out with
   "do not disturb" checked, but I don't think it's a serious problem)

 - the IM presence always represents the (most available) presence, including
   states which are excluded from the shell menu (hidden, away)

 - if the IM presence changes to "Available", "do not disturb" is deactivated;
   all other presence changes (including a change to "busy") have no effect on
   desktop presence

(not implemented yet)
 - the IM status dropdown contains at most three entries: "Available" and
   "Unavailable" are always shown, if the current presence is different
   from those, it is shown in a third entry


Comments?

Also, the mockups weasel out on the question of how "popup popups" should look like (e.g. should they behave like GtkComboBox, or slide out of the menu item, or ...)

(In reply to comment #13)
>  - if "do not disturb" is activated, the IM presence is set to busy if it is
>    "more present" (e.g. available - I think away/extended away are less present
>    than busy, but feel free to correct me

I think we should:
- change for: available
- not change for: offline, hidden, away, extend away, busy

You should probably always keep the existing status message, if any.

>  - if "do not disturb" is deactivated, the IM presence is set to the state it
>    had when activating "do not disturb" (kind of fails when logging out with
>    "do not disturb" checked, but I don't think it's a serious problem)

I think we should change it back only if hasn't been change while we were
away. Consider this case:
- I'm available
- I turn on DnD -> presence is busy
- I open empathy and set myself as hidden  -> presence is hidden
- I turn off DnD -> presence is available

In that case we should stay hidden. We should probably special case going away
because of idling.

>  - the IM presence always represents the (most available) presence, including
>    states which are excluded from the shell menu (hidden, away)

Agreed. Use tp_account_manager_get_most_available_presence().

>  - if the IM presence changes to "Available", "do not disturb" is deactivated;
>    all other presence changes (including a change to "busy") have no effect on
>    desktop presence

Not sure about this one. As I see it, IM presence is a kind of subset of the
desktop presence. Changing the superset because of a subset change is a bit
weird.

> (not implemented yet)
>  - the IM status dropdown contains at most three entries: "Available" and
>    "Unavailable" are always shown, if the current presence is different
>    from those, it is shown in a third entry

What's Unavailable? Offline? Hidden?

(In reply to comment #14)
> I think we should:
> - change for: available
> - not change for: offline, hidden, away, extend away, busy

Good, that's what I'm doing right now.


> You should probably always keep the existing status message, if any.

Mmmh, currently I don't do anything with the message parameter, just pass it through. Should I instead
 - store it with the previous state when changing to busy and setting a new
   message of null or "I'm busy"
 - restore it with the previous state when changing back from busy?

BTW, I don't think we want to expose custom status messages anywhere in the shell, so all we should do is ensure that we don't mess up the messages in Empathy.


> I think we should change it back only if hasn't been change while we were
> away.

Good point.


> In that case we should stay hidden. We should probably special case going away
> because of idling.

Is that was extended-away is used for?



> >  - the IM presence always represents the (most available) presence, including
> >    states which are excluded from the shell menu (hidden, away)
> 
> Agreed. Use tp_account_manager_get_most_available_presence().

Well, TpAccountManager::most-available-presence-changed, but yeah ...



> Not sure about this one. As I see it, IM presence is a kind of subset of the
> desktop presence. Changing the superset because of a subset change is a bit
> weird.

It's in the mockups. Feel free to fight it out with Allan ;-)


> What's Unavailable? Offline? Hidden?

Offline. "Unavailable" is the term used in the mockup.

(In reply to comment #15)
> > You should probably always keep the existing status message, if any.
> 
> Mmmh, currently I don't do anything with the message parameter, just pass it
> through. Should I instead
>  - store it with the previous state when changing to busy and setting a new
>    message of null or "I'm busy"
>  - restore it with the previous state when changing back from busy?

Look at the current _setIMStatus() in statusMenu.js: we get the existing
status message and re-use it when setting the new presence.

> BTW, I don't think we want to expose custom status messages anywhere in the
> shell, so all we should do is ensure that we don't mess up the messages in
> Empathy.

Agreed.

> > In that case we should stay hidden. We should probably special case going away
> > because of idling.
> 
> Is that was extended-away is used for?

extended-away should be set when we are away since $minutes. one hour maybe?

> > Not sure about this one. As I see it, IM presence is a kind of subset of the
> > desktop presence. Changing the superset because of a subset change is a bit
> > weird.
> 
> It's in the mockups. Feel free to fight it out with Allan ;-)

I don't mind that much tbh. :)

> > What's Unavailable? Offline? Hidden?
> 
> Offline. "Unavailable" is the term used in the mockup.

I think we should have Hidden/Invisible as well, loads of people are using it.
Or maybe we could just have this one instead of offline? During the hackfest
most people agreed that Hidden was a good choice (if we only display a subset
of statuses if we fallback to offline for accounts not supporting the hidden
status).

(In reply to comment #16)
> > Mmmh, currently I don't do anything with the message parameter, just pass it
> > through. [...]
> 
> Look at the current _setIMStatus() in statusMenu.js: we get the existing
> status message and re-use it when setting the new presence.

Good, less work then. (As mentioned, I'm already doing that)


> > Is that was extended-away is used for?
> 
> extended-away should be set when we are away since $minutes. one hour maybe?

Uhm, is shell really a good place to set that? Anyway, for now I'm only interested in the icon I should show in that case ;-)


> > > What's Unavailable? Offline? Hidden?
> > 
> > Offline. "Unavailable" is the term used in the mockup.
> 
> I think we should have Hidden/Invisible as well, loads of people are using it.

I disagree. I don't think we want to account for anything but "casual chat use" in the shell itself, for more advanced uses there's a dedicated app (read: Empathy). Also, neither hidden nor invisible look very useful without a contact list - to my understanding, those states exist so people can appear disconnected while still seeing who's online.

(In reply to comment #17)
> > > > What's Unavailable? Offline? Hidden?
> > > 
> > > Offline. "Unavailable" is the term used in the mockup.
> > 
> > I think we should have Hidden/Invisible as well, loads of people are using it.
> 
> I disagree. I don't think we want to account for anything but "casual chat use"
> in the shell itself, for more advanced uses there's a dedicated app (read:
> Empathy). Also, neither hidden nor invisible look very useful without a contact
> list - to my understanding, those states exist so people can appear
> disconnected while still seeing who's online.

I agree, the Shell should focus on "casual chat use", which is why we should
only provide a subset of status and that's also why I think we should provide
hidden instead of offline. See hidden as a super version of offline: we appear
as offline but can still see who is connected (assuming MC fallback to offline
if the account doesn't support hidden). Why not use this "enhanced" version
then? That's something I really liked about the "Visible on Chat" approach
actually.

It's true that hidden is not that useful without a contact list, but the Shell
should soon be able to display contacts in the search overview; Mortem is
working on it as part of his SoC project.

(In reply to comment #13)
> (In reply to comment #0)

Hi Florian! Sorry for the slow response. I've been busy with other things.

> I've started an implementation, and would like some feedback if the behavior
> makes sense:
> 
>  - if "do not disturb" is activated, the IM presence is set to busy if it is
>    "more present" (e.g. available - I think away/extended away are less present
>    than busy, but feel free to correct me
>
>  - if "do not disturb" is deactivated, the IM presence is set to the state it
>    had when activating "do not disturb" (kind of fails when logging out with
>    "do not disturb" checked, but I don't think it's a serious problem)
> 
>  - the IM presence always represents the (most available) presence, including
>    states which are excluded from the shell menu (hidden, away)

Sounds great to me. :)
 
>  - if the IM presence changes to "Available", "do not disturb" is deactivated;
>    all other presence changes (including a change to "busy") have no effect on
>    desktop presence

I would count a status change from unavailable to busy as a raising of your availability and would therefore deactivate do not disturb.

I would also suggest showing a notification message when do not disturb is deactivated due to an IM status change.

> (not implemented yet)
>  - the IM status dropdown contains at most three entries: "Available" and
>    "Unavailable" are always shown, if the current presence is different
>    from those, it is shown in a third entry

Ace.

> Also, the mockups weasel out on the question of how "popup popups" should look
> like (e.g. should they behave like GtkComboBox, or slide out of the menu item,
> or ...)

The idea was that it should be a combobox and, as such, behave like GtkComboBox.

(In reply to comment #16)
> I think we should have Hidden/Invisible as well, 

Guillaume - want to file that as a separate bug? :)

> (In reply to comment #16)
> > I think we should have Hidden/Invisible as well, 
> 
> Guillaume - want to file that as a separate bug? :)

Why? That's really part of the presence picker redesign. We have 3 options:
a) Don't display hidden
b) Display hidden
c) Replace offline/unavailable by hidden

I don't like a) and most people at the hackfest seem to like c) but I'm ready to be convinced otherwise.

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

I've reported this as a separate bug, but apparently it was marked as a duplicate of this bug, hence I am copying description here:

Remove user's name, just leave status icon. Or allow to hide user's full name.

A few reasons for that:
 * Users are not stupid, they know their names.
 * If user has a really long name, ie "Miguel Fernandez Gonzales", it takes too
much space.
 * Name is repeated in the menu...

(In reply to comment #22)
> Remove user's name, just leave status icon. Or allow to hide user's full name.

The menu header should communicate the contents of the menu. An IM status icon does not do this. What has IM status got to do with logging out or system settings, for example?

(In reply to comment #23)
> The menu header should communicate the contents of the menu. An IM status icon
> does not do this. What has IM status got to do with logging out or system
> settings, for example?

What does my name have to do with locking the screen?

see also bug 651299

(In reply to comment #24)
> (In reply to comment #23)
> > The menu header should communicate the contents of the menu. An IM status icon
> > does not do this. What has IM status got to do with logging out or system
> > settings, for example?
> 
> What does my name have to do with locking the screen?

The name communicates that the menu has options associated with the user's presence on the system. Locking the screen is a way of interupting that presence.

Having a menu labelled with the user name that contains high-level system controls is something of a convention nowadays, too (think Google, Twitter, etc).

> Having a menu labelled with the user name that contains high-level system
> controls is something of a convention nowadays, too (think Google, Twitter,
> etc).

Google shows a "Gear Icon" for settings. Additionally, I think the context of a web application is different than a web application. Logging off a web application is something you do all the time and is more tied to your presence. Not so for the desktop.

Created attachment 192699 [details]
Redesigned user menu mockup

I've tested the user-status-update branch - very cool indeed. :)

A few minor suggestions:

 * Make clicking the profile picture open the user's account

 * Push the user name and IM status closer to the profile picture

 * Reduce the padding between the IM status icon and the string

 * Increase the size of the user's name

Mockup attached with more details.

Since the profile will now open the user account, I'm proposing that we change the 'My Account' menu entry to 'Online Accounts', but maybe that requires a separate discussion...

I was finally able to try the IM switcher part of this the other day. It did feel a *bit* weird. I'm honestly not sure how big a problem that is though: many people might not even notice, and I got used to it quickly enough.

It might be worth checking around to see how people feel about this popup, though I can't see a way of doing what we want to by any other means (you could separate IM status from do not disturb fairly easily, but the menu would be too big with the avatar as well).

If we stick with the combobox approach, there are a few things we could do to reduce its visual impact: 

 * Making sure that it lines up with the content underneath

 * Making it a bit more transparent

 * Reducing the size, particularly by decreasing the radius of the corners

I've pushed an updated branch, which should address the comments. However, I have some doubts about some of those ...


(In reply to comment #27)
> A few minor suggestions:
>  * Make clicking the profile picture open the user's account

Clicking is fine, but it's rather sucky for keynav - the current code ignores the picture completely, implementing it is supposedly hacky (as picture and combo box are placed horizontally, but left/right is already taken for switching between menus, so we'd have to use up/down overwriting the automatic focus navigation).



>  * Push the user name and IM status closer to the profile picture
> 
>  * Reduce the padding between the IM status icon and the string

Done and done.


>  * Increase the size of the user's name

Done, but there's a "tiny" problem - at least for now, we don't have ellipsization in the menus, so long names will result in a far too wide menu.


> Since the profile will now open the user account, I'm proposing that we change
> the 'My Account' menu entry to 'Online Accounts', but maybe that requires a
> separate discussion...

To be honest, I'm not sure about that change - it's a bit odd that the menu entry which (ideally) allows to configure chat accounts is separated from the im functions, while the panel for the 'system account' which doesn't really have anything to do with chat is not. (But then, user name and picture are actually taken from the latter, so maybe it's not that much of a problem after all).

Of course there's still the keynav problem to figure out.



(In reply to comment #28)
> I was finally able to try the IM switcher part of this the other day. It did
> feel a *bit* weird. I'm honestly not sure how big a problem that is though:
> many people might not even notice, and I got used to it quickly enough.

Ha! You should try a combo box *inside* the combo box menu (and yes, I've actually tested that ...)


> (you could separate IM status from do not disturb fairly easily, 
> but the menu would be too big with the avatar as well).

Well, we could sacrifice the user name there, and show the two statuses similar to the old code. If the status changes to something not in the menu, we replace "Available" with the current status and allow getting back to it with the alt trick (Jakub will love that one, but not sure - maybe I've spend to much time getting used to the popup popup)


> If we stick with the combobox approach, there are a few things we could do to
> reduce its visual impact: 
> 
>  * Making sure that it lines up with the content underneath

Mostly done.


>  * Making it a bit more transparent
> 
>  * Reducing the size, particularly by decreasing the radius of the corners

Done and done.

(In reply to comment #29)
> I've pushed an updated branch, which should address the comments.

The updates are awesome. It's looking really nice.

> (In reply to comment #27)
> > A few minor suggestions:
> >  * Make clicking the profile picture open the user's account
> 
> Clicking is fine, but it's rather sucky for keynav - the current code ignores
> the picture completely, implementing it is supposedly hacky (as picture and
> combo box are placed horizontally, but left/right is already taken for
> switching between menus, so we'd have to use up/down overwriting the automatic
> focus navigation).

That's not too bad, I don't think. You can get to this through System Settings anyway. 
...

> >  * Increase the size of the user's name
> 
> Done, but there's a "tiny" problem - at least for now, we don't have
> ellipsization in the menus, so long names will result in a far too wide menu.

That could suck. I don't think people want to see their names ellipsised though. Scaling the name down and wrapping it over two lines seems like the best approach.

> > Since the profile will now open the user account, I'm proposing that we change
> > the 'My Account' menu entry to 'Online Accounts', but maybe that requires a
> > separate discussion...
> 
> To be honest, I'm not sure about that change - it's a bit odd that the menu
> entry which (ideally) allows to configure chat accounts is separated from the
> im functions, while the panel for the 'system account' which doesn't really
> have anything to do with chat is not. (But then, user name and picture are
> actually taken from the latter, so maybe it's not that much of a problem after
> all).

I thought messaging and voip is going to be dropped from systems settings - online accounts should be the primary way of setting up messaging. (Maybe I'm wrong though.)

...

(In reply to comment #30)
> That's not too bad, I don't think. You can get to this through System Settings
> anyway. 
> ...

OK.


> That could suck. I don't think people want to see their names ellipsised
> though. Scaling the name down and wrapping it over two lines seems like the
> best approach.

Mmmh, OK. It's definitively the most tricky approach ;-)


> I thought messaging and voip is going to be dropped from systems settings -
> online accounts should be the primary way of setting up messaging. (Maybe I'm
> wrong though.)
> 
> ...

No no no, you are right on this one, but that's not what I meant. I was referring to the position of "online accounts" (i.e. below "do not disturb", separate from the im status) vs. "user accounts" (now accessible through the user picture next to the im status, previously known as "My Account").

Some notes from our BoF during the desktop summit:

- Should the presence picker display custom message status (like "At the pub")?

- What's the exact semantic of the status displayed? The current status or the desired status? For example, if network is offline in NetworkManager, we can't obvioulsy be connected to IM. Should the presence picker be unsensitive while network is offline? Or can we be "Available" (with some visual hints) to indicate that we'd like to sign in to IM as soon as the network is on?

- Some visual hints showing that we are connecting to IM (a spinner?) would be good.

- GTalk uses "Sign out" instead of "Unavailable". Maybe we could do the same?

We also discussed future improvements regarding the avatar chooser. I described them in a separated bug as that's definitely post 3.2 material: bug #656628

Florian is on vacation this week, so I guess I'm picking this up to get it in before feature/ui freeze.

Do we *really* want a pop-up menu for presence? This means we now have two different kinds of submenus in two adjacent menus (user and network). (Not to mention having code for two different kinds of submenus.)

Also, most of Guillaume's questions above need to be figured out before UI freeze.

(In reply to comment #34)
> Florian is on vacation this week, so I guess I'm picking this up to get it in
> before feature/ui freeze.
>
> Do we *really* want a pop-up menu for presence? This means we now have two
> different kinds of submenus in two adjacent menus (user and network). (Not to
> mention having code for two different kinds of submenus.)

I'm not massively enthusiastic about it, but I haven't see a better solution, and it enables us to address the issues described in the original bug report. I was hoping to hear some feedback from Jon or Jimmac about it, tbh.
 
> Also, most of Guillaume's questions above need to be figured out before UI
> freeze.

OK...

(In reply to comment #32)
> Some notes from our BoF during the desktop summit:
> 
> - Should the presence picker display custom message status (like "At the pub")?

I guess so; it's not very nice displaying those and not being able to set them, but neither do we want to give an inaccurate impression of how the user is presenting themselves to others.

> - What's the exact semantic of the status displayed? The current status or the
> desired status? For example, if network is offline in NetworkManager, we can't
> obvioulsy be connected to IM. Should the presence picker be unsensitive while
> network is offline? Or can we be "Available" (with some visual hints) to
> indicate that we'd like to sign in to IM as soon as the network is on?

It should display the current status.

> - Some visual hints showing that we are connecting to IM (a spinner?) would be
> good.

A spinner is definitely off the cards (considering that using one in the network menu has been ruled out). Me and Florian have discussed using the equivalent of the network acquiring symbolic icons but I'd like to see if we can get away without them first.

> - GTalk uses "Sign out" instead of "Unavailable". Maybe we could do the same?

In GTalk the combobox contains a labelled 'Sign out of chat'. If you select it, the combobox is replaced by a 'Sign in' button. I don't think that changing the label to 'Sign out' without changing the widgety makes much sense.

(In reply to comment #35)
> (In reply to comment #34)
> > - GTalk uses "Sign out" instead of "Unavailable". Maybe we could do the same?
> 
> In GTalk the combobox contains a labelled 'Sign out of chat'. If you select it,
> the combobox is replaced by a 'Sign in' button. I don't think that changing the
> label to 'Sign out' without changing the widgety makes much sense.

We can definitely think about different strings here, by the way. Unavailable was largely meant as a place holder when I drew the mockups.

The suggestion to use 'Sign out' alludes to the fact that 'unavailable' could be misunderstood as meaning that you are connected to chat and displaying your status as unavailable, when in fact you are not connected at all. This should be mitigated by the status icon (user-offline), although that's not much help if you're using a screen reader.

One alternative could be to use 'Offline' instead of 'Unavailable'. We'd have to be sure that users understand it as purely relating to chat though.

The Google approach could be good too, of course, although it would require more work to implement.

Created attachment 194503 [details] [review]
popup-menu: Add support for child menus

Allow opening a popup menu from another menu. While the child menu
is open, events on the parent menu are blocked. The parent menu
is kept open when the child menu is closed; the child menu on the
other hand is closed with the parent, e.g. when the focus moves
to another toplevel menu.
This feature will be used to implement combo box menu items.

Created attachment 194504 [details] [review]
popup-menu: Add combo box menu item

Introduce a new menu widget, which displays the active item from
a set of options, and pops up a child menu to allow changing the
active item when activated.

Created attachment 194505 [details] [review]
popup-menu: Expand switch menu items

Given that our menus contain at most two columns, all switch widgets
in menus end up in the last columns, and thus aligned with the right
menu edge.
However, the updated user status menu will contain a section which
ignores the menu's column layout, so the switch might end up in
the middle of the menu if the overall width is determined by said
section.
At least for now, we always want the switch to align with the end,
so just expand switch menu items rather than adding an option.

Created attachment 194506 [details] [review]
status-menu: Implement new mockups

The current user status menu allow to set the session status,
which also influences the IM status when signed in with
mission-control. However, the way it is presented to the user
makes it hard to figure out how the statuses interact or that
there are two distinct status in the first place.
Therefore, use a separate control for each status, and update the
overall look to match gnome-contacts.

(In reply to comment #34)
> Florian is on vacation this week, so I guess I'm picking this up to get it in
> before feature/ui freeze.

Thanks!

I'll try to be around some, and react to reviews/requests, but internet is flaky and the weather fine ...

I've attached a squashed version of the user-status-update branch (rebased to master with some minor fixes for recent tp-glib breakage).

I spoke with Jon about this on IRC yesterday. He's unhappy with the design. I've got some ideas for how to address the issues he raised, but coming up with a new approach won't happen overnight.

Comment on attachment 194503 [details] [review]
popup-menu: Add support for child menus

Blah. I really dislike that we now have two different kinds of submenus. If we're keeping it this way, we need to eventually differentiate them better in the code.

Other than that, the code looks fine.

Comment on attachment 194504 [details] [review]
popup-menu: Add combo box menu item

>+    // Really want to test for Clutter.Container, but that's an interface,
>+    // which we don't support :(
>+    if (actor instanceof St.Container || actor instanceof Clutter.Group) {

File a gjs bug and add a link to the comment?

Alternatively, you could just duck-type and see if it has a get_children() method.

>+        Main.layoutManager.addChrome(this._menu.actor, { affectsStruts: false });

affectsStruts: false is the default now, so you can remove that.

>+        _ensureStyle(this._menu.actor);

why is this needed?

Comment on attachment 194506 [details] [review]
status-menu: Implement new mockups

random: should we rename the js file and class name while we're doing all this? We never call it the "status menu" any more...

>+.status-chooser-user-icon {
>+    border: 2px solid #8b8b8b;
>+    border-radius: 5px;
>+    width: 64px;
>+    height: 64px;

The width and height need to be in pts or it ends up being totally misaligned with larger or smaller text zoom levels. There are various other fixes needed for that too (eg, the line-wrapping stuff). I'll attach patches.

>+        //this._IMStatusChanged(this._accountMgr, presence, s, msg);

cruft?

>+                                                   _DIALOG_ICON_SIZE);

you copied this code from endSessionDialog.js but forgot to copy the definition of _DIALOG_ICON_SIZE.

>+    _onOnlineAccountSActivate: function() {

weird capitalization ("S")

Created attachment 194629 [details] [review]
popupMenu: do height-for-width negotiation

Created attachment 194631 [details] [review]
status-menu: remove line-wrapping kludge

Now that PopupMenuItem supports width-for-height, we can just wrap
normally (though we still need a ShellGenericContainer claiming a
natural width of "1", or else the menu will expand rather than
wrapping it).

Created attachment 194806 [details] [review]
popup-menu: Add combo box menu item

(In reply to comment #44)
> Alternatively, you could just duck-type and see if it has a get_children()
> method.

I went with that.


> affectsStruts: false is the default now, so you can remove that.

Done.


> >+        _ensureStyle(this._menu.actor);
> 
> why is this needed?

We want to pop up the menu so that the currently active item aligns with the source item.

Created attachment 194807 [details] [review]
status-menu: Implement new mockups

(In reply to comment #45)
> (From update of attachment 194506 [details] [review])
> random: should we rename the js file and class name while we're doing all this?
> We never call it the "status menu" any more...

Yeah, we could do that ... does userStatusMenu sound good?


> >+        //this._IMStatusChanged(this._accountMgr, presence, s, msg);
> 
> cruft?

Yes.


> >+                                                   _DIALOG_ICON_SIZE);
> 
> you copied this code from endSessionDialog.js but forgot to copy the definition
> of _DIALOG_ICON_SIZE.

Woops. Fixed.


> >+    _onOnlineAccountSActivate: function() {
> 
> weird capitalization ("S")

Fixes as well.

Review of attachment 194629 [details] [review]:

Looks mostly good.

::: js/ui/popupMenu.js
@@ +228,3 @@
         for (let i = 0; i < this._children.length; i++) {
             let child = this._children[i];
+            let width = this._columnWidths ? this._columnWidths[i] : child.actor.get_preferred_width(-1);

Looks unnecessary - the variable isn't used anywhere.

Review of attachment 194631 [details] [review]:

Minor nit about the wrapping, otherwise looks good.

::: js/ui/statusMenu.js
@@ +96,3 @@
     _wrapperGetPreferredWidth: function(actor, forHeight, alloc) {
+        alloc.min_size = 1;
+        alloc.natural_size = 1;

That behavior is slightly different that the previous one - we now start wrapping any name which is wider than the "do not disturb" switch, while previously we only wrapped names longer than WRAP_WIDTH (in other words: I don't consider my own name particularly long).

How about something like this:

 let [min, natural] = this.label.get_preferred_width(-1);
 alloc.min_size = Math.min(min, WRAP_WIDTH);
 alloc.natural_size = Math.min(natural, WRAP_WIDTH);

I had some discussions about this yesterday which reaffirmed my belief that we can move forward with it for 3.2. A few relevant points:

 * We need to have a way to disable IM from the user menu.

 * You need to be able to switch on notification inhibit (labelled do not disturb in the mockups) when IM is disabled. This implies that we need two controls, one for IM status and one for notification inhibit.

 * Though a combobox in a menu is generally not advisable, these are not ordinary menus, and the combobox has the advantage of mirroring the layout of contacts and IM status elsewhere.

Jon suggested that we change 'Do Not Disturb' to 'Notifications'; I'd be happy to make that change, although I have to say that I'm not 100% about it. Will people understand what 'notifications' are? Also, this switch doesn't disable all notifications, does it?

(In reply to comment #52)
> Jon suggested that we change 'Do Not Disturb' to 'Notifications'; I'd be happy
> to make that change, although I have to say that I'm not 100% about it. Will
> people understand what 'notifications' are? Also, this switch doesn't disable
> all notifications, does it?

Correct, notifications marked as 'critical' will still be displayed. By the way, the patch in bug 652718 has the same problem.

Comment on attachment 194807 [details] [review]
status-menu: Implement new mockups

good, but should be squashed with the updated version of the wrapping patch (which doesn't exist quite yet)

Created attachment 195072 [details] [review]
popupMenu: do height-for-width negotiation

(remove unused line)

Created attachment 195073 [details] [review]
status-menu: remove line-wrapping kludge

updated to have a minimum width, specified in pt rather than px so it
scales. Also fix indentation in a few places. to be squashed.

(In reply to comment #49)
> > random: should we rename the js file and class name while we're doing all this?
> > We never call it the "status menu" any more...
> 
> Yeah, we could do that ... does userStatusMenu sound good?

Sure. Or just userMenu.

Review of attachment 195072 [details] [review]:

LGTM

Review of attachment 195073 [details] [review]:

I'm afraid it's still not working as expected; in addition to the issue stated below, the combobox overlaps the switch if the user name is wrapped.

::: js/ui/statusMenu.js
@@ +94,3 @@
     _wrapperGetPreferredWidth: function(actor, forHeight, alloc) {
+        alloc.min_size = 1;
+        alloc.natural_size = 1;

Still not quite the same:
  1. min-width adds extra space to shorter names
       (I don't really care)
  2. somehow this confuses the column code
      (e.g. getColumnWidths() returns [1] for this
        row, rather than the adjusted widths (after the
        call to st_theme_node_adjust_preferred_width());
       I'm a bit clueless why exactly this is happening, but
       we need to fix this before landing ...

(In reply to comment #57)
> Sure. Or just userMenu.

Fine by me - I reckon I can consider a patch which simply renames the file a-c-n?

Attachment 194503 [details] pushed as 5d2b7e2 - popup-menu: Add support for child menus
Attachment 194505 [details] pushed as 709193d - popup-menu: Expand switch menu items
Attachment 194806 [details] pushed as d0d82cd - popup-menu: Add combo box menu item
Attachment 194807 [details] pushed as 0751a90 - user-menu: Implement new mockups

As agreed with Dan on IRC, I pushed my patches with a statusMenu.js=>userMenu.js patch preceding them, with some minor fixes due to Ray's popup changes and the style parts of attachment 195073 [details] [review] merged into the main patch.

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

Comment on attachment 195072 [details] [review]
popupMenu: do height-for-width negotiation

Attachment 195072 [details] pushed as caade78 - popupMenu: do height-for-width negotiation

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

Created attachment 196928 [details] [review]
status-menu: remove line-wrapping kludge

====

after rebasing, this is the current state of the patch, and I don't
see either of the two problems you mention... (I'm using this to test:

-            this._name.label.set_text(this._user.get_real_name());
+            this._name.label.set_text(this._user.get_real_name() + ' ' + this._user.get_real_name() + ' ' + this._user.get_real_name());

Created attachment 196929 [details]
worksforme

Created attachment 196986 [details] [review]
userMenu: fix line wrapping

The previous wrapping code hardcoded a width in pixels, making it
non-text-zoom-friendly. Specify a CSS width in pts, fix the userMenu
code to completely opt out of the popupMenu column behavior, and fix
the popupMenu width-for-height code in the non-columned case.

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

Review of attachment 196986 [details] [review]:

The down arrow in the combo box ends up misaligned (i.e. right after the label instead of at the end of the item).

(Also, the popupMenu changes are probably better split out)

Created attachment 197000 [details] [review]
popupMenu: fix a few width-for-height cases

specifically, non-columned menus, and span==-1

Created attachment 197001 [details] [review]
userMenu: fix line wrapping

The previous wrapping code hardcoded a width in pixels, making it
non-text-zoom-friendly. Specify a CSS width in pts, and fix the
userMenu code to completely opt out of the popupMenu column behavior.
Hack PopupComboBoxMenuItem slightly to deal with the fact that the
pop-up no longer gets setColumnWidth'ed.

Review of attachment 197001 [details] [review]:

Let's go with this!

(The design originally called for ellipsization after wrapping around once, but imo less hacky code is more important than wrapping behavior for long long user names ...)

Review of attachment 197000 [details] [review]:

Looks right.

Attachment 197000 [details] pushed as ed7d492 - popupMenu: fix a few width-for-height cases
Attachment 197001 [details] pushed as ab67c0f - userMenu: fix line wrapping