Closed Bug 680256 Opened 13 years ago Closed 8 years ago

Update Sidebar styles including Lion and Yosemite+ specific styles

Categories

(Firefox :: Theme, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: shorlander, Assigned: shorlander)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

Attached image Lion Sidebar Style Design Spec (obsolete) —
Update the sidebar styles in Lion to include:

- Background color gradient for Active/Inactive window
- Selected item color gradients for Active/Inactive window
- Lighter separator color
- Matching search field
- Inverted disclosure triangle icons and item icons
Summary: Update Sidebar Styles for Lion → Update Sidebar styles including Lion specific styles
Attached patch Update Sidebar Styling - 01 (obsolete) — Splinter Review
Updates the OSX sidebar styling with:

- More accurate Snow Leopard colors
- New Lion specific colors
- Contextually styled search field
- Sidebar specific twisty and inverted selected twisty
Attachment #555128 - Flags: review?(dao)
Attached image Screenshot - 01 (obsolete) —
Comment on attachment 555128 [details] [diff] [review]
Update Sidebar Styling - 01

>+#search-box {
>+  -moz-appearance: none;
>+  margin: 0;
>+  padding: 2px 5px 1px;
>+  border: 1px solid;
>+  border-color: #a2a9b1 #abb2ba #c3c8d0;
>+  border-radius: 10000px;
>+  box-shadow: inset 0 1px 2px hsla(214,32%,15%,.1), 
>+              inset 0 0 1px hsla(214,32%,15%,.05), 
>+                    0 1px hsla(0,0%,100%,.25);
>+  background: -moz-linear-gradient(#eef0f4, #f5f7fa);
>+  background-clip: padding-box;
>+}
>+
>+#search-box:-moz-window-inactive {
>+  border-color: #b6b6b6 #bfbfbf #d6d6d6;
>+  box-shadow: inset 0 1px 2px hsla(0,0%,0%,.1), 
>+              inset 0 0 1px hsla(0,0%,0%,.05), 
>+                    0 1px hsla(0,0%,100%,.25);
>+  background: -moz-linear-gradient(#f3f3f3, #fafafa);
>+}
>+
>+#search-box[focused] {
>+  box-shadow: inset 0 0 1px -moz-mac-focusring, 
>+              0 0 3px -moz-mac-focusring, 
>+              0 0 1.5px -moz-mac-focusring, 
>+              inset 0 1px 2px hsla(214,32%,15%,.1);
>+  border-color: -moz-mac-focusring;
>+}
>+
>+#search-box > .textbox-input-box {
>+  -moz-padding-start: 15px;
>+  background: url("chrome://global/skin/icons/search-textbox-sidebar.png") left no-repeat;
>+}
>+
>+#search-box > .textbox-input-box:-moz-locale-dir(rtl) {
>+  background-position: right;
>+}

Shouldn't widget code take care of this? Is bug 682541 incomplete?
(In reply to Dão Gottwald [:dao] from comment #3)
> Shouldn't widget code take care of this? Is bug 682541 incomplete?

That code styles the searchfield contextually for the sidebar. The native widget code only supplies a white version.
Could this land?
Review Ping.
Comment on attachment 555128 [details] [diff] [review]
Update Sidebar Styling - 01

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

r=me with the following changes made.

::: browser/themes/pinstripe/browser/browser.css
@@ +1488,5 @@
> +  #sidebar-box {
> +    box-shadow: inset 0 1px 0 hsla(214,32%,15%,.1),
> +                inset -1px 0 0 hsla(0,0%,100%,.2);
> +    background-color: hsl(212,19%,85%);
> +    background-image: -moz-linear-gradient(hsl(213,26%,93%), hsl(212,19%,85%));

Since this patch was written, linear-gradient has been unprefixed. Can you please use the unprefixed version here and elsewhere through the patch?

@@ +1490,5 @@
> +                inset -1px 0 0 hsla(0,0%,100%,.2);
> +    background-color: hsl(212,19%,85%);
> +    background-image: -moz-linear-gradient(hsl(213,26%,93%), hsl(212,19%,85%));
> +  }
> +  

nit: please remove these extras whitespace here and other places throughout this patch.
Attachment #555128 - Flags: review?(dao) → review+
(In reply to Stephen Horlander from comment #4)
> (In reply to Dão Gottwald [:dao] from comment #3)
> > Shouldn't widget code take care of this? Is bug 682541 incomplete?
> 
> That code styles the searchfield contextually for the sidebar. The native
> widget code only supplies a white version.

I don't see anything in the screenshot that would require custom contextual styling, i.e. something that couldn't be achieved with shades of white and black and transparency. Native theming should be able to provide this, assuming this style is a standard OS X thing.
What's the next step here?
Flags: needinfo?(dao)
(In reply to Jared Wein [:jaws] from comment #9)
> What's the next step here?

I need to update my patch :)
Somebody should investigate / poke the right people to investigate what (if anything) is needed to get the "contextual" search field styling through native widget rendering.
Flags: needinfo?(dao)
(In reply to Dão Gottwald [:dao] from comment #11)
> Somebody should investigate / poke the right people to investigate what (if
> anything) is needed to get the "contextual" search field styling through
> native widget rendering.

If there was a way to render the searchfield natively with a style that looks good in sidebars, how do you envision the CSS to look? Should the theme rendering detect automatically that the searchfield is in a sidebar, or should the CSS do something like -moz-appearance: searchfield-in-sidebar? Or should an attribute be set on the searchfield element which the native theme rendering looks at?
As I understand it, the "contextual" nature isn't sidebar-specific, so I imagine this would be a separate -moz-appearance value such as "contextual-searchfield" or whatever name makes more sense.
I understood "contextual" as "in context of a sidebar, blue background and all". Stephen, is that what you meant?
If there's some transparency allowing the background to shine through, it doesn't really matter whether it's blue or some other random color.
So the question is: If we had the searchfield style from this patch as a general style, would we use it anywhere else besides sidebars?
(In reply to Markus Stange from comment #14)
> I understood "contextual" as "in context of a sidebar, blue background and
> all". Stephen, is that what you meant?

Yes, that is what I meant. 

(In reply to Markus Stange from comment #16)
> So the question is: If we had the searchfield style from this patch as a
> general style, would we use it anywhere else besides sidebars?

We have the field in the add-ons manager. Might be other places to use it but I can't think of any off the top of my head. However I didn't use a generic one-size-fits-all solution because I wanted specific values and using translucent white and black won't give that.
#searchbox in browser/themes/pinstripe/tabview/tabview.css might be another candidate.

(In reply to Stephen Horlander from comment #17)
> using translucent white and black won't give that.

With the right brightness and transparency values, it should give you exactly that. I don't see why it wouldn't.
(In reply to Dão Gottwald [:dao] from comment #18)
> #searchbox in browser/themes/pinstripe/tabview/tabview.css might be another
> candidate.
> 
> (In reply to Stephen Horlander from comment #17)
> > using translucent white and black won't give that.
> 
> With the right brightness and transparency values, it should give you
> exactly that. I don't see why it wouldn't.

AFAIK we just do straight alpha blending and the more opaque the overlaying color the more it pushes towards that color. In this case the borders are pretty dark so at the right translucency it just gets mostly dull gray. If we had some kind of blending mode like multiply or screen it might work.

In this case it's the difference between the left and right seen here (blue hued color left and black/white on the right): http://cl.ly/image/0B1S090e2t0L
Ok, so what does OS X do? I'm still making wild guesses here as to whether this could be standard OS X widget. Do any applications have such search fields? Do the backgrounds vary?
(In reply to Dão Gottwald [:dao] from comment #20)
> Ok, so what does OS X do? I'm still making wild guesses here as to whether
> this could be standard OS X widget. Do any applications have such search
> fields? Do the backgrounds vary?

Depends on the application. Some examples of contextual search fields: http://cl.ly/image/24070g3j3B45

The backgrounds and style vary, but it is almost entirely on a case by case basis and not a common control.
Ok, so I guess we can't pull this from the OS, but we should still share it internally, possibly via shared.inc.
Blocks: 856307
Attached patch OS X Sidebar Updates - 01 (obsolete) — Splinter Review
This never got completed so I updated it for Yosemite+

The updated patch does a few things:
- Makes the base style the Lion style
- Adds media-queries for styling Yosemite+
- Adds an SVG disclosure/twisty icon
Attachment #554209 - Attachment is obsolete: true
Attachment #555128 - Attachment is obsolete: true
Attachment #555129 - Attachment is obsolete: true
Attachment #8731894 - Flags: review?(jaws)
(In reply to Stephen Horlander [:shorlander] from comment #23)
> Created attachment 8731894 [details] [diff] [review]
> OS X Sidebar Updates - 01
> 
> This never got completed so I updated it for Yosemite+
> 
> The updated patch does a few things:
> - Makes the base style the Lion style
> - Adds media-queries for styling Yosemite+
> - Adds an SVG disclosure/twisty icon

Also:
- Increases the height and font-size of the list items to increase legibility and hit target
Attached image Screenshot on 10.11
Comment on attachment 8731894 [details] [diff] [review]
OS X Sidebar Updates - 01

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

I am unable to test the patch but the changes look OK to me.

::: browser/themes/osx/places/places.css
@@ +152,5 @@
>    min-width: 0px;
>    min-height: 0px;
> +  border: 1px solid #a2a9b1;
> +  border-radius: 10px;
> +  background: linear-gradient(hsla(0,0%,100%,.75),hsla(0,0%,100%,.1));

Please use background-image here and below in the #viewButton:hover:active and #viewButton:-moz-window-inactive rules. When you switch this to background-image, make sure that we weren't depending on the background short-hand clearing any other background-related properties.

::: browser/themes/osx/syncedtabs/sidebar.css
@@ +26,5 @@
>    font-weight: bold;
>  }
>  
>  .item.selected > .item-title-container {
> +  background: @sidebarItemBackground@;

Ditto for the background here and in the multiple other places within this file.

::: toolkit/themes/osx/global/shared.inc
@@ +14,5 @@
>  %define scopeBarTitleColor #6D6D6D
>  
> +%define sidebarItemBorderTop 1px solid #bcc5d5
> +%define sidebarItemBorderBottom 1px solid #94a1b9
> +%define sidebarItemBackground linear-gradient(#c3ccdf 1px, #bdc7db 2px, #9dabc5) repeat-x

Please remove the repeat-x in this file and update the linear-gradients to be `linear-gradient(to bottom, ..., ...)`
Attachment #8731894 - Flags: review?(jaws) → review+
Summary: Update Sidebar styles including Lion specific styles → Update Sidebar styles including Lion and Yosemite+ specific styles
Assignee: nobody → shorlander
Status: NEW → ASSIGNED
On your screenshot, the word "Search" in the search box in the sidebar looks to have it's vertical-alignment too low. Can you fix this before checking it in?
Flags: needinfo?(shorlander)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #27)
> On your screenshot, the word "Search" in the search box in the sidebar looks
> to have it's vertical-alignment too low. Can you fix this before checking it
> in?

This looks to be some kind of local bug because release doesn't have the placeholder label at all. Probably due to using the latest OS SDK. I think we are using an older SDK for our builds?
Flags: needinfo?(shorlander)
Yeah, this is caused by the different SDK version.
Attachment #8731894 - Attachment is obsolete: true
Attachment #8734859 - Flags: review?(jaws)
Attachment #8734859 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/196a2b799ea0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
(In reply to Stephen Horlander [:shorlander] from comment #24)
> - Increases the height and font-size of the list items to increase
> legibility and hit target

Can I ask from where that came from? (especially Library looks awkward with it)
(In reply to Stefan Plewako [:stef] from comment #33)
> (In reply to Stephen Horlander [:shorlander] from comment #24)
> > - Increases the height and font-size of the list items to increase
> > legibility and hit target
> 
> Can I ask from where that came from? (especially Library looks awkward with
> it)

The rationale is just what you quoted. The old item height was pretty tight. Coincidentally it is also closer to sidebar item size in system applications (e.g. Finder or Mail).
(In reply to Stephen Horlander [:shorlander] from comment #34)
> The rationale is just what you quoted. The old item height was pretty tight.
> Coincidentally it is also closer to sidebar item size in system applications
> (e.g. Finder or Mail).

I'm not sure if legibility and hit target was a problem, how updated values have been chosen or if other sizes in a window were taken into account but OS X sidebar size is configurable for a good reason, maybe we could respect it?
Depends on: 1297485
See Also: 1297485
Depends on: 1309856
You need to log in before you can comment on or make changes to this bug.