Paul Rouget [:paul] Reporter
	Description • 13 years ago

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.

	Mihai Sucan [:msucan]
	Comment 2 • 13 years ago

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.

	Rob Campbell [:rc] (:robcee)
	Comment 3 • 13 years ago

still want solarized. (light and dark versions!)

http://ethanschoonover.com/solarized

	Panos Astithas (he/him) [:past] (please ni?)
	Comment 4 • 13 years ago

(In reply to Rob Campbell [:rc] (robcee) from comment #3)
> still want solarized. (light and dark versions!)
> 
> http://ethanschoonover.com/solarized

+1

	Paul Rouget [:paul] Reporter
	Comment 5 • 13 years ago

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.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 6 • 13 years ago

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.

	Pavel Ivanov [:ivanovpavel][:pivanov] UX Assignee
	Comment 7 • 13 years ago

I made this color schemes (light & dark).
you can see them here :
http://bit.ly/vqBB3A

I hope you like them :)

	Panos Astithas (he/him) [:past] (please ni?)
	Comment 8 • 13 years ago

(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.

	Paul Rouget [:paul] Reporter
	Comment 9 • 13 years ago

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.

	Paul Rouget [:paul] Reporter
	Comment 10 • 13 years ago

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

	Paul Rouget [:paul] Reporter
	Comment 13 • 13 years ago

This latest patch is based on Ivanov work. See http://pivanov.com/mozilla/style-editor/style-editor-v5.html

	Paul Rouget [:paul] Reporter
	Comment 14 • 13 years ago

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?

	Mihai Sucan [:msucan]
	Comment 15 • 13 years ago

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.

	Mihai Sucan [:msucan]
	Comment 16 • 13 years ago

Also, what's up with the png changes? Are they related to the new Orion theme?

	Paul Rouget [:paul] Reporter
	Comment 17 • 13 years ago

(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!

	Panos Astithas (he/him) [:past] (please ni?)
	Comment 18 • 13 years ago

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.

	Dão Gottwald [:dao]
	Comment 19 • 13 years ago

(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.

	Paul Rouget [:paul] Reporter
	Comment 20 • 13 years ago

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.

	Paul Rouget [:paul] Reporter
	Comment 22 • 13 years ago

https://hg.mozilla.org/integration/fx-team/rev/462e79456fe8

	Rob Campbell [:rc] (:robcee)
	Comment 23 • 13 years ago

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.

	Tim Taubert [:ttaubert] (inactive)
	Comment 26 • 13 years ago

https://hg.mozilla.org/mozilla-central/rev/462e79456fe8

	Scoobidiver (away)
	Comment 27 • 13 years ago

It missed the Firefox 11 release for 10 minutes.

	Rob Campbell [:rc] (:robcee)
	Comment 28 • 13 years ago

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.

	Scoobidiver (away)
	Comment 29 • 13 years ago

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

	Scoobidiver (away)
	Comment 30 • 13 years ago

I was definitively wrong about the Firefox 11 target. It made it.

	LegNeato
	Comment 31 • 13 years ago

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)

	Rob Campbell [:rc] (:robcee)
	Comment 32 • 13 years ago

excellent. Marking this bug fixed. Thanks for tracking it, Scoobidiver!

	Virgil Dicu [:virgil] [QA]
	Comment 33 • 13 years ago

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.