Last Comment Bug 755754 - Australis - quickfilter toolbar styling
: Australis - quickfilter toolbar styling
Product: Thunderbird
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Richard Marti (:Paenglab)
Depends on:
Blocks: 737347 733856
  Show dependency treegraph
Reported: 2012-05-16 08:11 PDT by Andreas Nilsson (:andreasn)
Modified: 2012-07-18 17:27 PDT (History)
7 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch (12.30 KB, patch)
2012-05-17 05:45 PDT, Richard Marti (:Paenglab)
bugs: feedback+
Details | Diff | Splinter Review
patch in action (23.74 KB, image/png)
2012-05-17 05:45 PDT, Richard Marti (:Paenglab)
no flags Details
full patch with amateurish icons (12.55 KB, patch)
2012-05-17 07:16 PDT, Richard Marti (:Paenglab)
no flags Details | Diff | Splinter Review
patch with updated graphics (14.96 KB, patch)
2012-06-25 08:44 PDT, Andreas Nilsson (:andreasn)
no flags Details | Diff | Splinter Review
Patch (13.19 KB, patch)
2012-06-25 09:36 PDT, Richard Marti (:Paenglab)
bwinton: review+
bwinton: ui‑review+
Details | Diff | Splinter Review
Fix tests (6.56 KB, patch)
2012-07-04 09:39 PDT, Mike Conley (:mconley)
no flags Details | Diff | Splinter Review
Fix Mozmill tests (6.54 KB, patch)
2012-07-04 12:02 PDT, Mike Conley (:mconley)
standard8: review+
Details | Diff | Splinter Review

Description Andreas Nilsson (:andreasn) 2012-05-16 08:11:43 PDT
It would be desirable to match

* No colored icons
* The pin needs to be square on selected
Comment 1 Richard Marti (:Paenglab) 2012-05-17 05:45:34 PDT
Created attachment 624712 [details] [diff] [review]

First try to style the QFB bar with amateurish icons.

I'm using the mockup's toolbar colors. Additionally I'm using -moz-mac-chrome-inactive on this bar when TB is inactive.
The buttons have now a height of 16 px like mockup.
I removed the hover state. No other toolbar-button has now a hover state. But I added the active state with slightly darker icons like the other toolbar-buttons.

Andreas, if you like this, please can you make professional icons?
Comment 2 Richard Marti (:Paenglab) 2012-05-17 05:45:57 PDT
Created attachment 624713 [details]
patch in action
Comment 3 Andreas Nilsson (:andreasn) 2012-05-17 05:56:13 PDT
The selected button state needs a bit of gradient inside the button, and the Quick Filter label needs to be slightly lighter. Apart from that it looks good!
Comment 4 Andreas Nilsson (:andreasn) 2012-05-17 05:56:37 PDT
Comment on attachment 624712 [details] [diff] [review]

adding feedback+
Comment 5 Richard Marti (:Paenglab) 2012-05-17 07:16:19 PDT
Created attachment 624729 [details] [diff] [review]
full patch with amateurish icons

Selected buttons have now a gradient and a bottom border.
Also reduced the left/right padding from 8px to 4px.
The Quick Filter label is lighter now.
Comment 6 Richard Marti (:Paenglab) 2012-06-23 09:27:23 PDT
Comment on attachment 624729 [details] [diff] [review]
full patch with amateurish icons

Adding f? to Andreas to not forget to make correct icons.
Comment 7 Andreas Nilsson (:andreasn) 2012-06-25 08:44:52 PDT
Created attachment 636326 [details] [diff] [review]
patch with updated graphics

Here is Richards patch with new icons.
Comment 8 Richard Marti (:Paenglab) 2012-06-25 09:36:47 PDT
Created attachment 636345 [details] [diff] [review]

To not let Andreas review his own icons I'm asking Blake for review.
Comment 9 Blake Winton (:bwinton) (:☕️) 2012-06-26 13:29:54 PDT
Comment on attachment 636345 [details] [diff] [review]

Looks nice, ui-r=me!

And I see no problems with the code either, so r=me, too!

Comment 10 Ryan VanderMeulen [:RyanVM] 2012-06-26 18:06:02 PDT
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-06-27 16:50:48 PDT
Unfortunately, I had to back this out due to causing OSX Mozmill failures.
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/mozmill/session-store/test-session-store.js | test-session-store.js::test_message_pane_width_persistence
Comment 12 Richard Marti (:Paenglab) 2012-06-28 00:02:58 PDT
David, you are a MozMill specialist. Please can you check if the test needs a change. I've almost no knowledge in JS and would apreciate your help.

My patch changes the width of the QuickFilter buttons and this can end in a different width of the message pane.
Comment 13 Mike Conley (:mconley) 2012-07-04 09:39:53 PDT
Created attachment 639128 [details] [diff] [review]
Fix tests
Comment 14 Mike Conley (:mconley) 2012-07-04 12:02:49 PDT
Created attachment 639154 [details] [diff] [review]
Fix Mozmill tests
Comment 15 Mike Conley (:mconley) 2012-07-04 12:05:59 PDT
Comment on attachment 639154 [details] [diff] [review]
Fix Mozmill tests

Hey Mark, not sure if you're the right person for this r? - let me know if I should redirect.

You're probably wondering why the tests need to be changed at all... well, to be honest, I'm not sure. Something about Richard's style change upset the way this test runs - in particular, it upset the way the splitter is selected and moved on OSX.

Unfortunately, I wasn't able to determine *why* the splitter is putting up such a fuss. Having read the comments in the test, I don't think I'm the first one to wrestle with that one too.

Like the comments say, however, the splitter isn't the thing being tested here - it's simply the storage and retrieval of session state. So I don't think we're weakening the tests very much with this change.

Try builds with this patch:

Note You need to log in before you can comment on or make changes to this bug.