Make that:
https://live.gnome.org/Hackfests/IMContacts Social2011/Tasks/ShellDesignPresence

A transient notification for this seems like a good idea.

Need to decide on the text.

Created attachment 190103 [details] [review]
status-menu: Tell user notifications are blocked while busy

For most users it is obvious that the user status controls the
IM/online status, but the additional effect of suppressing
(non-critical) notifications is less so. Display a transient
notifications educating the user about the feature.

(In reply to comment #2)
> Need to decide on the text.

Right. I attached a proposal, but it'll need to be improved ...

Comment on attachment 190103 [details] [review]
status-menu: Tell user notifications are blocked while busy

>+            Main.overview.shellInfo.setMessage(_("You won't get any more notifications while busy"));

If we're going to be using that from outside the overview, then it should really be outside the overview. Maybe add Main.notify() to go along with Main.notifyError().

Created attachment 192830 [details] [review]
main: Add Main.notify() for simple system messages

... similar to Main.notifyError(), but don't duplicate the message
on stderr/in the log.

(In reply to comment #4)
> If we're going to be using that from outside the overview, then it should
> really be outside the overview. Maybe add Main.notify() to go along with
> Main.notifyError().

Sure. Not updating the other patch for now, as it conflicts with the status menu work in bug 652837.

Comment on attachment 192830 [details] [review]
main: Add Main.notify() for simple system messages

Attachment 192830 [details] pushed as 8bc85d4 - main: Add Main.notify() for simple system messages

Created attachment 194797 [details] [review]
status-menu: Tell user notifications are blocked while busy

Woops, forgot to attach an updated version rebased on the patches in bug 652837 ...

Review of attachment 194797 [details] [review]:

Sure.

Not sure if Florian is around to commit this

(In reply to comment #9)
> Not sure if Florian is around to commit this

I am, but I'd like to land bug 652837 first ...

Allan, Jon: you had some concerns here - can we try figuring it out (e.g. rename "Do Not Disturb" to "Notifications" and not show the popup, land this patch as-is, or with a different message, ...) before string freeze?

(In reply to comment #11)
> Allan, Jon: you had some concerns here - can we try figuring it out (e.g.
> rename "Do Not Disturb" to "Notifications" and not show the popup, land this
> patch as-is, or with a different message, ...) before string freeze?

From my conversation with Jon:

<mccann>	is do not disturb = notifications off?
<mccann>	if so then maybe it should just be called Notifications [Off]
<aday>	yes, dnd == no notifications
<mccann>	do not disturb is kinda wrong there
<mccann>	because that is a signal
<mccann>	it is a request to others
<mccann>	"do not disturb" has the implied "please"
<mccann>	Notifications [On/Off] is much clearer I think

That makes sense to me - let's change it to 'Notifications [ on / off ]'.

I'm not sure where that leaves this bug. Maybe this change will be enough to make the functionality clear?

Created attachment 195631 [details] [review]
user-menu: Rename "Do Not Disturb" to "Notifications"

At least for the foreseeable future, the gnome-session desktop
presence won't be used for anything but suppressing (non-urgent)
notifications. To clarify this behavior, rename the "Do Not Disturb"
switch to "Notifications" (and adjust the switch logic accordingly).

(In reply to comment #12)
> That makes sense to me - let's change it to 'Notifications [ on / off ]'.

Sure.


> I'm not sure where that leaves this bug. Maybe this change will be enough to
> make the functionality clear?

It does in my opinion. However, the interaction with the IM status is a lot more mysterious with that patch - why would disabling "Notifications" switch my chat status to "Busy"? On the other hand, the behavior itself still makes sense to me - when sidestepping empathy by only using gnome-shell for IM, turning off notifications means missing every single chat message; notifying chat partners about this (by setting the IM status accordingly) looks like the right (polite) thing to do ...

Comments?

(In reply to comment #13)
> Comments?
That's probably OK, but I'd change the text of the "You will not receive further notifications while busy" notification to explain this too. If you say something, say it all.

(In reply to comment #13)
> > I'm not sure where that leaves this bug. Maybe this change will be enough to
> > make the functionality clear?
> 
> It does in my opinion. However, the interaction with the IM status is a lot
> more mysterious with that patch - why would disabling "Notifications" switch my
> chat status to "Busy"? On the other hand, the behavior itself still makes sense
> to me - when sidestepping empathy by only using gnome-shell for IM, turning off
> notifications means missing every single chat message; notifying chat partners
> about this (by setting the IM status accordingly) looks like the right (polite)
> thing to do ...
> 
> Comments?

I'd agree with all of that.

(In reply to comment #14)
> (In reply to comment #13)
> > Comments?
> That's probably OK, but I'd change the text of the "You will not receive
> further notifications while busy" notification to explain this too. If you say
> something, say it all.

I think I'd rather dump this notification (turning "Notifications" off disables notifications - sounds rather obvious to me) and display another one when we change the IM status automatically to explain the reasoning behind this.

(In reply to comment #13)
> Created an attachment (id=195631) [details] [review]
> user-menu: Rename "Do Not Disturb" to "Notifications"

Ping? String freeze is about to start ...

Comment on attachment 195631 [details] [review]
user-menu: Rename "Do Not Disturb" to "Notifications"

ok, looks right

(string freeze doesn't start until the tarball is released, which wasn't going to happen yesterday since it was a US holiday and Owen wasn't around.)

Comment on attachment 195631 [details] [review]
user-menu: Rename "Do Not Disturb" to "Notifications"

Attachment 195631 [details] pushed as a5d0ac7 - user-menu: Rename "Do Not Disturb" to "Notifications"

(In reply to comment #18)
> (string freeze doesn't start until the tarball is released [...])

Oh, I wasn't aware of that - thanks!

so, this is still going to land ?

Created attachment 196128 [details] [review]
user-menu: Explain why disabling notifications changes IM status

While the current behavior of setting the IM status to "busy" while
notifications are disabled makes sense, as incoming messages are
very likely to be missed, it is not immediately obvious.
Display a transient notification to explain the behavior to the user.


(In reply to comment #20)
> so, this is still going to land ?

Maybe, but not the previous patch, which does no longer make much sense after renaming "Do Not Disturb" to "Notifications".

I attached a new patch which tries to explain why we adjust the IM status when disabling notifications - I think the message is way too wordy, but right now I am unable to come up with anything better (hey, we've got a public holiday over here today, so I shouldn't be in front of a computer anyway ;-)

Seriously, any suggestion to improve the notification will be much appreciated.

Here is some rewording that tries to avoid the passive language, and some of the repetitiveness:

Notifications are now disabled, including chat messages. Your online status has been adjusted to let others know that you might not see their messages.

Created attachment 196316 [details] [review]
user-menu: Explain why disabling notifications changes IM status

(In reply to comment #22)
> Here is some rewording that tries to avoid the passive language, and some of
> the repetitiveness

Sure, thanks!

Comment on attachment 196316 [details] [review]
user-menu: Explain why disabling notifications changes IM status


>-            if (presence == Tp.ConnectionPresenceType.AVAILABLE ||
>-                presence == Tp.ConnectionPresenceType.EXTENDED_AWAY) {

>+            if (this._currentPresence == Tp.ConnectionPresenceType.AVAILABLE ||
>+                this._currentPresence == Tp.ConnectionPresenceType.EXTENDED_AWAY)

it's not obvious to me that these are the same... and if they are, is there any reason to call get_most_available_presence() in _sessionStatusChanged rather than just using this._currentPresence there as well?

(In reply to comment #24)
> (From update of attachment 196316 [details] [review])
> 
> >-            if (presence == Tp.ConnectionPresenceType.AVAILABLE ||
> >-                presence == Tp.ConnectionPresenceType.EXTENDED_AWAY) {
> 
> >+            if (this._currentPresence == Tp.ConnectionPresenceType.AVAILABLE ||
> >+                this._currentPresence == Tp.ConnectionPresenceType.EXTENDED_AWAY)
> 
> it's not obvious to me that these are the same...

this._currentPresence is set in the handler of Tp.AccountManager::most-available-presence-changed, so I very much expect it to always represent the correct presence.
(If "expecting" it from telepathy is too weak, I wouldn't mind a call to get_most_available_presence() here as well ...)


> and if they are, is there any reason to call get_most_available_presence() in
> _sessionStatusChanged rather than just using this._currentPresence there as well?

Yes, if the user has set a custom status message, we want to preserve that (3rd return value of get_most_available_presence(), 3rd parameter to set_all_requested_presences()) ...

Attachment 196316 [details] pushed as cd7b9e1 - user-menu: Explain why disabling notifications changes IM status

Pushed with i18n team approval