Improve look of contact drop target

RESOLVED FIXED in 1.2

Status

Instantbird
Contacts window
--
enhancement
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Mic, Assigned: Mic)

Tracking

Details

(Whiteboard: [wanted])

Attachments

(5 attachments, 6 obsolete attachments)

(Assignee)

Description

4 years ago
*** Original post on bio 1071 at 2011-10-13 07:53:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
(Assignee)

Comment 1

4 years ago
Created attachment 8352628 [details] [diff] [review]
Patch v1

*** Original post on bio 1071 as attmnt 885 at 2011-10-13 07:53:00 UTC ***

The contact drop target doesn't have a very attractive look at the moment. This patch attempts to fix that. It imitates the look of the new "about:blank" icon of Firefox.
(Assignee)

Comment 2

4 years ago
Created attachment 8352629 [details]
Screenshots v1 with Windows theme (Aero, Classic)

*** Original post on bio 1071 as attmnt 886 at 2011-10-13 07:55:00 UTC ***

Screenshots of the look on Windows.
Created attachment 8352630 [details] [diff] [review]
alternative patch. Dummy buddy more similar to normal buddy row

*** Original post on bio 1071 as attmnt 887 at 2011-10-13 14:29:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Created attachment 8352685 [details]
Screenshot with applied patch 887

*** Original post on bio 1071 as attmnt 943 at 2011-10-26 11:17:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
(Assignee)

Comment 5

4 years ago
*** Original post on bio 1071 at 2011-11-06 14:44:34 UTC ***

I'm not working on any of these -> unassigning from myself so I don't block anyone who'd be willing to take one of them.
(Assignee)

Comment 6

4 years ago
Created attachment 8352737 [details]
Mockup, drop target styled similar but discernable to normal buddies.

*** Original post on bio 1071 as attmnt 995 at 2011-11-14 10:52:00 UTC ***

I think I found a really nice way to display this. Checkout the new attachment.
(Assignee)

Comment 7

4 years ago
*** Original post on bio 1071 at 2011-11-14 10:54:56 UTC ***

CC'ing Florian Janßen.
(Assignee)

Comment 8

4 years ago
Created attachment 8352787 [details] [diff] [review]
Patch v2

*** Original post on bio 1071 as attmnt 1045 at 2011-12-02 22:54:00 UTC ***

Here's a patch that does something like shown in attachment 8352737 [details] (bio-attmnt 995).
(Assignee)

Comment 9

4 years ago
Comment on attachment 8352628 [details] [diff] [review]
Patch v1

*** Original change on bio 1071 attmnt 885 at 2011-12-02 22:54:01 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352628 - Attachment is obsolete: true
(Assignee)

Comment 10

4 years ago
Comment on attachment 8352629 [details]
Screenshots v1 with Windows theme (Aero, Classic)

*** Original change on bio 1071 attmnt 886 at 2011-12-02 22:54:01 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352629 - Attachment is obsolete: true
(Assignee)

Comment 11

4 years ago
Created attachment 8352788 [details]
Screenshot for patch v2

*** Original post on bio 1071 as attmnt 1046 at 2011-12-02 22:58:00 UTC ***

A screenshot showing what the drop target looks like with patch v2.

Updated

4 years ago
Depends on: 954561

Comment 12

4 years ago
*** Original post on bio 1071 at 2011-12-02 23:55:51 UTC ***

Just a note that the statusIcon/protoIcon css would/will be moved to a separate CSS file by the patch in bug 954561 (bio 1128), which would trivially conflict with the current version of this patch.

Updated

4 years ago
Whiteboard: [1.2-wanted]
*** Original post on bio 1071 at 2012-01-30 14:04:43 UTC ***

Mic said on IRC that attachment 8352787 [details] [diff] [review] (bio-attmnt 1045) isn't ready for review yet (some cleanup left to do, as some of the new elements may not be actually needed), so this is unlikely to make 1.2.
Whiteboard: [1.2-wanted] → [wanted]
(Assignee)

Comment 14

4 years ago
*** Original post on bio 1071 at 2012-03-10 14:30:24 UTC ***

Comment on attachment 8352787 [details] [diff] [review] (bio-attmnt 1045)
Patch v2

>+      <xul:stack class="prplBuddyIcon">
>+        <xul:image class="dummyBuddyIcon"/>
>+      </xul:stack>

The stack is the element in question. Keeping it would allow to reuse the CSS rule from the other buddy binding (since they have this stack) - removing it would need some extra paddings/margins for the icon somewhere. So it's either an element less or less CSS. I don't care which.
(Assignee)

Comment 15

4 years ago
Created attachment 8353120 [details] [diff] [review]
Patch v3 - removed extra stack, adapted CSS

*** Original post on bio 1071 as attmnt 1367 at 2012-04-18 15:50:00 UTC ***

I unbitrotted the patch and removed the extra stack that was there before. The text is now sitting one pixel lower than before and I don't know how to fix that.

See screenshot for v3.
Attachment #8353120 - Flags: review?(florian)
(Assignee)

Comment 16

4 years ago
Comment on attachment 8352787 [details] [diff] [review]
Patch v2

*** Original change on bio 1071 attmnt 1045 at 2012-04-18 15:50:20 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352787 - Attachment is obsolete: true
(Assignee)

Comment 17

4 years ago
Comment on attachment 8352788 [details]
Screenshot for patch v2

*** Original change on bio 1071 attmnt 1046 at 2012-04-18 15:50:20 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352788 - Attachment is obsolete: true
(Assignee)

Comment 18

4 years ago
Created attachment 8353121 [details]
Screenshot for v3, on Windows XP

*** Original post on bio 1071 as attmnt 1368 at 2012-04-18 15:53:00 UTC ***

Here's a screenshot, taken on Win XP. (No Aero today but I can take one later if needed).

The width of the contact list is that of a new profile and the (imo) important part of the message "Drop another contact here..." is visible.
Comment on attachment 8353120 [details] [diff] [review]
Patch v3 - removed extra stack, adapted CSS

*** Original change on bio 1071 attmnt 1367 at 2012-04-25 22:32:33 UTC ***

>diff --git a/instantbird/locales/en-US/chrome/instantbird/instantbird.dtd b/instantbird/locales/en-US/chrome/instantbird/instantbird.dtd

>-<!ENTITY contactDropTarget             "drop a contact here to merge it">
>+<!ENTITY contactDropTarget2            "Drop another contact here to merge them.">

I'm not convinced this string change is a good idea. I tend to think the current string is already too long and was getting cropped. Making it even longer doesn't seem good, but I would be curious to here the rational for this change.

Some random ideas:
- provide a tooltip when hovering that string, so that the string is fully readable.
- shorten that string, for example to "drop contact to merge" (obviously not good English, but the important words are more likely to be visible)
- provide a way longer description in a tooltip for the dummy buddy, to teach the user about contacts.


Looks great otherwise! r-ing because of the string, but I would really like to take this soon. Thanks for working on this :-).
Attachment #8353120 - Flags: review?(florian) → review-
(Assignee)

Comment 20

4 years ago
*** Original post on bio 1071 at 2012-05-11 10:30:32 UTC ***

(In reply to comment #14)
> Some random ideas:
> - provide a tooltip when hovering that string, so that the string is fully
> readable.

If that's fine with you I'd attach a patch for this. 

> - shorten that string, for example to "drop contact to merge" (obviously not
> good English, but the important words are more likely to be visible)
> - provide a way longer description in a tooltip for the dummy buddy, to teach
> the user about contacts.

Sorry, I couldn't come up with a decent long description :(
(Assignee)

Comment 21

4 years ago
Created attachment 8353255 [details] [diff] [review]
Patch v4, with tooltip

*** Original post on bio 1071 as attmnt 1503 at 2012-05-21 18:21:00 UTC ***

I had this patch ready already when I asked for an opinion, so I'll just attach it and wait for r+ or r- now instead ;)

In lack of a good long description, I chose to show the label as tooltip too. I had to remove the "mousethrough" attribute from some elements (a thing which I was a bit wary about to do) to make the tooltiptext appear but from test nothing broke. Dropping contacts anywhere still works as expected.
Attachment #8353255 - Flags: review?(florian)
(Assignee)

Comment 22

4 years ago
Comment on attachment 8353120 [details] [diff] [review]
Patch v3 - removed extra stack, adapted CSS

*** Original change on bio 1071 attmnt 1367 at 2012-05-21 18:21:53 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353120 - Attachment is obsolete: true
(Assignee)

Comment 23

4 years ago
Comment on attachment 8353121 [details]
Screenshot for v3, on Windows XP

*** Original change on bio 1071 attmnt 1368 at 2012-05-21 18:21:53 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353121 - Attachment is obsolete: true
(Assignee)

Comment 24

4 years ago
Created attachment 8353256 [details]
Screenshot of patch v4

*** Original post on bio 1071 as attmnt 1504 at 2012-05-21 18:23:00 UTC ***

This is how it looks on Windows (Aero).

Comment 25

4 years ago
*** Original post on bio 1071 at 2012-05-21 19:47:38 UTC ***

There was a discussion of possible strings for this on #instantbird 2012-04-27. I think "merge them" isn't ideal. Lack of the perfect string shouldn't stop this patch though.
Comment on attachment 8353255 [details] [diff] [review]
Patch v4, with tooltip

*** Original change on bio 1071 attmnt 1503 at 2012-05-22 22:57:31 UTC ***

Looks ready to be tried on nightlies! Thanks

(In reply to comment #16)
> I
> had to remove the "mousethrough" attribute from some elements (a thing which I
> was a bit wary about to do) to make the tooltiptext appear but from test
> nothing broke. Dropping contacts anywhere still works as expected.

I forgot to remove these mousethrough attributes on the dummybuddy. They are needed on the normal buddies to ensure the preview displayed while dragging doesn't display only the dragged anonymous element but the whole buddy. Dummy buddies aren't draggable, so that attribute was pointless.
Attachment #8353255 - Flags: review?(florian) → review+
*** Original post on bio 1071 at 2012-05-22 23:08:20 UTC ***

Committed as http://hg.instantbird.org/instantbird/rev/53d6f99b35a3

Thanks Mic!
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.