Closed Bug 964066 Opened 6 years ago Closed 6 years ago

Restyle the Quick filter bar

Categories

(Thunderbird :: Theme, defect)

All
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 29.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(3 files, 1 obsolete file)

Like bug 963946 this bug is to remove the border between the two QFB rows and to fix the different toolbar height when the tags are shown/not shown. On Aero the tags should become back the tag colors to make the recognition easier.
Attached patch patch (obsolete) — Splinter Review
Fixes for XP and Vista/7/8
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8365677 - Flags: ui-review?(josiah)
Attachment #8365677 - Flags: review?(josiah)
Attached image Win7.png
Upper half without patch on Aero and Classic. Remark the different button height.
Lower half with patch.
Attached image XP.png
The same on XP. Upper half without patch and lower half with patch on Luna blue and Classic.
Comment on attachment 8365677 [details] [diff] [review]
patch

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

So this look good to me. I do have a few questions though. r+ with responses to my questions.

::: mail/themes/windows/mail/quickFilterBar-aero.css
@@ +134,5 @@
>  
> +#qfb-boolean-mode {
> +  margin: 0 1px;
> +  padding-top: 0 !important;
> +  padding-bottom: 0 !important;

Did you check to make sure the !importants are necessary?

@@ +191,5 @@
>  }
>  
> +#quick-filter-bar toolbarbutton,
> +#quick-filter-bar toolbarbutton:hover:active,
> +#quick-filter-bar toolbarbutton[checked] {

If we're sharing all these rules with all three selectors why don't we just use "#quick-filter-bar toolbarbutton"? Why do we need to include :hover:active and [checked]?
Attachment #8365677 - Flags: ui-review?(josiah)
Attachment #8365677 - Flags: ui-review+
Attachment #8365677 - Flags: review?(josiah)
Attachment #8365677 - Flags: review+
(In reply to Josiah Bruner [:JosiahOne] from comment #4)
> Comment on attachment 8365677 [details] [diff] [review]
> patch
> 
> Review of attachment 8365677 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So this look good to me. I do have a few questions though. r+ with responses
> to my questions.
> 
> ::: mail/themes/windows/mail/quickFilterBar-aero.css
> @@ +134,5 @@
> >  
> > +#qfb-boolean-mode {
> > +  margin: 0 1px;
> > +  padding-top: 0 !important;
> > +  padding-bottom: 0 !important;
> 
> Did you check to make sure the !importants are necessary?

Yes, this is needed to override this http://mxr.mozilla.org/comm-central/source/mail/themes/windows/mail/mailWindow1-aero.css#231
 
> @@ +191,5 @@
> >  }
> >  
> > +#quick-filter-bar toolbarbutton,
> > +#quick-filter-bar toolbarbutton:hover:active,
> > +#quick-filter-bar toolbarbutton[checked] {
> 
> If we're sharing all these rules with all three selectors why don't we just
> use "#quick-filter-bar toolbarbutton"? Why do we need to include
> :hover:active and [checked]?

You're right, I moved it to the first #quick-filter-bar toolbarbutton rule and removed a padding: 3px on the #quick-filter-bar toolbarbutton:hover:active, #quick-filter-bar toolbarbutton[checked="true"] rule.
Attachment #8365677 - Attachment is obsolete: true
Attachment #8366057 - Flags: ui-review+
Attachment #8366057 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/63f631f47649
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
You need to log in before you can comment on or make changes to this bug.