927815 - Update the Infobar design according to shorlander's mockups

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Paul Rouget [:paul]]

Mockups:

http://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/[e-mail]
http://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/[e-mail]

Pallets:
http://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/[e-mail]
http://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/[e-mail]

Comment 1

[Paul Rouget [:paul]]

Attachment: Screenshot of the current infobar

Comment 2

[Albert Juhé]

Hi Paul, I come from Bug 709740. Yes, I would like to contribute to this bug. It's going to be my first attempt to work on Firefox's code so I would need a little bit of help, if it is possible.

I have already downloaded the mozilla-central code, compiled it in my computer and made some small tests to try.

Regarding to this bug, could you mentor me? I suppose I will have to modify a CSS file (given that I have to change the colors). I checked on /browser/devtools/inspector but there is only one CSS file called inspector.css and it doesn't seem to be where the colors are defined.

Could you give me a hand? Where should I search? Thanks in advance!

Comment 3

[Paul Rouget [:paul]]

Albert,

the 2 files you want to look at are:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/highlighter.css#33
http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/highlighter.inc.css#20

The first one is about layout, the second one about the style.

I believe you'll only need to touch the second one.

Come by IRC (#devtools) and ping me (paul) if you have any question.

Thanks!

Comment 4

[Albert Juhé]

Thank you Paul, now it seems quite straightforward. I have only one problem remaining: in the mockups the colors are different depending on the theme: in the dark one they are lighter than in the light theme, but in the CSS file I can't modify them separately. Do you know if there is some parent class or something similar I can use to differentiate them?

And one last thing, I am not sure which blue in the mockups corresponds with which blue in the pallet. I think it is the most bottom left one, but I prefer to ask. :P

Comment 5

[Paul Rouget [:paul]]

You should use the pref `devtools.theme` to check what theme is used.

But for now, I'd recommend that you start with the dark theme. We'll implement the light theme later.

Comment 6

[Albert Juhé]

Attachment: Dark theme color changes

First attempt. It changes the colors of the infobar according to the dark theme mockups.

Comment 7

[Albert Juhé]

Attachment: darkinfobardesign.patch

Changed background and removed shadow. I still have to make the separators shorter because now they occupy all the height.

Comment 8

[Paul Rouget [:paul]]

Comment on attachment 818708 [details] [diff] [review]
darkinfobardesign.patch

Looks good.

You removed the highlighter-outline. You should keep this (when I mentioned removing the border, I meant removing the border of the infobar).

You'll also need to remove the separators around the the inspect icon and the dropdown menu.

I think the triangle below the infobar still has borders.

In the next version of your patch, can you combined the 2 patches into one file?
Also, can you attach a screenshot as well?

Thank you!

Comment 9

[Paul Rouget [:paul]]

We also need the updated icons (hidpi). Shorlander, can you take care of that?

Comment 10

[Albert Juhé]

Attachment: Attemp 3: new infobar dark theme

I added back the shadow and removed arrow borders and separators. I also combined all patches in one.

Comment 11

[Albert Juhé]

Attachment: Attemp 3: screenshot

Comment 12

[Paul Rouget [:paul]]

Comment on attachment 819362 [details] [diff] [review]
Attemp 3: new infobar dark theme

># HG changeset patch
># Parent 28fd40f6f59af594399237a7a955919482380be2
># User Albert Juhe <[e-mail]>
>New dark infobar design.
>
>
>diff --git a/browser/themes/shared/devtools/highlighter.inc.css b/browser/themes/shared/devtools/highlighter.inc.css
>--- a/browser/themes/shared/devtools/highlighter.inc.css
>+++ b/browser/themes/shared/devtools/highlighter.inc.css
>@@ -16,114 +16,95 @@
>   box-shadow: 0 0 0 1px rgba(0,0,0,0.3);
>   outline-color: rgba(255,255,255,0.7);
> }
> 
> /* Highlighter - Node Infobar */
> 
> .highlighter-nodeinfobar {
>   color: hsl(200, 100%, 65%);
>-  border: 1px solid hsla(210, 19%, 63%, .5);
>   border-radius: 3px;
>-  background: linear-gradient(hsl(209, 18%, 30%), hsl(210, 24%, 16%)) no-repeat padding-box;
>+  background: hsl(210, 2%, 22%) no-repeat padding-box;

background-color: hsl(210,2%,22%);

Even though we don't correctly do that in this file, color values should not include spaces before commas.

The colors you use should come from the pallet:
http://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/[e-mail]

Not directly from the mockup.

I think the correct colors are:
background: hsl(214,13%,24%)
tag: hsl(352,79%,62%)
id: hsl(103,46%,54%)
class: hsl(200,74%,57%)
(I'm not 100% sure, don't trust me on this. Check by yourself in the palette, and see what colors are closer to the mockup)

Should we have different colors for classes and pseudo classes?

Comment 13

[Albert Juhé]

Attachment: Attemp 4: new infobar dark theme

That's true, I had a problem with the colors. I verified and copied your values except tag, which I think is purple instead of ping. I also removed the white-spaces as suggested.

Thanks for your reviews!

Comment 14

[Albert Juhé]

Attachment: Attemp 4: screenshot

Comment 15

[Paul Rouget [:paul]]

Comment on attachment 819809 [details] [diff] [review]
Attemp 4: new infobar dark theme

># HG changeset patch
># Parent acaef53e42643889992e85e6573e6867d97c86c7
># User Albert Juhe <[e-mail]>
>New colors


The message here should be:
Bug 927815 - Update the Infobar design according to shorlander's mockups. r=paul

Next round, ask we're getting closer, don't ask me for feedback, but for review.


> html|*.highlighter-nodeinfobar-id {
>-  color: hsl(90, 79%, 52%);
>+  color: hsl(103,46%,54%);
> }


Shorlander confirmed that we should use the pink color, not the purple one (square #4).


> html|*.highlighter-nodeinfobar-pseudo-classes {
>-  color: hsl(20, 100%, 70%);
>+  color: hsl(200,74%,57%);
> }


For some reason, we're missing the classes rule. Can you add:
> html|*.highlighter-nodeinfobar-classes
?

Use the same rule as -pseudo-classes.

> .highlighter-nodeinfobar-container[position="top"] > .highlighter-nodeinfobar,
> .highlighter-nodeinfobar-container[position="overlap"] > .highlighter-nodeinfobar {
>-  box-shadow: 0 1px 0 hsla(0, 0%, 100%, .1) inset;
>+  box-shadow: 0 1px 0 hsla(0,0%,100%,.1) inset;
> }


Any good reason to keep this box-shadow?

Comment 16

[Paul Rouget [:paul]]

Shorlander has updated the palette. We should use the 3rd square from the left (bottom row). Make sure to force the reload the palette (ctrl/cmd + shift + R):

http://people.mozilla.org/~shorlander/mockups/devTools/ux-refresh-2013/[e-mail]

Comment 17

[Albert Juhé]

Attachment: Attempt 5: new infobar dark theme

Ok, I've just applied the changes you said. I hope it's improving.

Comment 18

[Albert Juhé]

Attachment: Attempt 5 screenshot

Comment 19

[Paul Rouget [:paul]]

Comment on attachment 819859 [details] [diff] [review]
Attempt 5: new infobar dark theme

> .highlighter-nodeinfobar {
>-  color: hsl(200, 100%, 65%);
>-  border: 1px solid hsla(210, 19%, 63%, .5);
>+  color: hsl(200,100%,65%);

This is the default color used in the infobar.
You might want to use the white from the palett (216,33%,97%)
or the classes blue (200,74%,57%).

> html|*.highlighter-nodeinfobar-tagname {
>-  color: white;
>+  color: hsl(352,79%,62%);
> }

The right value is: 285,100%,75%
This is the 3rd square in the palette.
Make sure to look at the latest version of the palette (force refresh, I was seeing the cached version here).

Once you made these changes, we'll land your patch.
Thank you!

Comment 20

[Albert Juhé]

Attachment: Attempt 6: new infobar dark theme

Ok, here it is.

> This is the default color used in the infobar.
> You might want to use the white from the palett (216,33%,97%)
> or the classes blue (200,74%,57%).

Not sure which one to choose, is this color used anywhere? I wrote hsl(216,33%,97%).

I will wait for your review.

Comment 21

[Albert Juhé]

Attachment: Attempt 6: screenshot

Comment 22

[Paul Rouget [:paul]]

Comment on attachment 819916 [details] [diff] [review]
Attempt 6: new infobar dark theme

Thank you!

Comment 23

[Ed Morley [:emorley]]

Pushed this along with a bunch of other checkin-neededs to try:
https://tbpl.mozilla.org/?tree=Try&rev=2c426f843d08

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/fx-team/rev/1623dffd15e1

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1623dffd15e1

Comment 26

[Tim Nguyen :ntim]

hmm, there seem to be a glitch, see the active state of the "unlock" button.

Comment 27

[Albert Juhé]

I am not sure what do you mean, is the "unlock" button the button with the "Select element with mouse" tooltip?

I don't see any problem with it.

Comment 28

[Tim Nguyen :ntim]

The buttons on the infobar have a "windows classic" hover and active state. I see this on Firefox Nightly. Do you want a screenshot ?

Comment 29

[Paul Rouget [:paul]]

(In reply to ntim007 from comment #28)
> The buttons on the infobar have a "windows classic" hover and active state.
> I see this on Firefox Nightly. Do you want a screenshot ?

Yes please (nightly 27 + aurora 26, to see the difference). What's your OS?

Comment 30

[Tim Nguyen :ntim]

Attachment: screenshot.png

My OS is Windows 8.1.
Happens on both buttons.

Comment 31

[Paul Rouget [:paul]]

(In reply to ntim007 from comment #30)
> Created attachment 823373 [details]
> screenshot.png
> 
> My OS is Windows 8.1.
> Happens on both buttons.

Thanks a lot! I filed bug 931834.