662142 - Adapt Thunderbird Aero theme changes to Calendar

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Richard Marti (:Paenglab)]

This Bug is to make Lightning looking better under Vista/Win7

Comment 1

[Richard Marti (:Paenglab)]

Attachment: Patch for Aero integration

This Patch changes following:
- get rid of the borders of the Task-view tool bar (also under XP)
- correct the -moz-image-region of the closers (also under XP)
- correct the -moz-image-region of the Task-view buttons (not centered and #task-actions-priority has an artefact)
- make #unifinder-searchBox and #task-addition-box fit better to the Tab color
- get rid of the thick dotted lines in the trees
- follow the appearance of the sidebarheader
- make all splitters like in TB
- give #today-pane-panel the border under Aero Glass like TB

I added bwinton for ui-r because andreasn has problems with his windows machine and bwinton in other reviews said Lightning needs changes for Aero.

Comment 2

[Decathlon]

(In reply to comment #1)

> This Patch changes following:

> - correct the -moz-image-region of the closers (also under XP)

Philipp, you don't need to checkin the patch in bug 650747 because this patch has the same fix.

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 537447 [details] [diff] [review]
Patch for Aero integration

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

All in all the patch looks very good. r=philipp

::: calendar/base/themes/winstripe/calendar-task-view.css
@@ +151,4 @@
>      -moz-appearance: none;
>      -moz-padding-end: 6px;
>      padding-top: 3px;
> +    border-top-width: 0;

I like Decathlon's solution with border: none better, so lets go with that.

@@ +46,5 @@
>  }
>  
>  #task-actions-priority {
>      list-style-image: url(chrome://calendar/skin/tasks-actions-aero.png);
> +    -moz-image-region: rect(0 52px 18px 34px);

Does this fix the wrong dimensions ssitter noted on bug 639799?

@@ +54,5 @@
>      list-style-image: url(chrome://messenger/skin/icons/mail-toolbar.png);
> +    -moz-image-region: rect(0 144px 18px 126px);
> +}
> +
> +@media all and (-moz-windows-default-theme) {

Pretty cool, I didn't know this was possible!

Comment 5

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

Comment on attachment 537447 [details] [diff] [review]
Patch for Aero integration

I don't think I'm allowed to ui-review Calendar code, so I'm going to pass this off to Daniel, because he's listed as a UI peer in https://wiki.mozilla.org/Calendar:Module_Ownership  :)

Comment 6

[Richard Marti (:Paenglab)]

(In reply to comment #4)

I'm awaiting the ui-r with the update of the patch.

> > +    border-top-width: 0;
> 
> I like Decathlon's solution with border: none better, so lets go with that.

My solution may saving some pico seconds because it changes only one attribute than three with border-top:none. But I'm no specialist in this, maybe I'm writing rubbish ;)

I'll change this in the update.

> @@ +46,5 @@
> >  }
> >  
> >  #task-actions-priority {
> >      list-style-image: url(chrome://calendar/skin/tasks-actions-aero.png);
> > +    -moz-image-region: rect(0 52px 18px 34px);
> 
> Does this fix the wrong dimensions ssitter noted on bug 639799?

Yes, this is exactly this.

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 537447 [details] [diff] [review]
Patch for Aero integration

Oh, don't worry, we're not that strict on such things, calendar is happy if there is someone looking at our patches! Daniel is pretty tied up with non-calendar things, so if you have time, I'd also value your review. If you'd like someone else to take care, then please move this to andreasn.

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to comment #6)
> (In reply to comment #4)
> > 
> > I like Decathlon's solution with border: none better, so lets go with that.
> 
> My solution may saving some pico seconds because it changes only one
> attribute than three with border-top:none. But I'm no specialist in this,
> maybe I'm writing rubbish ;)
Maybe you're right, but I'd prefer simplicity in this case. I don't think styling is our bottleneck in the UI :)

> 
> I'll change this in the update.
Since its already in decathlon's patch, I'd prefer you leave it out to avoid conflicts when checking in.

Comment 9

[Richard Marti (:Paenglab)]

Attachment: Patch for Aero integration

Removed the part which is checked in by bug 570124.

Carrying over the r+ from previous patch

Comment 10

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

Comment on attachment 537959 [details] [diff] [review]
Patch for Aero integration

Well, I think it looks a lot better, but there are some small changes I think you should make while you're digging in this code...

The search boxes in Calendar and Tasks are a little too square for my liking, as seen in http://dl.dropbox.com/u/2301433/Attachment/SquareSearch.png  (The two boxes with two pink lines linking them should be very similar looking.)  Hopefully that's an easy fix.

The forward and back buttons in the Today Pane seem really heavyweight, and I think it would look better if we could use the ones from our toolbar, as shown in http://dl.dropbox.com/u/2301433/Attachment/ForwardBack.png  Also in that image you can see that the line under "Events" doesn't quite line up with the top of the tab bar, and I think it _really_ should.  :)

So, I'll say ui-r=me with those fixed.

Thanks,
Blake.

Comment 11

[Stefan Sitter [:ssitter]]

> The forward and back buttons in the Today Pane seem really heavyweight

The buttons toggle the sidebar mode. The icons are actually the same as used in the Thunderbird folder pane on the left to toggle the display mode. If you change the Thunderbird icon Lightning will automatically pick up the changes.

Comment 12

[Richard Marti (:Paenglab)]

Attachment: Patch addressing the review

I fixed from ui-r the search boxes and the sidebar header height under Aero Glass. I haven't changed the arrows because as ssitter wrote, this should be changed in TB itself.

I'm asking for review again because of the changes I had to do. I'm not so happy with the negative margin I had to use for the .folderview-cycler. I had to do this because the toolbarbutton has a system min-height of 26px, which I can't change without loosing the nicer appearance of this buttons.

Comment 13

[Richard Marti (:Paenglab)]

Attachment: Cycle buttons with the arrows bwinton mentioned

The cycle buttons with this arrows are looking a little bit blurry.

Comment 14

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 538058 [details] [diff] [review]
Patch addressing the review

I'm surprised this doesn't wreak havoc under linux, but from simple testing it looks super. r=philipp

We really need to get gnomestripe working, I'm looking forward to after the release!

Comment 15

[Philipp Kewisch [:Fallen][📱📆]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/e6a000e3b33d>
-> FIXED

Comment 16

[Philipp Kewisch [:Fallen][📱📆]]

Backported to comm-miramar <http://hg.mozilla.org/releases/comm-miramar/rev/a17231dbb73d>