727689 - Clean up borders for attachment lists

Status: RESOLVED FIXED

(Thunderbird :: Theme, enhancement)

Description

[Jim Porter (:squib)]

Attachment: Fix the borders

The borders on the attachment lists in the reader and composer are a bit ugly looking. We should fix this. Here's a patch, since I'm sick of looking at the ugliness every day. :)

I also made the total attachment size line up with each attachment's size in the composer.

Comment 1

[Jim Porter (:squib)]

Attachment: Before/after on Linux

Comment 2

[Jim Porter (:squib)]

Attachment: Before/after on XP

Comment 3

[Jim Porter (:squib)]

Attachment: Before/after on Aero

Comment 4

[Andreas Nilsson (:andreasn)]

Looks good on Linux. Will make sure it also works for all theme variants on Windows before I give ui-r+

Comment 5

[Andreas Nilsson (:andreasn)]

For some reason I keep getting a double border on Windows in the compose window still when I build this. I get the same results as in your screenshot only if I set the splitter to 1px and change the #FormatToolbar top-width to 0px

The attachment in a message looks nice so it the patch did bite.

Comment 6

[Jim Porter (:squib)]

Whoops, I probably screwed up when I merged all the patches together. I'll post a better one tonight.

Comment 7

[Andreas Nilsson (:andreasn)]

Upon closer inspection I noticed that you had the formatting bar turned off in your screenshot and this explains why I get a double border on the bottom and you don't. If you want to fix that in the code as well while you're poking around in that file it would be great!

Comment 8

[Andreas Nilsson (:andreasn)]

Comment on attachment 597643 [details] [diff] [review]
Fix the borders

setting ui-r minus since due to issues with the Windows theme part.

Comment 9

[Jim Porter (:squib)]

Attachment: Fix Aero theme

This should resolve the issues with the Aero theme, including the format toolbar border.

Comment 10

[Andreas Nilsson (:andreasn)]

Comment on attachment 598073 [details] [diff] [review]
Fix Aero theme

Works as expected now!
ui-review+

Comment 11

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 598073 [details] [diff] [review]
Fix Aero theme

>+++ b/mail/themes/qute/mail/compose/messengercompose-aero.css
>@@ -418,12 +422,16 @@
>+#attachmentBucketSize {
>+  color: #888a85;
>+}

Where did this colour come from?

That's a pretty trivial question, so I'm going to say r=me.

Thanks,
Blake.

Comment 12

[Jim Porter (:squib)]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #11)
> Comment on attachment 598073 [details] [diff] [review]
> Fix Aero theme
> 
> >+++ b/mail/themes/qute/mail/compose/messengercompose-aero.css
> >@@ -418,12 +422,16 @@
> >+#attachmentBucketSize {
> >+  color: #888a85;
> >+}
> 
> Where did this colour come from?

It originally came from the message header fields ("From" and the like). It's pretty commonly-used in our code (everywhere but XP): http://mxr.mozilla.org/comm-central/search?string=888a85

I do think we should turn that rule into "color: windowtext; opacity: 0.5;", which is what XP does. That's a bug for a different day though, but I did at least make XP consistent in its definitions here.

Comment 13

[Jim Porter (:squib)]

Checked in: http://hg.mozilla.org/comm-central/rev/dcc10c245fc7

Comment 14

[Jim Porter (:squib)]

Comment on attachment 598073 [details] [diff] [review]
Fix Aero theme

Should this go into aurora/beta? It's just a handful of minor CSS changes to clean things up visually. It's about as low-risk as it gets, and I think a little more prettiness always helps.

Comment 15

[Jim Porter (:squib)]

Attachment: Fix XP theme

Whoops. I missed a single character in one of the selectors that caused the top border on the attachment pane in the reader to be invisible when it's collapsed on XP. Here's the fix.

Comment 16

[Joe Sabash [:JoeS1]]

Attachment: attachmentbucket

This is with xp classic default theme.
Needs more visible indication of a sizer on left border.
Easy to miss, if you don't know it's there.

Comment 17

[Jim Porter (:squib)]

That's intentional and has ui-review from andreasn.

Comment 18

[Joe Sabash [:JoeS1]]

(In reply to Jim Porter (:squib) from comment #17)
> That's intentional and has ui-review from andreasn.

OK, but it seems inconsistent with other panes that are re-sizable.
All those have a double border that defines the panes extent.
Still, even that is not very discoverable to the new user.
We do have the tooltip that expands long file names, but no visual cue that the pane can be expanded.

Comment 19

[u382332]

I'm going to have to agree with Joe. The sizer should remain visible and consistent with the other panes so that users can easily 'discover' that the attachment bucket is resizable in order to see long file names.

Comment 20

[Joe Sabash [:JoeS1]]

Attachment: win7 screenshot

So..I have been reminded (elsewhere) that the winxp theme was the only theme that seemed to accentuate the sizer to the user.
Anyhow, here is what the attachmentbucket looks like in win7.
At the very least, the winxp rendering should at least delineate the expandable pane in the vertical.

@andreasn
Do we need a bug filed on the general discover ability of windows that can be re-sized.

Comment 21

[Joe Sabash [:JoeS1]]

Attachment: n7.2

Listen, I'm not meaning to spam this bug, but when we talk about "cleanup" and intuitive UI design, maybe we should look to the past sometimes and compare our current paradigm to that. I'm sure you will agree that the left border insinuates something different than a normal border (It's draggable) How do we convey that today. The clip is from Netscape 7.2

Comment 22

[Mark Banner (:standard8)]

Comment on attachment 598073 [details] [diff] [review]
Fix Aero theme

Looks like there's still issues here, so not taking for beta at this time.

Comment 23

[Andreas Nilsson (:andreasn)]

(In reply to Joe Sabash from comment #20)

> @andreasn
> Do we need a bug filed on the general discover ability of windows that can
> be re-sized.

Looking at some other Win7 applications (File Explorer and Windows Media Player, but also Firefox) it seems like the <-> cursor on hover to indicate resizing is a pretty well established pattern, so I think we're ok.

Comment 24

[Andreas Nilsson (:andreasn)]

Comment on attachment 600602 [details] [diff] [review]
Fix XP theme

Looks good!

Comment 25

[Mark Banner (:standard8)]

So is this ready to land? Should we be getting the reviewed attachment landed before merge?

Comment 26

[Jim Porter (:squib)]

Whoops, forgot about this. Checked in:

http://hg.mozilla.org/comm-central/rev/6af3f1722764

Comment 27

[Jim Porter (:squib)]

Comment on attachment 600602 [details] [diff] [review]
Fix XP theme

This should go onto aurora, since the previous checkin happened before the merge.

Comment 28

[Mark Banner (:standard8)]

Comment on attachment 598073 [details] [diff] [review]
Fix Aero theme

so this patch is now on aurora already, I'm not sure its that advantageous to take across to beta, unless Blake disagrees with me.

Comment 29

[Mark Banner (:standard8)]

Comment on attachment 600602 [details] [diff] [review]
Fix XP theme

but yes, this patch should land on aurora now.

Comment 30

[Mark Banner (:standard8)]

Checked in:

http://hg.mozilla.org/releases/comm-aurora/rev/a247b5a311e0