The default bug view has changed. See this FAQ.

Social side bar splitter should be styled like the standard side bar splitter

RESOLVED FIXED in Firefox 17

Status

()

Firefox
SocialAPI
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

unspecified
Firefox 17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Fx16])

Attachments

(4 attachments, 7 obsolete attachments)

(Assignee)

Description

5 years ago
sidebar uses default xul css on the splitter, need to make it appear like the left sidebars.
(Assignee)

Comment 1

5 years ago
we basically need to copy the styles for #devtools-side-splitter on all platforms.
(Assignee)

Comment 2

5 years ago
Created attachment 645599 [details] [diff] [review]
slitter css
Attachment #645599 - Flags: feedback?(jaws)
Comment on attachment 645599 [details] [diff] [review]
slitter css

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

::: browser/themes/winstripe/browser.css
@@ +3422,5 @@
>    max-height: 600px;
>    max-width: 400px;
>  }
>  
> +#social-sidebar-splitter {

Could you add the #social-sidebar-splitter selector to the rule for #devtools-sidebar-splitter for this and the other theme CSS files?
Attachment #645599 - Flags: feedback?(jaws) → feedback+
(Assignee)

Comment 4

5 years ago
Created attachment 645617 [details] [diff] [review]
slitter css
Attachment #645599 - Attachment is obsolete: true
Attachment #645617 - Flags: review?(jaws)
Attachment #645617 - Flags: review?(dao)
Comment on attachment 645617 [details] [diff] [review]
slitter css

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

Still get the review from Dao, but I think this is fine from my point of view.
Attachment #645617 - Flags: review?(jaws) → review+
Comment on attachment 645617 [details] [diff] [review]
slitter css

#devtools-side-splitter uses a color that doesn't make sense here. You're also burying the #social-sidebar-splitter styling deep in devtools style code, which is undesirable.
Attachment #645617 - Flags: review?(dao) → review-
We can instead just add a .sidebar-splitter class in browser.js, and use it for both splitters.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #7)
> We can instead just add a .sidebar-splitter class in browser.js, and use it
> for both splitters.

We couldn't use it for the generic side bar, though, since the positioning is reversed on the left side of the browser.
(Assignee)

Comment 9

5 years ago
Created attachment 646319 [details] [diff] [review]
new splitter css patch

created new class for right side splitter, using devtools css but with corrected colors.  devtools class overrides the color to leave as-is for them.  not using a class on linux as the left sidebar splitter doesn't seem to have any custom css.

I've verified working on osx, but lack of build VMs on travel laptop leaves me unable to verify on other platforms.
Attachment #645617 - Attachment is obsolete: true
Attachment #646319 - Flags: review?(gavin.sharp)
Attachment #646319 - Flags: review?(dao)
Comment on attachment 646319 [details] [diff] [review]
new splitter css patch

>+.sidebar-splitter-right {

This name doesn't make sense in RTL settings.

>+  -moz-border-start: 1px solid #404040;

Where did you get this color value from? Why not use a system color?

You didn't patch gnomestripe. Not sure if this is intentional.

On Windows, the generic side bar uses the default splitter styling except for aero glass and aero basic. Not sure why this splitter should be different.
Attachment #646319 - Flags: review?(dao) → review-
This should get a UI review too.  Shane, it'd be awesome if you could post screenshots and/or screenshare with me when the patches are ready.
(Assignee)

Comment 12

5 years ago
(In reply to Dão Gottwald [:dao] from comment #10)
> Comment on attachment 646319 [details] [diff] [review]
> new splitter css patch
> 
> >+.sidebar-splitter-right {
> 
> This name doesn't make sense in RTL settings.

I'm not clear how RTL will affect this sidebar.  There is the sidebar-splitter class for sidebars appearing on the left, this has tweaks to provide the same appearance of a sidebar on the right.

> >+  -moz-border-start: 1px solid #404040;
> 
> Where did you get this color value from? Why not use a system color?

It matches the left sidebar splitter:

https://mxr.mozilla.org/mozilla-central/source/browser/themes/pinstripe/browser.css#1654

> You didn't patch gnomestripe. Not sure if this is intentional.

There is no sidebar-splitter class for the left, I am assuming that the left uses the default xul styling for splitters.  I'm trying to simply match the right-side sidebar to the left.

> On Windows, the generic side bar uses the default splitter styling except
> for aero glass and aero basic. Not sure why this splitter should be
> different.

I'll change this to do the same.
(Assignee)

Comment 13

5 years ago
(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #11)
> This should get a UI review too.  Shane, it'd be awesome if you could post
> screenshots and/or screenshare with me when the patches are ready.

We're matching the right sidebar splitter to that of the left (splitter for bookmarks and history sidebars) for each platform.  On OSX and aero that is the thin dark line.  Otherwise it is the default splitter style.  Does this need UI review and screenshots?
(Assignee)

Comment 14

5 years ago
Created attachment 647597 [details] [diff] [review]
new splitter css patch

This patch only changes the css for OSX and Aero.  The colors and style match the left sidebar splitter.

I have opted to leave the developer tools splitter as-is since it does not match the splitters on a per-platform basis.

Non-aero and linux are left with the default style for splitters, which is how the left sidebar splitter is styled.

Boriss, do you need pictures, or is matching the left sidebar splitter fine?
Attachment #646319 - Attachment is obsolete: true
Attachment #646319 - Flags: review?(gavin.sharp)
Attachment #647597 - Flags: ui-review?(jboriss)
Attachment #647597 - Flags: review?(gavin.sharp)
Attachment #647597 - Flags: review?(dao)
(Assignee)

Comment 15

5 years ago
Created attachment 647757 [details]
osx splitter
Attachment #647757 - Flags: ui-review?(jboriss)
(Assignee)

Comment 16

5 years ago
Created attachment 647758 [details]
windows splitter
Attachment #647758 - Flags: ui-review?(jboriss)
(Assignee)

Updated

5 years ago
Group: mozilla-corporation-confidential
(Assignee)

Comment 17

5 years ago
posted the wrong pic, so made this confidential
(Assignee)

Comment 18

5 years ago
Created attachment 647762 [details]
linux default splitter
The content of attachment 647757 [details] has been deleted by
    Dave Miller [:justdave] <justdave@mozilla.com>
who provided the following reason:

Uploader requested deletion of unsanitized data

The token used to delete this attachment was generated at 2012-07-31 16:43:02 PDT.
Group: mozilla-corporation-confidential
(Assignee)

Comment 20

5 years ago
Created attachment 647768 [details]
osx splitter
Attachment #647768 - Flags: ui-review?(jboriss)
(Assignee)

Comment 21

5 years ago
Created attachment 647769 [details] [diff] [review]
new splitter css patch

adding 3px width to make grabbing easier, matches what is done on windows
Attachment #647597 - Attachment is obsolete: true
Attachment #647597 - Flags: ui-review?(jboriss)
Attachment #647597 - Flags: review?(gavin.sharp)
Attachment #647597 - Flags: review?(dao)
Attachment #647769 - Flags: ui-review?(jboriss)
Attachment #647769 - Flags: review?(gavin.sharp)
Attachment #647769 - Flags: review?(dao)
Comment on attachment 647769 [details] [diff] [review]
new splitter css patch

You can use ".sidebar-splitter" and "#appcontent ~ .sidebar-splitter" to discern left and right splitters.
Attachment #647769 - Flags: review?(dao) → review-
(Assignee)

Comment 23

5 years ago
Created attachment 647779 [details] [diff] [review]
new splitter css patch

cool, updated
Attachment #647769 - Attachment is obsolete: true
Attachment #647769 - Flags: ui-review?(jboriss)
Attachment #647769 - Flags: review?(gavin.sharp)
Attachment #647779 - Flags: review?(dao)
Comment on attachment 647779 [details] [diff] [review]
new splitter css patch

>+  .sidebar-splitter {
>     border: 0;
>+    -moz-border-start: none;
>     -moz-border-end: 1px solid #A9B7C9;
>     min-width: 0;
>     width: 3px;
>     background-color: transparent;
>     -moz-margin-start: -3px;
>+    -moz-margin-end: 0;
>     position: relative;
>   }

What's the point of these added lines?
(Assignee)

Comment 25

5 years ago
Created attachment 647789 [details] [diff] [review]
new splitter css patch

removed extra unnecessary css
Attachment #647779 - Attachment is obsolete: true
Attachment #647779 - Flags: review?(dao)
Attachment #647789 - Flags: review?(dao)

Updated

5 years ago
Attachment #647789 - Flags: review?(dao) → review+

Updated

5 years ago
OS: Mac OS X → All
Hardware: x86 → All
Summary: fix sidebar splitter style → Social side bar should be styled like the standard browser side bar
Attachment #647758 - Flags: ui-review?(jboriss) → ui-review+
Comment on attachment 647768 [details]
osx splitter

+'d, and thank you for attaching screenshots!
Attachment #647768 - Flags: ui-review?(jboriss) → ui-review+
Attachment #647762 - Flags: ui-review+
(Assignee)

Comment 27

5 years ago
passing to gavin for landing
Assignee: mixedpuppy → gavin.sharp

Updated

5 years ago
Summary: Social side bar should be styled like the standard browser side bar → Social side bar splitter should be styled like the standard side bar splitter

Updated

5 years ago
Attachment #647789 - Attachment is patch: true

Updated

5 years ago
Assignee: gavin.sharp → mixedpuppy
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c835504961b
Keywords: checkin-needed
Target Milestone: --- → Firefox 17
https://hg.mozilla.org/mozilla-central/rev/9c835504961b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.