Closed Bug 906264 Opened 11 years ago Closed 10 years ago

Linux - Update Compose Window UI

Categories

(Thunderbird :: Theme, enhancement)

All
Linux
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 29.0

People

(Reporter: jsbruner, Assigned: jsbruner)

References

(Depends on 1 open bug)

Details

(Whiteboard: [ux-feature-wanted-31])

Attachments

(6 files, 9 obsolete files)

109.91 KB, image/png
Details
106.99 KB, image/png
Details
110.90 KB, image/png
Details
1.29 KB, image/png
Details
27.72 KB, patch
jsbruner
: review+
Paenglab
: ui-review+
Details | Diff | Splinter Review
16.63 KB, image/png
Details
Similar to bug 867161, but for Linux. Also note that this *depends* on that bug for the XUL layout changes.
Attached patch Patch. (obsolete) — Splinter Review
It's about time, but here's the first version of the linux compose UI. I'll upload a screenshot of it too shortly.
Attachment #832002 - Flags: ui-review?(richard.marti)
Attachment #832002 - Flags: review?(richard.marti)
Attached image Screenshot of new look (obsolete) —
Comment on attachment 832004 [details]
Screenshot of new look

Why are the addressing fields and subject field grayed out? I don't think they should.
(In reply to Josiah Bruner [:JosiahOne] from comment #1)
> Created attachment 832002 [details] [diff] [review]
> Patch.
> 
> It's about time, but here's the first version of the linux compose UI. I'll
> upload a screenshot of it too shortly.

I made only a quick peek on the patch. Here my first feedback:
You are using hard coded colors, please use system-colors. Linux (and also Windows) have a lot of different themes where your fixed colors will clash. Please change to system-colors.

You added linux-noise.png as a background. This can give the same effect as on folder tree with dark themes. Do we want use it now or should we wait with this background until the issue with dark themes is solved and introduce i then?

The subject line is on both sides a little it to long compared to the addressing lines. Please can you fix this in the next patch?

I'll try this patch this evening and can give more feedback.
(In reply to Magnus Melin from comment #3)
> Comment on attachment 832004 [details]
> Screenshot of new look
> 
> Why are the addressing fields and subject field grayed out? I don't think
> they should.

They change to normal appearance when hovered or active. This is made to let the header area look a little bit lighter (and to look like the addressing field of a postcard).
Attached image Patch in action (obsolete) —
In the upper half of the image is your patch with a slightly different grey theme and a dark theme. Good visible is the different grey of the FormatToolbar on light theme. On dark theme it's totally wrong. Also the bottom borders are hard to see on light theme.

On the lower half of the image I used system colors for the border and textfield backgrounds. Note, it's only a DOMi quick hack and I used threedhighlight for the borders and -moz-field for the active textfield background. There could be better colors, but I used the default colors for the textfields. I think a 1px border is okay here and the default 2px border isn't needed.

Josiah, please can you also move the dropdown arrow a little bit inwards? Now it is completely at the edge and looks weird. Maybe a 2px - 3px padding would look better.
Comment on attachment 832002 [details] [diff] [review]
Patch.

I'm cancelling the review for now until the changes are in a new patch.
Attachment #832002 - Flags: ui-review?(richard.marti)
Attachment #832002 - Flags: review?(richard.marti)
(In reply to Richard Marti [:Paenglab] from comment #5)
> (In reply to Magnus Melin from comment #3)
> > Comment on attachment 832004 [details]
> > Screenshot of new look
> > 
> > Why are the addressing fields and subject field grayed out? I don't think
> > they should.
> 
> They change to normal appearance when hovered or active. This is made to let
> the header area look a little bit lighter (and to look like the addressing
> field of a postcard).

Yes, but it still looks (and feels) very broken. Graying out is usually used only for disabled stuff, and I don't recall ever seeing it used like this - so it's just very confusing.
Attached patch Patch V2 (obsolete) — Splinter Review
This addresses most of the preliminary feedback. Will upload screenshots momentarily.
Attachment #832002 - Attachment is obsolete: true
Attachment #832697 - Flags: ui-review?(richard.marti)
Attachment #832697 - Flags: review?(richard.marti)
Attached image New Look - Light Theme (obsolete) —
Image of what the new patch looks like with a light theme...
Attachment #832004 - Attachment is obsolete: true
Attached image New Look - Dark Theme (obsolete) —
Image of what it looks like with a dark theme.
Comment on attachment 832697 [details] [diff] [review]
Patch V2

Review of attachment 832697 [details] [diff] [review]:
-----------------------------------------------------------------

It looks already appealing but the FormatToolbar looks on some themes like Ambiance or Radiance (both Ubuntu themes) like an alien. Screenshot follows. Also on Radiance the threedhighlight colored lines are hardly visible and the selected textboxes aren't looking good contoured. For this I'm giving ui-r-.

In code there are two in console visible errors and some code could be optimized. r-

::: mail/themes/linux/mail/compose/messengercompose.css
@@ +255,3 @@
>  #compose-toolbar-sizer {
> +  background-color: rgb(242, 242, 242);
> +  border-bottom: 1px solid #DCDCDC;

Hard coded colors. You could also remove the whole #compose-toolbar-sizer because on Linux when -moz-appearance: splitter is set, then no custom styling is working.

@@ +264,5 @@
> +  -moz-appearance: none !important;
> +  border-top: 1px solid transparent;
> +  border-left: 1px solid transparent;
> +  border-right: 1px solid transparent;
> +  border-bottom: 1px solid threedhighlight;

This could be shortened to:
border: 1px solid transparent;
border-bottom-color: threedhighlight;

@@ +266,5 @@
> +  border-left: 1px solid transparent;
> +  border-right: 1px solid transparent;
> +  border-bottom: 1px solid threedhighlight;
> +  border-bottom-left-radius: 0px;
> +  border-top-right-radius: 0px;

Setting of border-radius isn't needed, it isn't set any in toolkit.

@@ +283,5 @@
> +  background-image: none;
> +  border-top: 1px solid threedhighlight;
> +  border-left: 1px solid threedhighlight;
> +  border-right: 1px solid threedhighlight;
> +  border-bottom: 1px solid threedhighlight;

Only one line with border-color: threedhighlight; needed. The width and style are already defined.

@@ +343,5 @@
> +.dummy-row-cell:not(:first-child) {
> +  border-top: 1px solid transparent !important;
> +  border-left: 1px solid transparent !important;
> +  border-right: 1px solid transparent !important;
> +  border-bottom: 1px solid threedhighlight !important;

The same as for #msgSubject with only two lines.

@@ +361,5 @@
> +  background-image: none;
> +  border-top: 1px solid threedhighlight !important;
> +  border-left: 1px solid threedhighlight !important;
> +  border-right: 1px solid threedhighlight !important;
> +  border-bottom: 1px solid threedhighlight !important;

Again, a one liner would work.

@@ +379,5 @@
>  }
>  
> +#composeContentBox {
> +  box-shadow: 0px 2px 4px rgba(0, 0, 0, 0.4) inset;
> +  -moz-appearance: -moz-field;

-moz-appearance: -moz-field; isn't valid -> error in console. It works also without it.

@@ +422,5 @@
> +  background: transparent;
> +  height: 22px;
> +  line-height: 1;
> +  border: 1px solid transparent;
> +  border-bottom: 1px solid threedhighlight;

border-bottom-color would be enough.

@@ +423,5 @@
> +  height: 22px;
> +  line-height: 1;
> +  border: 1px solid transparent;
> +  border-bottom: 1px solid threedhighlight;
> +  border-radius: 0;

No border-radius definition needed.

@@ +437,5 @@
>  
>  /* ::::: format toolbar ::::: */
>  
>  #FormatToolbar {
> +  -moz-appearance: -moz-field;

-moz-appearance: -moz-field; isn't valid -> error in console. Change it to none and add the grain, then it fits with the addressing widget.

@@ +442,3 @@
>    color: WindowText;
> +  margin-left: 6px;
> +  margin-right: 6px; 

Trailing whitespace.
Attachment #832697 - Flags: ui-review?(richard.marti)
Attachment #832697 - Flags: ui-review-
Attachment #832697 - Flags: review?(richard.marti)
Attachment #832697 - Flags: review-
Attached image Screenshot on Radiance (obsolete) —
The right half shows the how it looks with the patch. The FormatToolbar has a other color and the lines are hardly visible.

On the right is a proposal with 2px lines and two different colors. This may be a little to much but at minimum one of the colors is on all my themes visible. On High Contrast theme only the dark line is visible as black line. Also the textboxes have always a border around them with this approach.
Attached patch patch proposal (obsolete) — Splinter Review
The patch for my proposal.
Attached patch Patch V3. (obsolete) — Splinter Review
That's actually a briliant idea! I love it. This patch uses your proposal and addresses the review feedback. Thanks Richard.
Attachment #832697 - Attachment is obsolete: true
Attachment #833161 - Attachment is obsolete: true
Attachment #833242 - Flags: ui-review?(richard.marti)
Attachment #833242 - Flags: review?(richard.marti)
Attached image New Look - Mint X
Screenshot of the composer with the Mint X theme.
Attachment #832385 - Attachment is obsolete: true
Attachment #832702 - Attachment is obsolete: true
Attachment #832704 - Attachment is obsolete: true
Attachment #833159 - Attachment is obsolete: true
Attached image New Look - Blackbird
Screenshot with the blackbird theme.
Attached image New Look - Adwaita
Screenshot of the new look with the Adwaita theme.
Comment on attachment 833242 [details] [diff] [review]
Patch V3.

Review of attachment 833242 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry to come this late but there is to less padding on both sides. You could revert the negative margins with padding on #composeContentBox. But thanks to this I saw the idendity menulist has 1px less margin than the textboxes below.

Then wouldn't noise.png instead of linux-noise.png be enough? The images aren't in a shared directory there we need to differentiate them.

The code is looking good so r=me but ui-r- for the padding issues.
Again sorry to come so late with this but I'm sure the next patch would be the final. Maybe the OS X patch could also be adapted to this then.

::: mail/themes/linux/mail/compose/messengercompose.css
@@ +206,4 @@
>  #attachmentbucket-sizer {
>    border-top: none;
>    border-bottom: none;
> +  -moz-margin-end: 3px;

Should be no more needed with the padding on #composeContentBox.

@@ +376,5 @@
> +   * a margin-left or margin-right change.
> +   */
> +
> +  margin-right: -3px;
> +  margin-left: -3px;

With adding padding 3px on both sides the normal gaps are rebuilt. And you don't need to add on different child elements to add paddings or margins.

@@ +384,5 @@
> +  box-shadow: 0px 1px 2px rgba(0, 0, 0, 0.3) inset;
> +}
> +
> +#content-frame {
> +  -moz-margin-end: 3px;

Should be no more needed with the padding on #composeContentBox.

@@ +388,5 @@
> +  -moz-margin-end: 3px;
> +}
> +
> +#attachments-box {
> +  -moz-padding-end: 3px;

Should be no more needed with the padding on #composeContentBox.

@@ +434,3 @@
>    color: WindowText;
> +  margin-left: 6px;
> +  margin-right: 6px;

With the padding on #composeContentBox where would be only 3px needed..
Attachment #833242 - Flags: ui-review?(richard.marti)
Attachment #833242 - Flags: ui-review-
Attachment #833242 - Flags: review?(richard.marti)
Attachment #833242 - Flags: review+
Attached file OST.ZIP
Screenshot showing the different margins of menulist and textbox.
Attached file OST.ZIP
Screenshot showing the different margins of menulist and textbox.
Attachment #833375 - Attachment is obsolete: true
Comment on attachment 833242 [details] [diff] [review]
Patch V3.

Review of attachment 833242 [details] [diff] [review]:
-----------------------------------------------------------------

Huh great, my review is erased during the planned work! Okay, here again.

I'm sorry to come so late but the gap between the window border and the elements in addressing header is to small. If you compensate the negative margin with 3px padding on #composeContentBox the gap is correct again. And the special margins and padding on children aren't needed.

And when you are on changing this, wouldn noise.png instead of linux-noise.png also work? This file isn't in a common place there we have to differentiate between platforms.

Maybe you could also change both things on OS X patch.

The idendity menulist is 1px nearer to the window border than the textboxes. Screenshot follows.

The code is looking good, so r=me but ui-r- for the margin issues. But I'm sure the next patch passes the review.

::: mail/themes/linux/mail/compose/messengercompose.css
@@ +206,4 @@
>  #attachmentbucket-sizer {
>    border-top: none;
>    border-bottom: none;
> +  -moz-margin-end: 3px;

Should be no more needed with the padding on #composeContentBox.

@@ +335,5 @@
> +  -moz-border-top-colors: none !important;
> +  -moz-border-left-colors: none !important;
> +  -moz-border-right-colors: none !important;
> +  -moz-border-bottom-colors: ThreeDShadow ThreeDHighlight !important;
> +

remove this empty line.

@@ +376,5 @@
> +   * a margin-left or margin-right change.
> +   */
> +
> +  margin-right: -3px;
> +  margin-left: -3px;

Here you could add padding-right and -left 3px to revert the negative margin.

@@ +384,5 @@
> +  box-shadow: 0px 1px 2px rgba(0, 0, 0, 0.3) inset;
> +}
> +
> +#content-frame {
> +  -moz-margin-end: 3px;

Should be no more needed with the padding on #composeContentBox.

@@ +388,5 @@
> +  -moz-margin-end: 3px;
> +}
> +
> +#attachments-box {
> +  -moz-padding-end: 3px;

Should be no more needed with the padding on #composeContentBox.

@@ +434,3 @@
>    color: WindowText;
> +  margin-left: 6px;
> +  margin-right: 6px;

This can be reduced with the padding on #composeContentBox.
Attachment #833242 - Flags: ui-review?(richard.marti)
Attachment #833242 - Flags: ui-review-
Attachment #833242 - Flags: review?(richard.marti)
Attachment #833242 - Flags: review+
Attached image Margin issue
Thanks to the small margin the different margin is good visible between menulist and textbox to window border.
Whiteboard: [ux-feature-wanted-31]
Status: NEW → ASSIGNED
Attached patch Patch.Splinter Review
Addresses review comments and fixes the margin issue. Re-flagging for ui-review. Carrying review+ flag...
Attachment #833242 - Attachment is obsolete: true
Attachment #8347287 - Flags: ui-review?(richard.marti)
Attachment #8347287 - Flags: review+
Comment on attachment 8347287 [details] [diff] [review]
Patch.

Review of attachment 8347287 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good.

But one thing: The text in #msgIdentity isn't aligned with the text from addressingWidget and subject-box. Could you add a -moz-padding-start: 3px? But please check this on your system.

With this ui-r=me

When you are on it and already mentioned in comment 22, could you also rename linux-noise.png to noise.png? I see no need to have the platform name in it.
Attachment #8347287 - Flags: ui-review?(richard.marti) → ui-review+
https://hg.mozilla.org/comm-central/rev/39b45b030c76
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
It seems the padding issue is still there (today's trunk, linux).  A second problem is, when an attachment is present, the x attachment(s) label text is also about 4px too high.  All text on the first row should align, methinks.
Please can you file a bug with screenshots?
Depends on: 1010714
Is this the correct bug thread for the Thunderbird 31 Compose Window UI - I don't like it and would like to change it to the previous version... or edit the CSS to make it have the white text boxes. Image here - http://www.anony.ws/image/Donf
Paul, this bug is closed. If you want to change anything specific, please open a new bug report. However, just wanting to revert it is probably unlikely to convince the devs to do so. Frequently it's just a matter of getting used to something new after a "yuck" moment, but taste sure differs.

The nice thing about Mozilla applications: You can override styles on your own if you don't like it. Have a look at http://forums.mozillazine.org/viewtopic.php?p=13624709#p13624709 for Bozz's style code mostly reverting it, and http://kb.mozillazine.org/UserChrome.css for instructions where to add it.

That code was written for Windows and seems to work on Mac OSX, I haven't tried it on Linux.
Is there an Add-on, to disable this UI-change?
(In reply to Markus Multrus from comment #32)
> Is there an Add-on, to disable this UI-change?

No, not currently. I may create one in the near future though if a significant number of people email me with complaints/preferences.
Depends on: 1042732
Depends on: 1042780
Depends on: 1115034
You need to log in before you can comment on or make changes to this bug.