The default bug view has changed. See this FAQ.

Clean up borders for attachment lists

RESOLVED FIXED in Thunderbird 13.0

Status

Thunderbird
Theme
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: squib, Assigned: squib)

Tracking

unspecified
Thunderbird 13.0

Thunderbird Tracking Flags

(thunderbird13 fixed)

Details

Attachments

(8 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 597643 [details] [diff] [review]
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.
Attachment #597643 - Flags: ui-review?(nisses.mail)
Attachment #597643 - Flags: review?(bwinton)
(Assignee)

Updated

5 years ago
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
Created attachment 597644 [details]
Before/after on Linux
(Assignee)

Comment 2

5 years ago
Created attachment 597645 [details]
Before/after on XP
(Assignee)

Comment 3

5 years ago
Created attachment 597646 [details]
Before/after on Aero
(Assignee)

Updated

5 years ago
Attachment #597645 - Attachment description: Before/After on XP → Before/after on XP
Looks good on Linux. Will make sure it also works for all theme variants on Windows before I give ui-r+
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.
(Assignee)

Comment 6

5 years ago
Whoops, I probably screwed up when I merged all the patches together. I'll post a better one tonight.
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 on attachment 597643 [details] [diff] [review]
Fix the borders

setting ui-r minus since due to issues with the Windows theme part.
Attachment #597643 - Flags: ui-review?(nisses.mail) → ui-review-
(Assignee)

Comment 9

5 years ago
Created attachment 598073 [details] [diff] [review]
Fix Aero theme

This should resolve the issues with the Aero theme, including the format toolbar border.
Attachment #597643 - Attachment is obsolete: true
Attachment #598073 - Flags: ui-review?(nisses.mail)
Attachment #598073 - Flags: review?(bwinton)
Attachment #597643 - Flags: review?(bwinton)
Comment on attachment 598073 [details] [diff] [review]
Fix Aero theme

Works as expected now!
ui-review+
Attachment #598073 - Flags: ui-review?(nisses.mail) → ui-review+
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.
Attachment #598073 - Flags: review?(bwinton) → review+
(Assignee)

Comment 12

5 years ago
(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.
(Assignee)

Comment 13

5 years ago
Checked in: http://hg.mozilla.org/comm-central/rev/dcc10c245fc7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
(Assignee)

Comment 14

5 years ago
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.
Attachment #598073 - Flags: approval-comm-beta?
Attachment #598073 - Flags: approval-comm-aurora?
(Assignee)

Comment 15

5 years ago
Created attachment 600602 [details] [diff] [review]
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.
Attachment #600602 - Flags: ui-review?(nisses.mail)
Attachment #600602 - Flags: review?(nisses.mail)
(Assignee)

Updated

5 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 16

5 years ago
Created attachment 600717 [details]
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.
(Assignee)

Comment 17

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

Comment 18

5 years ago
(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

5 years ago
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

5 years ago
Created attachment 600798 [details]
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

5 years ago
Created attachment 600819 [details]
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 on attachment 598073 [details] [diff] [review]
Fix Aero theme

Looks like there's still issues here, so not taking for beta at this time.
Attachment #598073 - Flags: approval-comm-beta? → approval-comm-beta-
(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 on attachment 600602 [details] [diff] [review]
Fix XP theme

Looks good!
Attachment #600602 - Flags: ui-review?(nisses.mail)
Attachment #600602 - Flags: ui-review+
Attachment #600602 - Flags: review?(nisses.mail)
Attachment #600602 - Flags: review+
So is this ready to land? Should we be getting the reviewed attachment landed before merge?
(Assignee)

Comment 26

5 years ago
Whoops, forgot about this. Checked in:

http://hg.mozilla.org/comm-central/rev/6af3f1722764
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 27

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

This should go onto aurora, since the previous checkin happened before the merge.
Attachment #600602 - Flags: approval-comm-aurora?
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.
Attachment #598073 - Flags: approval-comm-aurora? → approval-comm-aurora-
Comment on attachment 600602 [details] [diff] [review]
Fix XP theme

but yes, this patch should land on aurora now.
Attachment #600602 - Flags: approval-comm-aurora? → approval-comm-aurora+
Checked in:

http://hg.mozilla.org/releases/comm-aurora/rev/a247b5a311e0
status-thunderbird13: --- → fixed
You need to log in before you can comment on or make changes to this bug.