Closed Bug 954506 Opened 10 years ago Closed 10 years ago

Improve look of contact drop target

Categories

(Instantbird Graveyard :: Contacts window, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: benediktp, Assigned: benediktp)

References

Details

(Whiteboard: [wanted])

Attachments

(5 files, 6 obsolete files)

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

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch Patch v1 (obsolete) — Splinter Review
*** 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.
*** Original post on bio 1071 as attmnt 886 at 2011-10-13 07:55:00 UTC ***

Screenshots of the look on Windows.
*** 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 ***
*** 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 ***
*** 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.
*** 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.
*** Original post on bio 1071 at 2011-11-14 10:54:56 UTC ***

CC'ing Florian Janßen.
Attached patch Patch v2 (obsolete) — Splinter Review
*** 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).
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
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
Attached image Screenshot for patch v2 (obsolete) —
*** 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.
Depends on: 954561
*** 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.
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]
*** 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.
*** 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)
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
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
Attached image Screenshot for v3, on Windows XP (obsolete) —
*** 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-
*** 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 :(
*** 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)
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
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
Attached image 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).
*** 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
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.