Closed Bug 1229525 Opened 4 years ago Closed 4 years ago

Make the main toolbar the same height as the FX toolbar

Categories

(Thunderbird :: Theme, defect)

45 Branch
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

Attachments

(3 files, 2 obsolete files)

In bug 1211864 aleth saw the main toolbar is taller than the FX one which makes it a bit "bulky". Also the top border color of tabmail didn't is a bit darker than on FX under Yosemite+.
Attached patch Toolbar.patch (obsolete) — Splinter Review
This patch fixes the different height on all platforms. On OS X where is now no need to differ between the versions.

Also for Yosemite+ it fixes the tabmail border color.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8694397 - Flags: review?(aleth)
Attached image Screen Shot
Looks great :-)

Issues:

* Toolbar is now too narrow in the Chat/Tasks/Calendar tabs. But maybe that will be fixed when you fix the other search/filter bars.

* Inactive toolbar colour doesn't match Firefox (not sure if that's an issue).
Another tiny detail: it looks like the bottom border of the active toolbar might be a little too dark.
(In reply to aleth [:aleth] from comment #2)
> * Toolbar is now too narrow in the Chat/Tasks/Calendar tabs. But maybe that
> will be fixed when you fix the other search/filter bars.

To be clear, the toolbar changes height when you switch to those tabs.
Attached patch Toolbar.patch v2 (obsolete) — Splinter Review
(In reply to aleth [:aleth] from comment #2)
> 
> Issues:
> 
> * Toolbar is now too narrow in the Chat/Tasks/Calendar tabs. But maybe that
> will be fixed when you fix the other search/filter bars.

Yes, one part will be fixed by the searchBox patch. But I found also 1px which is no more needed. Until the searchBox patch lands where will be a 2px difference (the main toolbar will be too tall, chat should be correct like FX).

> * Inactive toolbar colour doesn't match Firefox (not sure if that's an
> issue).

I don't see this on Yosemite as FX and TB are using background: linear-gradient(hsl(0, 0%, 97%), hsl(0, 0%, 95%));

(In reply to aleth [:aleth] from comment #3)
> Another tiny detail: it looks like the bottom border of the active toolbar
> might be a little too dark.

Made it lighter now.
Attachment #8694397 - Attachment is obsolete: true
Attachment #8694397 - Flags: review?(aleth)
Attachment #8695402 - Flags: review?(aleth)
If you want test the toolbars are no more changing the height you can try it with bug 1230250 together.
Blocks: 1230250
Attached image searchbardetail.png
(In reply to Richard Marti (:Paenglab) from comment #6)
> If you want test the toolbars are no more changing the height you can try it
> with bug 1230250 together.

Tested together with this patch.

If you zoom in on the screenshot, you can see there's still three small details:
* Fx searchbar is 1px higher up
* Fx searchbar has a 1px shadow missing in Tb.
* Fx toolbar bottom border has a 1px lighter border above the 1px dark border.
(In reply to aleth [:aleth] from comment #7)
> Created attachment 8695507 [details]
> searchbardetail.png
> 
> If you zoom in on the screenshot, you can see there's still three small
> details:
> * Fx searchbar is 1px higher up
> * Fx searchbar has a 1px shadow missing in Tb.

Both are on the same height but the TB search has on top a light grey border and on bottom the shadow isn't painted or white.

> * Fx toolbar bottom border has a 1px lighter border above the 1px dark
> border.

I need to wait until DOMi works again to investigate why the shadow etc. aren't correct (hopefully tomorrow). It's hard to develop without it and change the things on the host platform and copy then the omni.ja through a USB stick to the OS X VM.
(In reply to Richard Marti (:Paenglab) from comment #8)
> I need to wait until DOMi works again to investigate why the shadow etc.
> aren't correct (hopefully tomorrow). It's hard to develop without it and
> change the things on the host platform and copy then the omni.ja through a
> USB stick to the OS X VM.

Does remote debugging work? (using the dev tools inspector)
(In reply to aleth [:aleth] from comment #9)
> Does remote debugging work? (using the dev tools inspector)

Not tried it on my VM as it very slow and have also issues with changing windows in Yosemite client. But the needed patch is now in m-c and I started a try build. So this evening I should be able to better dig into the issues.

Aleth, one question, is the FX also self built or from Mozilla? The OS X 10.11 SDK could also make some visual differences to the older SDKs like the doubled search placeholders.
(In reply to Richard Marti (:Paenglab) from comment #10)
> Aleth, one question, is the FX also self built or from Mozilla? The OS X
> 10.11 SDK could also make some visual differences to the older SDKs like the
> doubled search placeholders.

Good point. The Fx is from mozilla, but I should build TB with the release SDK version (what's that at the monent, 10.8?) to test your patches.
It seems it's 10.8 regarding https://developer.mozilla.org/en-US/docs/Simple_Thunderbird_build#OS_X_10.910.10_Notice

But you can also try https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=81ffd03b1ab2 when it's built. But I think it has the issues you noted.
I built with 10.8 and it made no difference (which is good).
Attached patch Toolbar.patch v3Splinter Review
All issues should be solved now.
Attachment #8695402 - Attachment is obsolete: true
Attachment #8695402 - Flags: review?(aleth)
Attachment #8696090 - Flags: review?(aleth)
Comment on attachment 8696090 [details] [diff] [review]
Toolbar.patch v3

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

Looks fantastic now, thanks!

Before or after landing, we should ask someone with a retina screen to test this patch series too. Maybe Fallen has one? Please f? or n? someone suitable.
Attachment #8696090 - Flags: review?(aleth) → review+
http://hg.mozilla.org/comm-central/rev/e433fbff27f7
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Version: unspecified → 45
Comment on attachment 8696090 [details] [diff] [review]
Toolbar.patch v3

Philipp, please can you check if the main toolbars are looking equal to the FX toolbar on HiDPI Mac? Specially the colors, gradient and border colors. The bottom border is darker than on FX to better circumvent the toolbar from the mail content.
Attachment #8696090 - Flags: feedback?(philipp)
(In reply to Richard Marti (:Paenglab) from comment #17)
> Philipp, please can you check if the main toolbars are looking equal to the
> FX toolbar on HiDPI Mac? Specially the colors, gradient and border colors.
> The bottom border is darker than on FX to better circumvent the toolbar from
> the mail content.

It's probably a good idea to apply the searchbar patch from bug 1230250 as well before testing this.
Comment on attachment 8696090 [details] [diff] [review]
Toolbar.patch v3

(In reply to Richard Marti (:Paenglab) from comment #17)
> Philipp, please can you check if the main toolbars are looking equal to the
> FX toolbar on HiDPI Mac? Specially the colors, gradient and border colors.
> The bottom border is darker than on FX to better circumvent the toolbar from
> the mail content.

I don't have a HiDPI mac unfortunately, still a mid 2010 model without retina.
Attachment #8696090 - Flags: feedback?(philipp)
You need to log in before you can comment on or make changes to this bug.