Closed Bug 973191 Opened 6 years ago Closed 6 years ago

DevTools themes - In sidebar dock mode, some splitters are dark in light theme

Categories

(DevTools :: Framework, defect)

30 Branch
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 30

People

(Reporter: ntim, Assigned: bgrins)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140213030201
The affected tools are :
- Inspector 
- Style Editor (you should make the toolbox small to see the issue)
- Network monitor
- Console
Blocks: 966661
Flags: needinfo?(bgrinstead)
Component: Untriaged → Developer Tools: Framework
OS: Windows 8.1 → All
Hardware: x86_64 → All
This seems to be an issue only when the toolbox is narrow enough to force vertical layout.
Assignee: nobody → bgrinstead
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(bgrinstead)
I've moved the responsive styling into widgets.inc.css and away from common.css (as in Bug 957663).  Also setting the border colors in each theme next to the other splitter colors instead of using black in both cases.

The problem can be seen when shrinking the toolbox horizontal size until it forces vertical layout.  In the light theme the splitter will be black in this case (it should be #aaa).  With this patch applied, the color should be correct.
Attachment #8380663 - Flags: review?(vporof)
Comment on attachment 8380663 [details] [diff] [review]
side-splitter-responsive.patch

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

::: browser/themes/shared/devtools/widgets.inc.css
@@ +15,5 @@
>  .generic-toggled-side-pane[animated] {
>    transition: margin 0.25s ease-in-out;
>  }
>  
> +/* Responsive container */

I assume moving this is based on the fact that common.css is included in browser, and we don't need it there.
Attachment #8380663 - Flags: review?(vporof) → review+
(In reply to Victor Porof [:vporof][:vp] from comment #4)
> Comment on attachment 8380663 [details] [diff] [review]
> side-splitter-responsive.patch
> 
> Review of attachment 8380663 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/shared/devtools/widgets.inc.css
> @@ +15,5 @@
> >  .generic-toggled-side-pane[animated] {
> >    transition: margin 0.25s ease-in-out;
> >  }
> >  
> > +/* Responsive container */
> 
> I assume moving this is based on the fact that common.css is included in
> browser, and we don't need it there.

Yes, exactly.  The devtools-responsive-container is only loaded inside some of the tools xul, never used inside of the browser.xul: https://mxr.mozilla.org/mozilla-central/search?find=%2Fbrowser%2F&string=devtools-responsive-container.
Brian, thanks for the fix ! Make sure style editor is also fixed, because it uses a different splitter (thicker).
https://hg.mozilla.org/mozilla-central/rev/7fd003a98b55
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 30
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.