GNOME Bugzilla – Bug 591756
Warn the user before leaving chat rooms
Last modified: 2012-01-30 13:52:54 UTC
I often accidentally close IRC channel tabs, and it's incredibly annoying. Sometimes, I don't notice I've done it (maybe the focus was in the wrong place when I hit ^W; maybe I double-tapped it). <http://git.collabora.co.uk/?p=user/wjt/empathy.git;a=shortlog;h=refs/heads/close-confirmation> adds confirmation dialogs. There's one more special-case to add to the messages, but it works.
Created attachment 140728 [details] [review] Add empathy_chat_is_multi_user()
Created attachment 140729 [details] [review] Confirm closing chat room tabs I often accidentally close tabs with Control-W, and more occasionally by clicking the [X] by mistake. This is okay for 1-1 conversations (I can just reopen them) but is destructive for chat rooms. So let's make the user confirm.
Created attachment 140730 [details] [review] Confirm closing windows containing chat rooms. Getting the messages to feel right took quite a few iterations. Maybe we need one more case: when you only have one chat room in a window, and nothing else, the message from closing the window should probably be the same as it would have been if you'd hit ^W.
Created attachment 140731 [details] [review] Special-case closing a window with exactly one tab The confirmation message when closing a window containing one tab, which is a chat room, should be identical to the confirmation message when closing that tab with ^W.
I've added the remaining special-case.
This arguably does the wrong thing in the following pathological case: * Open a window containing one chat room and at least one other tab. * Close it. * While the close confirmation is open, click a blinking "new message" icon in the tray. This opens a new tab in the window. * Now when you confirm the dialog it closes the window including the new tab.
+ gtk_dialog_add_button (GTK_DIALOG (dialog), + _("Leave chat room"), GTK_RESPONSE_ACCEPT); Isn't that a bit long for a button? Don't know how strict are the HIG about the "labels of button should be verb" rule. Maybe "Leave" would be enough? I'm sure some users will be annoyed by this dialog. I think we should make it optionnal using a gconf key as gnome-terminal does. Note: I generally attach a squashed diff of my branch instead of one patch per commit. The main goal of the attached patch is to mark the bug as "proposed patch", most reviewers use a git tool (gitweb, etc) to actually review.
Oh, you should also include "#591756" in one of your commit message to be credited of your contribution in the NEWS file.
(In reply to comment #7) > + gtk_dialog_add_button (GTK_DIALOG (dialog), > + _("Leave chat room"), GTK_RESPONSE_ACCEPT); > > Isn't that a bit long for a button? Don't know how strict are the HIG about the > "labels of button should be verb" rule. Maybe "Leave" would be enough? "Leave room"? I named the buttons following Gnome Terminal's "Close terminal" and "Close window" buttons. > I'm sure some users will be annoyed by this dialog. I think we should make it > optionnal using a gconf key as gnome-terminal does. Okay. > Note: I generally attach a squashed diff of my branch instead of one patch per > commit. The main goal of the attached patch is to mark the bug as "proposed > patch", most reviewers use a git tool (gitweb, etc) to actually review. Okay, I'll hack that into git-bz. (I find it much nicer to use than git-send-bugzilla.) (In reply to comment #8) > Oh, you should also include "#591756" in one of your commit message to be > credited of your contribution in the NEWS file. Okay.
Once this branch is merged, this feature could be extended to fix bug 569316 as well (warn about just received messages on close/exit).
(In reply to comment #9) > > I'm sure some users will be annoyed by this dialog. I think we should make it > > optionnal using a gconf key as gnome-terminal does. > > Okay. Actually, on second thoughts I don't agree here. I don't think people leave chat rooms intentionally very often at all; the slightly increased overhead of doing so is not a big deal. Terminals are a different matter: people open and close terminals all day. Secret options in gconf make me sad. Could we initially have no such option, and if it actually annoys people add one then?
I don't have a strong opinion here. If another Empathy developper is fine with that that's OK for me.
from my personal experience and from the ui in some other programs the dialog which comes when the chat rooms are closed should have a checkbox and the text "Dont show again" or something. This makes it really easy to fix for those who get annoyed by this behaviour ( and there will be many including me) while still serving the purpose.
*** Bug 600146 has been marked as a duplicate of this bug. ***
Let's do this for 2.32.
Still not fixed in 2.32? It's really easy to accidentally kill chat state. Could be related to bug 599184, which requests the ability to close the window without parting the channel. Maybe that should be considered here? Perhaps the usual "closing" a chat case could be more of a "hide" action? (tough to get the feedback for this right though)
(In reply to comment #12) > I don't have a strong opinion here. If another Empathy developper is fine with > that that's OK for me. I don't think we need a setting. Prompting on all chat rooms would be fine. Alternatively we could fix bug #599184 instead.
Created attachment 206422 [details] [review] Confirm closing chat room tabs I often accidentally close tabs with Control-W, and more occasionally by clicking the [X] by mistake. This is okay for 1-1 conversations (I can just reopen them) but is destructive for chat rooms. So let's make the user confirm.
Created attachment 206423 [details] [review] Confirm closing windows containing chat rooms. Getting the messages to feel right took quite a few iterations. Maybe we need one more case: when you only have one chat room in a window, and nothing else, the message from closing the window should probably be the same as it would have been if you'd hit ^W.
Created attachment 206424 [details] [review] Special-case closing a window with exactly one tab The confirmation message when closing a window containing one tab, which is a chat room, should be identical to the confirmation message when closing that tab with ^W.
Created attachment 206426 [details] [review] Shorten "Leave chat room" to "Leave room"
Created attachment 206427 [details] [review] Don't warn before leaving disconnected chatrooms
Review of attachment 206422 [details] [review]: Looks good ::: src/empathy-chat-window.c @@ +261,3 @@ + + g_signal_connect (dialog, "response", + gchar *chat_name = empathy_chat_dup_name (chat); Using gtk_dialog_run() would make things easier but that's fine.
Review of attachment 206423 [details] [review]: ++
Review of attachment 206424 [details] [review]: ++
Review of attachment 206426 [details] [review]: ++
Review of attachment 206427 [details] [review]: ++
(In reply to comment #23) > Review of attachment 206422 [details] [review]: > > Looks good > > ::: src/empathy-chat-window.c > @@ +261,3 @@ > + > + g_signal_connect (dialog, "response", > + gchar *chat_name = empathy_chat_dup_name (chat); > > Using gtk_dialog_run() would make things easier but that's fine. I'm allergic to gtk_dialog_run() :) Merged, thanks!