Closed Bug 1230250 Opened 4 years ago Closed 4 years ago

Add a global searchBox class

Categories

(Thunderbird :: Theme, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 45.0

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

Attachments

(1 file, 2 obsolete files)

With the global searchBox class we can centralise the styles to one file (searchBox.css). On Windows the same styles are until now on different files and on OS X and Linux we can now put our new styles in this file without duplicates.
Attached patch globalSearchBox.patch (obsolete) — Splinter Review
To apply cleanly this patch needs to be applied after bug 1228074.

This patch adds on all searchboxes the class and moves on Windows all styles to searchBox.css.
On Linux and OS X we use now -moz-appearance: none; to style them like we want (but it's like FX has done it :) ). It has also some small changes in it to make the toolbars the same height even with or without a searchbox.

I also removed the ellipsis from the QuickFilterBar searchbox placeholder text like it's done in gloda searchbox.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8695413 - Flags: review?(aleth)
Depends on: 1228074, 1229525
I noticed a couple of small things, some of which might be for other bugs:

Calendar tab: searchbox string is empty (this may be intentional? But all the other searchboxes seem to have strings.).

Tasks tab: "filter tasks" string still has ellipses. "Click here to add new task" box height is too small/doesn't match "Filter tasks" searchbox height (if that matters).

Chat tab nit: searchbar width is slightly different from that of the Mail tab.
Attached patch globalSearchBox.patch v2 (obsolete) — Splinter Review
The searchbox should look now correct like the FX one. Strange thing is on TB the 0.5px box-shadow isn't shown but on FX without problems. I changed it now to 1px and it's shown.
Attachment #8695413 - Attachment is obsolete: true
Attachment #8695413 - Flags: review?(aleth)
Attachment #8696091 - Flags: review?(aleth)
(In reply to aleth [:aleth] from comment #2)
> I noticed a couple of small things, some of which might be for other bugs:
> 
> Calendar tab: searchbox string is empty (this may be intentional? But all
> the other searchboxes seem to have strings.).

It's not a normal searchbox. The whole bar is more like a sentence like [filtered events] contain [expression]. The 'contain' describes what is expected in this field.

> Tasks tab: "filter tasks" string still has ellipses. "Click here to add new
> task" box height is too small/doesn't match "Filter tasks" searchbox height
> (if that matters).

I'll file a Lightning bug to remove the ellipses. The boxes are already before the my patches different in height. But I look into it and file a Lightning bug.

> Chat tab nit: searchbar width is slightly different from that of the Mail
> tab.

I don't see how this could be done. The boxes are flexing depending of the toolbar elements.
(In reply to Richard Marti (:Paenglab) from comment #3)
> Created attachment 8696091 [details] [diff] [review]
> globalSearchBox.patch v2
> 
> The searchbox should look now correct like the FX one. Strange thing is on
> TB the 0.5px box-shadow isn't shown but on FX without problems. I changed it
> now to 1px and it's shown.

(In reply to Richard Marti (:Paenglab) from comment #4)
Maybe there is another half-pixel value floating around somewhere else in the margins/positioning?
The only reason I can think of for half-pixel sizes is for retina screens.

Looks good, anyway :-)

> > Chat tab nit: searchbar width is slightly different from that of the Mail
> > tab.
> 
> I don't see how this could be done. The boxes are flexing depending of the
> toolbar elements.

No problem, it doesn't matter anyway. I just thought I'd mention it.
Comment on attachment 8696091 [details] [diff] [review]
globalSearchBox.patch v2

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

Great, this also streamlines the code!

r+ with nits fixed.

::: mail/themes/osx/mail/searchBox.css
@@ +23,5 @@
>  }
>  
> +#searchInput,
> +#IMSearchInput,
> +#peopleSearchInput {

Please add a brief comment explaining why these are treated differently (or use a self-explanatory additional class name).

Is .remote-gloda-search missing here? (seems to show up in the other groupings of these ids)

@@ +55,5 @@
>  
> +  #searchInput:not([focused]):not(:-moz-window-inactive),
> +  #IMSearchInput:not([focused]):not(:-moz-window-inactive),
> +  #peopleSearchInput:not([focused]):not(:-moz-window-inactive),
> +  .remote-gloda-search:not([focused]):not(:-moz-window-inactive) {

Same here.

@@ +75,5 @@
>  
> +#searchInput:-moz-lwtheme:not([focused]),
> +#IMSearchInput:-moz-lwtheme:not([focused]),
> +#peopleSearchInput:-moz-lwtheme:not([focused]),
> +.remote-gloda-search:-moz-lwtheme:not([focused]) {

Hmm, maybe an additional class would be the way to go? Or is that less performant?

I don't have a strong opinion about this though.
Attachment #8696091 - Flags: review?(aleth)
Attachment #8696091 - Flags: review+
Addressed the review comments.
Attachment #8696091 - Attachment is obsolete: true
Attachment #8696223 - Flags: review+
Blocks: 1230748
Duplicate of this bug: 1213962
Rebased to not need bug 1228074
http://hg.mozilla.org/comm-central/rev/207effb012eb
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0
How does Lightning looks now in SeaMonkey after Lightning style is moved to Thunderbird only css file?
Not tested but it should use the toolkit styles and especially on Windows better fit to their theme.

I hope SM builds tomorrow a nightly on Win and I will test it.
No longer depends on: 1228074
You need to log in before you can comment on or make changes to this bug.