Closed Bug 954133 Opened 10 years ago Closed 10 years ago

Provide a way to merge contacts

Categories

(Instantbird Graveyard :: Contacts window, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: florian)

References

Details

(Whiteboard: [0.3-blocking])

Attachments

(5 files, 2 obsolete files)

*** Original post on bio 698 at 2011-02-16 16:27:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch Early WIP (obsolete) — Splinter Review
*** Original post on bio 698 as attmnt 522 at 2011-02-16 16:27:00 UTC ***

This WIP only handles the drag&drop operation on the buddy list. The code in imContacts.js is not implemented yet.
Known issues:
 - tooltip of the dragged contact gets in the way while dragging
 - the :-moz-drag-over pseudo class persists after a successfully finished drop.
*** Original post on bio 698 at 2011-02-16 16:27:40 UTC ***

(Blocking 0.3 because this is a key point of the roadmap.)
Whiteboard: [0.3-blocking]
OS: Other → All
Hardware: x86 → All
Attached patch Patch v2 (obsolete) — Splinter Review
*** Original post on bio 698 as attmnt 523 at 2011-02-17 18:22:00 UTC ***

This works.

Probably not ready to be used though, because of the 2 issues mentioned in comment 0, and:
 - when looking at a contact in the buddy list, there's no way to know it's not just a single buddy. We probably need to change the tooltip, and provide a way to "expand" a contact.
 - there's no way to detach a buddy from a contact, so the "merge" operation is not reversible.
 - the buddies are not ordered inside a contact, so which buddy will be used for a conversation is undefined if several buddies have the same availability.
 - conversations don't deal with contacts:
  * if a contact starts using a different protocol, it will open a new conversation tab
  * if the buddy goes offline, it won't auto-switch to a different protocol [this may be addressed in a follow up]
  * there's no way to select by hand which buddy of the contact will received the messages.

Feel free to look and comment :).
Comment on attachment 8352263 [details] [diff] [review]
Early WIP

*** Original change on bio 698 attmnt 522 at 2011-02-17 18:22:58 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352263 - Attachment is obsolete: true
Assignee: nobody → florian
Status: NEW → ASSIGNED
Attached patch Patch v3 (part1)Splinter Review
*** Original post on bio 698 as attmnt 541 at 2011-02-25 00:09:00 UTC ***

(In reply to comment #2)

>  - the buddies are not ordered inside a contact, so which buddy will be used
> for a conversation is undefined if several buddies have the same availability.

This new patch addresses this point (even though it provides no UI to change the order).
Comment on attachment 8352264 [details] [diff] [review]
Patch v2

*** Original change on bio 698 attmnt 523 at 2011-02-25 00:09:38 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352264 - Attachment is obsolete: true
*** Original post on bio 698 as attmnt 542 at 2011-02-25 00:11:00 UTC ***

Interdiff between attachment 8352264 [details] [diff] [review] (bio-attmnt 523) and attachment 8352282 [details] [diff] [review] (bio-attmnt 541).
*** Original post on bio 698 at 2011-02-25 00:27:40 UTC ***

Comment on attachment 8352282 [details] [diff] [review] (bio-attmnt 541)
Patch v3 (part1)

Pushed as https://hg.instantbird.org/instantbird/rev/9afcfbd70e11

Next: the UI work!
*** Original post on bio 698 as attmnt 544 at 2011-03-02 23:13:00 UTC ***

(In reply to comment #0)

> Known issues:
>  - tooltip of the dragged contact gets in the way while dragging
>  - the :-moz-drag-over pseudo class persists after a successfully finished
> drop.

Both issues are fixed by this attachment.

(In reply to comment #2)

>  - when looking at a contact in the buddy list, there's no way to know it's not
> just a single buddy. We probably need to change the tooltip, and provide a way
> to "expand" a contact.

This attachment adds a way to expand contacts.
The tooltip and context menu could be improved in another bug: maybe display the list of buddies in the tooltip with less information for the non-preferred buddies, and possibly list buddies in the context menu with a submenu for each to do actions on them (Not sure if that's really useful).
Adding an "Expand"/"Collapse" item to the context menu could be useful too (and may be usable on groups too).

>  - there's no way to detach a buddy from a contact, so the "merge" operation is
> not reversible.

Fixed.

>  - the buddies are not ordered inside a contact, so which buddy will be used
> for a conversation is undefined if several buddies have the same availability.

Fixed, with some UI this time!

>  - conversations don't deal with contacts: [...]

I plan to address this in the next part/patch.

> Feel free to look and comment :).
The same applies for this new patch of course. :)
*** Original post on bio 698 as attmnt 545 at 2011-03-03 00:38:00 UTC ***

Trivial changes made while self-reviewing attachment 8352285 [details] [diff] [review] (bio-attmnt 544).

A few things that I'm not really satisfied of about this attachment:
 - the code of _DragOk and _DragLeave is duplicated in buddy.xml and contact.xml. I would prefer if a single instance of it existed, in the window maybe.
 - in imIContact, I'm not convinced moveBuddyBefore and adoptBuddy should really be 2 different methods. The technical reason for the separation between these two methods is that adoptBuddy uses the contact setter which can't take any argument for the position of the newly inserted buddy (because setters can't take arguments). Merging these two methods would require several changes in imContacts.js, but it would simplify the callers in buddy.xml and contact.xml.
 - The contact-tagged and contact-moved notifications sent to the observer service are inconsistent depending on whether the moved tag was inherited or a local tag. Maybe they should just be removed if they are not reliable. Anyway, the code around moving buddies/contacts between tags and adding or removing tags needs some cleanup to be more understandable, less redundant and more reliable.
 - The duplicated code to convert an imIStatusInfo into a string used as an attribute for the CSS should be moved somewhere else (JS module?) to remove the duplication. I plan to address that in a follow-up patch or bug.
 - The -moz-Dialog color used in the gradient of the background of the list of buddies in an expanded contact may not be appropriate on non-Mac OSes.
*** Original post on bio 698 at 2011-03-03 01:30:56 UTC ***

Comment on attachment 8352285 [details] [diff] [review] (bio-attmnt 544)
Patch v1 (part2: blist UI changes)

Pushed as http://hg.instantbird.org/instantbird/rev/60cf5556e90a with the additional changes of attachment 8352286 [details] [diff] [review] (bio-attmnt 545).

The issues listed in comment 7 are all relatively minor and weren't worth delaying more the testing of this in nightlies.
*** Original post on bio 698 at 2011-03-04 00:50:54 UTC ***

http://hg.instantbird.org/instantbird/rev/80b1aaf9488c - Fix appearance of contact buttons and of droptarget highlight on Windows.
*** Original post on bio 698 at 2011-03-04 22:24:14 UTC ***

https://hg.instantbird.org/instantbird/rev/99f1e4bad760 - package expand.png and collapse.png for aero too.
Blocks: 954157
*** Original post on bio 698 as attmnt 574 at 2011-03-28 19:49:00 UTC ***

(In reply to comment #2)

>  - conversations don't deal with contacts:
>   * if a contact starts using a different protocol, it will open a new
> conversation tab

The attached patch addresses this issue.
*** Original post on bio 698 at 2011-03-28 20:43:48 UTC ***

Pushed attachment 8352316 [details] [diff] [review] (bio-attmnt 574) as https://hg.instantbird.org/instantbird/rev/c119343f2f9f

I'm resolving this bug. The remaining issues will be handled in follow-ups.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.3a2
Depends on: 954176
Depends on: 954177
Blocks: 954178
*** Original post on bio 698 at 2011-04-11 08:39:56 UTC ***

Pushed https://hg.instantbird.org/instantbird/rev/989f5bc8b942 to fix a mistake (pointed out by clokep) in attachment 8352316 [details] [diff] [review] (bio-attmnt 574).
Depends on: 954185
You need to log in before you can comment on or make changes to this bug.