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

RESOLVED FIXED in Thunderbird 15.0

Status

Thunderbird
Instant Messaging
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

Trunk
Thunderbird 15.0

Thunderbird Tracking Flags

(thunderbird14 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
- 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).

Updated

6 years ago
Duplicate of this bug: 742637
(Assignee)

Comment 2

5 years ago
Created attachment 619075 [details] [diff] [review]
Patch

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+
(Assignee)

Comment 4

5 years ago
Created attachment 619925 [details] [diff] [review]
Patch v2

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+
(Assignee)

Comment 5

5 years ago
http://hg.mozilla.org/comm-central/rev/9d1568bb5b2a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
(Assignee)

Updated

5 years ago
Attachment #619925 - Flags: approval-comm-aurora?

Updated

5 years ago
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
status-thunderbird14: --- → fixed
(Assignee)

Updated

5 years ago
Depends on: 756136
You need to log in before you can comment on or make changes to this bug.