Last Comment Bug 709006 - Implement a Orion theme that matches the DevTools theme
: Implement a Orion theme that matches the DevTools theme
Status: VERIFIED FIXED
[sourceeditor][orion][qa!]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 11
Assigned To: Pavel Ivanov [:ivanovpavel][:pivanov] UX
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
Depends on: 687702 702331
Blocks: 708257
  Show dependency treegraph
 
Reported: 2011-12-09 03:35 PST by Paul Rouget [:paul]
Modified: 2012-02-17 07:18 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
verified


Attachments
style editor mockup with orion (226.36 KB, image/jpeg)
2011-12-09 03:36 PST, Paul Rouget [:paul]
no flags Details
patch v1 (27.15 KB, patch)
2011-12-17 11:24 PST, Paul Rouget [:paul]
mihai.sucan: review+
Details | Diff | Splinter Review
screenshot with new theme (131.23 KB, image/png)
2011-12-17 11:27 PST, Paul Rouget [:paul]
no flags Details
screenshot with the solarized theme (139.61 KB, image/png)
2011-12-18 07:36 PST, Paul Rouget [:paul]
no flags Details
patch - final (28.04 KB, patch)
2011-12-18 07:59 PST, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
mac screenshot (262.75 KB, image/png)
2011-12-19 09:21 PST, Rob Campbell [:rc] (:robcee)
no flags Details
windows screenshot (322.76 KB, image/png)
2011-12-19 09:22 PST, Rob Campbell [:rc] (:robcee)
no flags Details

Description Paul Rouget [:paul] 2011-12-09 03:35:55 PST
We don't want a dark theme (white on black) but a light one (black on white). But the theme should fit in the devtools overall look.
Comment 1 Paul Rouget [:paul] 2011-12-09 03:36:57 PST
Created attachment 580358 [details]
style editor mockup with orion
Comment 2 Mihai Sucan [:msucan] 2011-12-09 05:03:31 PST
Bug 702331 includes preparations for allowing us to customize the Orion theme in a way that doesn't break with new upstream versions of Orion.
Comment 3 Rob Campbell [:rc] (:robcee) 2011-12-16 06:25:46 PST
still want solarized. (light and dark versions!)

http://ethanschoonover.com/solarized
Comment 4 Panos Astithas [:past] 2011-12-16 06:28:00 PST
(In reply to Rob Campbell [:rc] (robcee) from comment #3)
> still want solarized. (light and dark versions!)
> 
> http://ethanschoonover.com/solarized

+1
Comment 5 Paul Rouget [:paul] 2011-12-16 07:14:20 PST
I am not against Solarized (I like monokai too), but we need something that matches the devtools theme, and that can be used for the HTML Tree as well. It also needs to match the colors of the infobar and the breadcrumbs.

The good think with Solarized is that it uses a blue color for the variable names.
	
Pavel, do you think you can try to work on a colorscheme that take into account:
- the colors of the toolbar: See this https://bug676253.bugzilla.mozilla.org/attachment.cgi?id=556968
- the colors of the infobar: See this https://bug663833.bugzilla.mozilla.org/attachment.cgi?id=554557

Inspiration from http://ethanschoonover.com/solarized is allowed :)

You don't need to build a CSS for Orion. Just some mockups are enough I think.

Let me know if you need any help.
Comment 6 Pavel Ivanov [:ivanovpavel][:pivanov] UX 2011-12-16 07:27:32 PST
I agree with Paul. We need something that matches the devtools theme.
I work on them :). Maybe i'll finish them on the end of the weekend.
Comment 7 Pavel Ivanov [:ivanovpavel][:pivanov] UX 2011-12-17 06:29:12 PST
I made this color schemes (light & dark).
you can see them here :
http://bit.ly/vqBB3A

I hope you like them :)
Comment 8 Panos Astithas [:past] 2011-12-17 06:42:42 PST
(In reply to Pavel Ivanov from comment #7)
> I made this color schemes (light & dark).
> you can see them here :
> http://bit.ly/vqBB3A
> 
> I hope you like them :)

It's definitely distinctive and elegant, but I think the contrast is too low. I'm having a hard time reading the text in a rather dark room at dusk.
Comment 9 Paul Rouget [:paul] 2011-12-17 07:23:38 PST
Thank you Pavel! Looks nice :)

About the light theme, like Panos said, the contrast is a bit too low. Do you think you can make it a bit more contrasted (especially the purple colors)?

Let's focus on a light theme for now.

I think we need a dark theme, but let's do that later.
Comment 10 Paul Rouget [:paul] 2011-12-17 10:09:08 PST
So Orion's tokenizer is quite limited. We need to style these elements:
- normal text
- strings (elements between "" and '')
- keywords (in CSS: propertye names, in JS: keywords, like "if", "function", "else", ...)
- comments
Comment 11 Paul Rouget [:paul] 2011-12-17 11:24:29 PST
Created attachment 582558 [details] [diff] [review]
patch v1
Comment 12 Paul Rouget [:paul] 2011-12-17 11:27:47 PST
Created attachment 582560 [details]
screenshot with new theme
Comment 13 Paul Rouget [:paul] 2011-12-17 11:29:04 PST
This latest patch is based on Ivanov work. See http://pivanov.com/mozilla/style-editor/style-editor-v5.html
Comment 14 Paul Rouget [:paul] 2011-12-17 11:35:34 PST
So this is a "good-enough" theme, that is better than the previous one. I would like to get this for Firefox 11. I don't think we will get a UI review in time. Can we agree on this one and iterate later?
Comment 15 Mihai Sucan [:msucan] 2011-12-17 12:00:42 PST
Comment on attachment 582558 [details] [diff] [review]
patch v1

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

Patch looks good, just some indentation problems.

I do have some suggestions for improved contrast. I hope they are acceptable.

One point that needs to be made: stock settings on today's monitors give off faar too much brightness, making people's eyes hurt. This is why some programmers prefer darker backgrounds, lower contrast and so on. However others do change their monitor settings to make watching white/bright colors bearable, less painful for the eyes. The side effect is that we need good contrast.

Hence I do suggest we add a dark theme (post Fx11, I assume), and keep the bright theme a bright one - not something in-between with low contrast.

Thank you!

(I'm giving the patch r+, since I won't keep personal tastes on colors and themes holding it off from landing. Please update as you see fit.)

::: browser/devtools/sourceeditor/orion/mozilla.css
@@ +1,5 @@
>  /* Any copyright is dedicated to the Public Domain.
>     http://creativecommons.org/publicdomain/zero/1.0/ */
>  
>  .viewContainer {
> +	background-color: #cddae5; /* This will be seen as the continuation of the ruler */

Why the change from background to background-color?

@@ +7,5 @@
>  	font-size: inherit; /* inherit browser's default monospace font size */
>  }
>  
>  .view {
> +	background: #e6e6e6; /* Background of the editor */

I believe this background is far too dark, low contrast.

I would suggest #fdfdfd. (works for me)

@@ +15,5 @@
> +	background: #d6d6d6;
> +}
> +
> +.viewContent > div { /* One line */
> +  padding-left: 4px; /* Margin between the ruler and the editor */

Different indentation from the code above.

@@ +26,5 @@
>  
>  /* Styles for the line number ruler */
>  .rulerLines {
> +	border-right: 1px solid #b4c4d3;
> +	background-color: #cddae5;

Same as above: why go from 'background' to 'background-color'?

@@ +27,5 @@
>  /* Styles for the line number ruler */
>  .rulerLines {
> +	border-right: 1px solid #b4c4d3;
> +	background-color: #cddae5;
> +	color: #7a8a99;

Too low contrast here as well. I suggest #3a4a59.

@@ +48,4 @@
>  }
>  
>  .token_doc_html_markup {
> +  color: #dd0058; /* purple */

Nit: indentation.

@@ +52,4 @@
>  }
>  
>  .token_doc_tag {
> +  color: #dd0058; /* purple */

Nit: indentation.

@@ +56,5 @@
>  }
>  
> +.token_task_tag { /* "TODO" */
> +	color: black;
> +  background-color: yellow;

Nit: indentation.

@@ +83,5 @@
>   	background-position: left center;
>  }
>  
> +.line_caret { /* Current line */
> +  background: #eaf2fe; /* lighter than the background */

More indentation nits here and below.
Comment 16 Mihai Sucan [:msucan] 2011-12-17 12:09:31 PST
Also, what's up with the png changes? Are they related to the new Orion theme?
Comment 17 Paul Rouget [:paul] 2011-12-17 12:34:52 PST
(In reply to Mihai Sucan [:msucan]:
> Also, what's up with the png changes? Are they related to the new Orion theme?

Yes. The arrows from the file list need to use the background color of the ruler.

> Comment on attachment 582558 [details] [diff] [review]
> Hence I do suggest we add a dark theme (post Fx11, I assume), and keep the
> bright theme a bright one - not something in-between with low contrast.

Ok - I'll try to improve the contrast.

Thank you!
Comment 18 Panos Astithas [:past] 2011-12-17 13:56:02 PST
I find the readability of Paul's latest screenshot better than the previous ones. I also agree with Mihai's contrast observations, but I certainly wouldn't mind delaying theme improvements until Firefox 12.
Comment 19 Dão Gottwald [:dao] 2011-12-18 04:47:50 PST
(In reply to Mihai Sucan [:msucan] from comment #15)
> > +	background: #d6d6d6;
> > +}
> > +
> > +.viewContent > div { /* One line */
> > +  padding-left: 4px; /* Margin between the ruler and the editor */
> 
> Different indentation from the code above.

Indentation in CSS files is two spaces.
Comment 20 Paul Rouget [:paul] 2011-12-18 07:36:34 PST
Created attachment 582665 [details]
screenshot with the solarized theme

Because people are mentioning it, I thought that I could give it a try. I think this doesn't look very well integrated with our tool.
Comment 21 Paul Rouget [:paul] 2011-12-18 07:59:05 PST
Created attachment 582671 [details] [diff] [review]
patch - final
Comment 23 Rob Campbell [:rc] (:robcee) 2011-12-19 09:19:43 PST
after looking at a couple of screens, I feel the background color chosen in this theme is a little hard on the eyes. There's too much red in it ... or something. Suggesting we revert to the light grey background until we can get some UX assessment.

attaching screenshots for the mac and windows.
Comment 24 Rob Campbell [:rc] (:robcee) 2011-12-19 09:21:27 PST
Created attachment 582853 [details]
mac screenshot
Comment 25 Rob Campbell [:rc] (:robcee) 2011-12-19 09:22:34 PST
Created attachment 582854 [details]
windows screenshot
Comment 26 Tim Taubert [:ttaubert] 2011-12-20 00:58:40 PST
https://hg.mozilla.org/mozilla-central/rev/462e79456fe8
Comment 27 Scoobidiver (away) 2011-12-21 05:13:45 PST
It missed the Firefox 11 release for 10 minutes.
Comment 28 Rob Campbell [:rc] (:robcee) 2011-12-21 05:33:58 PST
Comment on attachment 582671 [details] [diff] [review]
patch - final

New theme for our code-editor which more closely-matches the over-arching "dark" theme for developer tools. Solid contributor contribution on this patch to get it ready for Firefox 11. I would hate to make them wait another release. :)

Minimal risk, applies only to the source editor, CSS only.
Comment 29 Scoobidiver (away) 2011-12-21 05:43:19 PST
May be I am wrong about the Firefox 11 target because this changeset is between:
* The latest changeset in 11.0a1/20111220: http://hg.mozilla.org/mozilla-central/rev/2afd7ae68e8b.
* The first changeset in 11.0a2/20111220: http://hg.mozilla.org/mozilla-central/rev/a8506ab2c654
Comment 30 Scoobidiver (away) 2011-12-21 09:32:42 PST
I was definitively wrong about the Firefox 11 target. It made it.
Comment 31 christian 2011-12-21 15:53:15 PST
Comment on attachment 582671 [details] [diff] [review]
patch - final

[triage comment]
Clearing the approval request as it is not needed due to comment 30 (made the source uplift/migration)
Comment 32 Rob Campbell [:rc] (:robcee) 2011-12-22 07:39:51 PST
excellent. Marking this bug fixed. Thanks for tracking it, Scoobidiver!
Comment 33 Virgil Dicu [:virgil] [QA] 2012-02-17 07:18:10 PST
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0

The theme is implemented in Firefox 11 (beta 3)-as screenshot in comment 24.

Note You need to log in before you can comment on or make changes to this bug.