Closed Bug 740765 Opened 12 years ago Closed 12 years ago

Starting or selecting a conversation with the mouse should focus the input box immediately

Categories

(Thunderbird :: Instant Messaging, defect)

Product:
Thunderbird
Component:
Instant Messaging
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(thunderbird14 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 15.0
Tracking Flags:
Tracking Status
thunderbird14 --- fixed

People

(Reporter: florian, Assigned: florian)

References

Details

Attachments

(1 file, 1 obsolete file)

Patch v2
6.76 KB, patch
florian
: review+
florian
: ui-review+
Bienvenu
: approval-comm-aurora+
Details | Diff | Splinter Review 
- When a conversation is selected with a mouse interaction, it should be immediately possible to type in that conversation (the input box should be focused).
- Also, starting a conversation with the mouse (for example by clicking on the "Start conversation" bubble) should select that conversation automatically (currently it's done only if the the contact was selected at the time the conversation was started).
Attached patch Patch (obsolete) — — Splinter Review 
Behavior implemented by this patch:
- When the user starts a conversation (either through the keyboard by pressing enter on a selected contact, or through the mouse by double clicking a contact or single clicking the chat bubble of a hovered contact), its textbox is focused immediately.
- Selecting a conversation with the mouse gives focus to the textbox immediately.
- Selecting a conversation through the keyboard (using the up/down arrow keys while the listbox of the left pane is focused) doesn't change the focus; it's still on the listbox.
- When the focus is on the listbox and a conversation is selected:
-- pressing the enter or return keys gives the focus to the input box of the conversation.
-- typing a character focuses the input box of the conversation AND resends the keyboard event so that the input box receives is.
Assignee: nobody → florian
Attachment #619075 - Flags: review?(bwinton)
Comment on attachment 619075 [details] [diff] [review]
Patch

So, I like the "typing will auto-focus the content box", but I found it didn't handle ctrl-v, which seemed a little annoying.  I also wonder about people mis-sending messages to the wrong contact.

Still, the rest of this patch seems like an improvement, so ui-r=me with the paste problem mentioned above fixed.

Now, on to the code-review.  :)


>+++ b/mail/components/im/content/chat-messenger-overlay.js
>@@ -330,16 +339,23 @@ var chatHandler = {
>+  onListItemClick: function(aEvent) {
>+    if (aEvent.button != 0 || aEvent.detail != 1)
>+      return;

It would be nice to have a comment here explaining which button you intend to handle.

>+++ b/mail/components/im/content/imcontact.xml
>@@ -198,17 +198,19 @@
>      <method name="openConversation">
>       <body>
>        <![CDATA[
>-         this.contact.createConversation();
>+         let prplConv = this.contact.createConversation();
>+         let uiConv = Services.conversations.getUIConversation(prplConv);
>+         chatHandler.focusConversation(prplConv);

You don't seem to use uiConv anywhere in this function...

Other than that I think I like it.  r=me, too.

Thanks,
Blake.
Attachment #619075 - Flags: ui-review+
Attachment #619075 - Flags: review?(bwinton)
Attachment #619075 - Flags: review+
Attached patch Patch v2 — — Splinter Review 
Updated patch taking into account Blake's comments. Carrying forward the r and ui-r flags.

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #3)

> >+++ b/mail/components/im/content/imcontact.xml
> >@@ -198,17 +198,19 @@
> >      <method name="openConversation">
> >       <body>
> >        <![CDATA[
> >-         this.contact.createConversation();
> >+         let prplConv = this.contact.createConversation();
> >+         let uiConv = Services.conversations.getUIConversation(prplConv);
> >+         chatHandler.focusConversation(prplConv);
> 
> You don't seem to use uiConv anywhere in this function...

Very good catch. chatHandler.focusConversation expects the uiConversation, not the prplConversation. This mistake wasn't visible during my testing because the prpl and ui conversation ids currently match, but they won't always match in the future.
Attachment #619075 - Attachment is obsolete: true
Attachment #619925 - Flags: ui-review+
Attachment #619925 - Flags: review+
http://hg.mozilla.org/comm-central/rev/9d1568bb5b2a
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
Attachment #619925 - Flags: approval-comm-aurora?
Attachment #619925 - Flags: approval-comm-aurora? → approval-comm-aurora+
I landed this on aurora for the closed tree, so that I could check for issues with the new automation:

https://hg.mozilla.org/releases/comm-aurora/rev/363737497ed6
Depends on: 756136
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: