Closed
Bug 929127
Opened 11 years ago
Closed 11 years ago
Update the inspector sidebar tabs design according to shorlander's mockups
Categories
(DevTools :: Inspector, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 28
People
(Reporter: me, Assigned: me)
References
Details
(Whiteboard: [lang=css][mentor=paul][good first verify])
Attachments
(3 files, 15 obsolete files)
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130911155223
Assignee | ||
Updated•11 years ago
|
Blocks: 916766
Component: Untriaged → Developer Tools: Framework
OS: Linux → All
Hardware: x86_64 → x86
Assignee | ||
Updated•11 years ago
|
Component: Developer Tools: Framework → Developer Tools: Inspector
Whiteboard: [lang=css][mentor=paul]
Updated•11 years ago
|
Summary: Update the inspector tabs design according to shorlander's mockups → Update the inspector sidebar tabs design according to shorlander's mockups
Updated•11 years ago
|
Assignee: nobody → aljullu
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 1•11 years ago
|
||
Hi, I am new to Bugzilla and am interested in fixing this as my first bug. Please could you assign this to me and point my in the right direction.
Comment 2•11 years ago
|
||
Liam, Albert is already looking at this. Sorry.
Assignee | ||
Comment 3•11 years ago
|
||
First attempt. This patch is only for Linux.
I took the colors from bug 916766 comment 8 and 9 but I think it does not look like the mockup. Is there something I am missing?
In addition, I don't know the colors for :hover and :active, I kept the same colors only removing the gradients.
Waiting for your feedback.
Attachment #823519 -
Flags: feedback?(paul)
Assignee | ||
Comment 4•11 years ago
|
||
Comment 5•11 years ago
|
||
(In reply to Albert Juhé from comment #3)
> Created attachment 823519 [details] [diff] [review]
> Bug 929127 - Update the inspector sidebar tabs design according to
> shorlander's mockups; r=paul
>
> First attempt. This patch is only for Linux.
>
> I took the colors from bug 916766 comment 8 and 9 but I think it does not
> look like the mockup. Is there something I am missing?
Which colors from the palette did you use? You didn't find anything close to the mockup?
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Paul Rouget [:paul] from comment #5)
>
> Which colors from the palette did you use? You didn't find anything close to
> the mockup?
I took this colors:
Background tab bar: #252c33 (from bug 916766 comment 9)
Color tab bar: #f5f7fa (from bug 916766 comment 9)
Separator: hsl(212,8%,38%) (I used a color picker)
Background tab selected: #1d4f73 (from bug 916766 comment 9)
Background tab selected (hover): no change
Background tab selected (active): no change
For what you say, I understand I should ignore the comment in bug 916766 and use only the palette and the mockup, so the colors would be:
Background tab bar: hsl(211,24%,27%) (from palette)
Color tab bar: hsl(210,30%,85%) (no change)
Separator: hsl(213,4%,74%) (from palette)
Background tab selected: hsl(205,74%,45%) (from palette)
Background tab selected (hover): no change
Background tab selected (active): no change
I will try to build a patch today or tomorrow with the new colors.
Comment 7•11 years ago
|
||
(In reply to Albert Juhé from comment #6)
> For what you say, I understand I should ignore the comment in bug 916766 and
> use only the palette and the mockup, so the colors would be:
To be honest, I don't know. I'll make sure we have a static list of colors asap. Sorry :/
Comment 8•11 years ago
|
||
Can you use this: https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors
You should use the "Toolbars" color. The "Tab Toolbar" is for the top level tabs.
Assignee | ||
Comment 9•11 years ago
|
||
New patch with the new colors.
I also changed the background-position of the tabs (I think in my last patch it was not correctly set), however, I couldn't test it on rtl.
Attachment #823519 -
Attachment is obsolete: true
Attachment #823519 -
Flags: feedback?(paul)
Attachment #824159 -
Flags: feedback?(paul)
Assignee | ||
Comment 10•11 years ago
|
||
Attachment #823521 -
Attachment is obsolete: true
Assignee | ||
Comment 11•11 years ago
|
||
There are two things I would like to mention:
1) I took the separator color from the palette, but I think none of them are the same than in the mockup. IMO in my first patch they looked better so, if you agree, I will recover that color.
2) I didn't change the colors for :hover and :active states of the open tab but I think they don't look good with the new default color. Shall we change it?
Updated•11 years ago
|
Flags: needinfo?(paul)
Comment 12•11 years ago
|
||
Albert, the background color is right. It looks different because the main tabs toolbar (above) doesn't use the new color. Also, the separator color looks off as well.
Can you true to also change the color of the inspector toolbar (use the "Tab Toolbar" color)? And maybe the main tabs toolbar too (use the same color as the inspector tabs).
Flags: needinfo?(paul)
Assignee | ||
Comment 13•11 years ago
|
||
Ok, I attached the new patch.
I think it should be the other way around: "Tab toolbar" color for the main tabs toolbar and "Toolbars" color for the inspector toolbar, shouldn't it?
Attachment #824159 -
Attachment is obsolete: true
Attachment #824159 -
Flags: feedback?(paul)
Attachment #824714 -
Flags: feedback?(paul)
Assignee | ||
Comment 14•11 years ago
|
||
Attachment #824160 -
Attachment is obsolete: true
Comment 15•11 years ago
|
||
(In reply to Albert Juhé from comment #14)
> Created attachment 824715 [details]
> Screenshot patch 3.png
Ok - we need to make sure we talk about the same things.
In your screenshot, there are 3 parts:
- the top tabs, where you see Console, Inspector, … we call that toolbox tabs
- the inspector toolbar (the bar the includes the "inspect" button, the breadcrumbs and the search input)
- the inspector tabs (Rules, Computed, …)
In the inspector toolbar AND the inspector tabs, the background should be the color "Toolbars".
Don't change the code of the toolbox tabs.
Changing the background color for the inspector toolbar and the inspector tabs might impact the other tools, and I think it's alright.
Assignee | ||
Comment 16•11 years ago
|
||
Wow, sorry for misunderstanding!
I attached a new patch, it is like patch 2 but with the new inspector toolbar color.
Attachment #824714 -
Attachment is obsolete: true
Attachment #824714 -
Flags: feedback?(paul)
Attachment #824789 -
Flags: feedback?(paul)
Assignee | ||
Comment 17•11 years ago
|
||
Attachment #824715 -
Attachment is obsolete: true
Comment 18•11 years ago
|
||
That looks better. I think the separator color is wrong. Can you try to find a better color from https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors ?
Assignee | ||
Comment 19•11 years ago
|
||
I made tests with the four more similar colors from the wiki:
Chrome
Foreground (Text) - Grey: #b6babf ; rgba(182, 186, 191, 1)
Code
Content (Text) - Light: #b8c8d9 ; rgba(184, 200, 217, 1)
Content (Text) - Grey: #8fa1b2 ; rgba(143, 161, 178, 1)
Content (Text) - Dark Grey: #5f7387 ; rgba(95, 115, 135, 1)
But I am not sure which one to choose. Could you give me a hand?
Flags: needinfo?(paul)
Comment 20•11 years ago
|
||
Stephen, can you tell us what is the right color for the tab separators in the inspector tabs? If it's not already mentioned in https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors - can you please add it?
Flags: needinfo?(shorlander)
Updated•11 years ago
|
Flags: needinfo?(paul)
Assignee | ||
Comment 21•11 years ago
|
||
It would be good if we could also get the colors for the :hover and :active state in tabs. Specially for the open tab, given that the previous blue for the normal state and the new blue for the :hover state are quite similar.
Comment 22•11 years ago
|
||
To be honest, I like the current tab design better.
Comment 23•11 years ago
|
||
(In reply to Paul Rouget [:paul] from comment #20)
> Stephen, can you tell us what is the right color for the tab separators in
> the inspector tabs? If it's not already mentioned in
> https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors - can you
> please add it?
The color on the selected separator is #2d5b7d
The color for the regular grey separators is #5a6169
Or we could try and find a light color with an opacity setting that would work for both.
Flags: needinfo?(shorlander)
Assignee | ||
Comment 24•11 years ago
|
||
Here comes the patch with the new colors.
> Or we could try and find a light color with an opacity setting that would work for both.
I don't know how to approach this. Any help? Otherwise, can we left it with hard-coded colors?
Attachment #824789 -
Attachment is obsolete: true
Attachment #824789 -
Flags: feedback?(paul)
Attachment #826910 -
Flags: review?(paul)
Assignee | ||
Comment 25•11 years ago
|
||
Attachment #824791 -
Attachment is obsolete: true
Attachment #824840 -
Attachment is obsolete: true
Comment 26•11 years ago
|
||
Comment on attachment 826910 [details] [diff] [review]
Patch 5
Don't use the `background` shorthand, use `background-color`.
The left toolbar has an inner shadow that makes it stand out: see the light inner border in the left section here: http://i.imgur.com/LgZUAkm.png - you might want to remove it (not sure if that means removing the show box-shadow or just a section of it).
This is only for Linux. Can you do the same modifications for the 3 platforms?
Attachment #826910 -
Flags: review?(paul)
Assignee | ||
Comment 27•11 years ago
|
||
Ok, I changed 'background' for 'background-color', removed the box-shadow and made the same changes to osx and windows. However, I couldn't test it on these platforms.
Attachment #826910 -
Attachment is obsolete: true
Attachment #828161 -
Flags: review?(paul)
Assignee | ||
Comment 28•11 years ago
|
||
Attachment #826911 -
Attachment is obsolete: true
Comment 29•11 years ago
|
||
Comment on attachment 828161 [details] [diff] [review]
Patch 6
> %filter substitution
>-%define smallSeparator linear-gradient(hsla(204,45%,98%,0), hsla(204,45%,98%,.1), hsla(204,45%,98%,0)), linear-gradient(hsla(206,37%,4%,0), hsla(206,37%,4%,.6), hsla(206,37%,4%,0)), linear-gradient(hsla(204,45%,98%,0), hsla(204,45%,98%,.1), hsla(204,45%,98%,0))
>-%define solidSeparator linear-gradient(transparent, transparent), linear-gradient(hsla(206,37%,4%,.6), hsla(206,37%,4%,.7)), linear-gradient(hsla(204,45%,98%,.1), hsla(204,45%,98%,.1))
>+%define smallSeparator linear-gradient(#343c45 15%, #5a6169 15%, #5a6169 85%, #343c45 85%)
>+%define solidSeparator linear-gradient(#2d5b7d, #2d5b7d)
Don't reuse #343c45 here, use transparent.
Do we still need to use 2 background-images for the tabs?
Can we use a background-color for the background and a background-image for the border?
Attachment #828161 -
Flags: review?(paul)
Assignee | ||
Comment 30•11 years ago
|
||
>Don't reuse #343c45 here, use transparent.
I used #343c45 instead of transparent because otherwise when hovering a tab it showed the dark color behind the separator (see right separator in the attached image).
>Do we still need to use 2 background-images for the tabs?
>Can we use a background-color for the background and a background-image for the border?
I could never use background-image with gradient in Firefox. I think it's caused by bug 709587.
I created an example in jsfiddle: http://jsfiddle.net/FA9B5/2/ In Chrome it looks ok but in Firefox doesn't.
Comment 31•11 years ago
|
||
(In reply to Albert Juhé from comment #30)
> Created attachment 829436 [details]
> Transparent background separators example.png
>
> >Don't reuse #343c45 here, use transparent.
> I used #343c45 instead of transparent because otherwise when hovering a tab
> it showed the dark color behind the separator (see right separator in the
> attached image).
It's because the background linear gradient goes that far. Stop it before. This should work:
+ background-size: 100% calc(100% - 1px), 1px 100%;
>
> >Do we still need to use 2 background-images for the tabs?
> >Can we use a background-color for the background and a background-image for the border?
What I said above would be impossible if we'd use background-color. So forget about that.
> I could never use background-image with gradient in Firefox. I think it's
> caused by bug 709587.
>
> I created an example in jsfiddle: http://jsfiddle.net/FA9B5/2/ In Chrome it
> looks ok but in Firefox doesn't.
border-image and background-image are different beasts.
---
r+ if we you can use transparent instead of #343c45 (or convince me it's not possible :p).
Assignee | ||
Comment 32•11 years ago
|
||
Wow, sorry, I misunderstood you! I've attached a new patch with your proposal:
+ background-size: 100% calc(100% - 1px), 1px 100%;
I had a problem and I am not sure if I solved it properly. Before creating the patch I cloned the code from mozilla-central and I saw that the OSX common.css file I was modifying was a little bit different. Is this one:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/devtools/common.css#21
From when I started working on this patch, these lines have been removed:
-color: hsl(210,30%,85%);
-background-image: url(background-noise-toolbar.png), linear-gradient(#3e4750, #3e4750);
-box-shadow: 0 1px 0 hsla(204,45%,98%,.05) inset, -1px 0 0 hsla(204,45%,98%,.05) inset, 0 -1px 0 hsla(204,45%,98%,.05) inset;
I added back:
+color: hsl(210,30%,85%);
+background-color: #343c45;
I don't have a Mac so I can't test if it's ok.
Attachment #828161 -
Attachment is obsolete: true
Attachment #829436 -
Attachment is obsolete: true
Attachment #8335464 -
Flags: review?(paul)
Flags: needinfo?(paul)
Assignee | ||
Updated•11 years ago
|
Attachment #8335464 -
Attachment is obsolete: true
Attachment #8335464 -
Flags: review?(paul)
Assignee | ||
Comment 33•11 years ago
|
||
Sorry, I uploaded the wrong patch.
In this one I set:
background-size: calc(100% - 2px) 100%, 1px 100%;
Given that background-position: 1px, 0; so we must deduct 2px (one for the separator and another for the initial position).
Attachment #8335477 -
Flags: review?(paul)
Assignee | ||
Comment 34•11 years ago
|
||
Attachment #828162 -
Attachment is obsolete: true
Comment 35•11 years ago
|
||
Comment on attachment 8335477 [details] [diff] [review]
Patch 8
Thank you Albert! Now you can add "checkin-needed" in the keywords field.
Attachment #8335477 -
Flags: review?(paul) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 36•11 years ago
|
||
Keywords: checkin-needed
Whiteboard: [lang=css][mentor=paul] → [lang=css][mentor=paul][fixed-in-fx-team]
Comment 37•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [lang=css][mentor=paul][fixed-in-fx-team] → [lang=css][mentor=paul]
Target Milestone: --- → Firefox 28
Updated•11 years ago
|
Flags: needinfo?(paul)
Updated•11 years ago
|
Whiteboard: [lang=css][mentor=paul] → [lang=css][mentor=paul][good first verify]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•