After using the style editor more day-to-day, a use case that I find myself doing quite frequently which I think could be better served is that of visually scanning downwards through a list of styles to find a particular selector. As an example, I found myself inspecting the twitter bootstrap examples to see how they're put together ( http://cl.ly/012t2B3d0n432V3c0W2B ). My current goal was to find out how the "pull-left" class was affecting the form element I was inspecting. My immediate task then is to scan down the left-hand-side of the style editor to find the .pull-left selector. As you can see from the screenshot, the font and indentation along the left-hand side of the style editor are all uniform. This, coupled with the fact that the filenames, selectors, checkboxes, and dropdowns are all given similar visual weights and colors, makes the visual hierarchy very flat and difficult to scan for anything in particular. I believe that we can make this totally awesome though, with just a few tweaks! Some proposals: - Decrease the prominence of the checkboxes and dropdown indicators, so that the indentation of style properties is more profound. This could mean moving them to the right hand side, or just reducing their visual weight. - Reduce the visual prominence given to the file name / line number, as it is in general not as important as the style selector. Then increase the visual prominence of the style selector. This can be done by using colors or at least varying shades of grey to further differentiate levels in the information hierarchy, or simply moving the position of these objects. I'd love to provide you with some mockups that illustrate potential solutions for these visual problems if that would be helpful. I'd need to take into account your plans for current functionality and any unimplemented design work you've already got, so if this is interesting to you, just let me know and we can talk more! For point of contrast, in the Firebug style editor design makes this use-case really really clear, by having the selectors be a different color than the properties, values, and file names ( http://cl.ly/2F1p151z3n0Z0G3K050D ). The indentation also makes very clear what is a selector and what isn't. Similarly, the Chrome style editor uses color to differentiate elements, and also gives the file name/line number less prominence ( http://cl.ly/260G3x2h1p330q0f0J2G ). Though not as easy to scan for selectors as firebug (due to the "Matched CSS Rules" bar and the dropdowns/alert icons), it is still easier to scan for selectors than our style editor. We can do better! :D :D

First, this is a very clear and fair feedback. And I agree for most of it. Thank you for that. Second, afaik, we are not planning to update the style of this tool soon. But I believe that we really need to provide a much better user experience for the Rule View. So yes, please, mockups would be very much appreciated.

thanks for this bug, Jason. It is indeed good stuff. I'm rewording the summary to reflect the feature name. We have something of a nomenclature issue with some of these different tools now. The Style Editor is the new, full text editor for working with CSS in Firefox 11 (currently in Aurora). The Inspector Rule View is the feature you're referring to here and is part of the Inspector.

Thanks for the info, Rob! Inspector Rule View it is :D I'm going to be working on some mockups for this soon - next week is an Apps work week, so I won't be able to start then, but shortly thereafter.

Also, see bug 716516.

The 5 points relating to the rule view from bug 716516 are: 1. CSS selectors (along with braces) should be color 1 2. Names and values should be colors 2 and 3 3. Names and values should be indented where appropriate to make them easier to read 4. CSS rule checkboxes should only be visible only when the appropriate property is moused over ... this will greatly tidy up the visual space. 5. In order to free up even more visual space in the rule view the file and line number should be on the right-hand side of the CSS selector. I will make bug 716516 dependant on this one so that we know which colors and styles we should use with other tools.

Jason, can you assign yourself to this bug if you're working on it? Thanks! filter on pegasus

filter on pegasus

I've got a question for these mocks - are longhand properties in their expanded form going to be editable? http://cl.ly/0A2W350C3b3s3u0g3p1S I ask because right now, they look editable, but are not. So we should fix that, one way or the other ;)

Indeed, right now, they are not editable. I am not sure what that would mean if they could be editable. It would create a new declaration, and that would be weird. I think they should not be editable, and should be seen as a hint/help. Also, how useful are these longhand properties if they are not editable? I think they make the UI of a line quite wonky. The twisty should not be that "visible".

Ok, cool - I agree that making them editable would be weird, and that this leaves their purpose as mainly being educational. Yeah, I totally agree that the twisty should be less prominent because of this!

We are now showing the checkbox only on hover.

YESSSSSSSS~

To give you some more context, here is a mockup of how we would like to show the tools in the future. This is not 100% we will implement that, but I thought that could help (and the mockups shows the property view, not the rule view).

Jason, any update on the mockup?

Hey Paul - I've been in a bit of a crunch recently with the apps work. I'm not sure when it will let up, and I definitely don't want to hold you guys up. Since I'm at an offsite this week, I know that I will at least not be able to work on this any more for the remainder of the week, but am hoping to pick it up again afterwards.

Here, I am experimenting with reproducing the look of the Source Editor (same colors). I also indent the code and moved the file reference to the right (float:right). Screenshot: attachment 603146 [details] Code: attachment 603147 [details] [diff] [review] Might be a good temporary solution.

fixed the alignment

(haven't tested with Mac and Windows yet)

Also - we need to `overflow:ellipsis` the selectors, the file reference and the declarations. This is not very easy.

If the style sheet and line number is going to be a link to the Style Editor then I don't think it makes sense to make it look like a CSS comment. It should look like a link, but I love how it's off to the right side so you can focus more on the rule itself.

> I don't think it makes sense to make it look like a CSS comment Making it look like a CSS comment helps the user to scan the content of the sidebar. I understand that it makes the feature less discoverable, but I think it's a good tradeoff. I also think that the user will eventually over the comment and then see that underline and the "hand", and he will then understand that he can click there. I can also underline the reference by default: http://i.imgur.com/UoaQ1.png I think we need some Shorlander magic here.

Hey Paul - awesome work on this! I totally agree about the user eventually mousing over the style sheet/line number link and discovering it, and I think using the comment color there is a totally good call. I agree with Heather that the /* */ is probably overkill, though. I'd still like to try out your patch to get a feel for how the expanded styles work, but in the mean time I was too excited, so I whipped something up really quickly to demonstrate a couple concepts. :D Main differences: - Selectors are grouped by inheritance, and the "Inherited from ____"statements are lumped into one title for each group. The group of selectors that directly match this element (the one at the very top of the sidebar) doesn't have a group title. Please let me know if there are any problems with this. - Though I kept the CSS comment color on the style sheet/line number, I removed the /* */ to keep the visual noise of the sidebar at a minimum. - The style sheet/line number is underlined on hover. Thanks, I'd love to hear what you think!

I agree that the /* */ would only clutter the inspector. Rest of the mockup looks promising.

I would also find a solution to make obvious what is editable (property names, property values, and curly brackets) and what is not. I was thinking about using a dashed border-bottom to do that (see this screenshot: http://i.imgur.com/UoaQ1.png). If you guys have a better idea…

buttons do have more margin :) i also agree with this idea.

The light dashed underline and maybe a tooltip should be enough to tell the user that the property is editable. Maybe only the underline on hover? The underline is pretty much on the edge of cluttering the view. Maybe also drop the semi-colons? They don't mean anything in this context.

may I suggest adding a way to easily comment an entire selector (instead of having to click each element inside it)? thanks

(In reply to sys.sgx from comment #31) > may I suggest adding a way to easily comment an entire selector (instead of > having to click each element inside it)? > thanks Commenting a selector? You mean folding a rule set?

Ho, disabling a rule set.

Please file a bug for that.

done: https://bugzilla.mozilla.org/show_bug.cgi?id=734934 topic moved there.

we can relate this topic with this one: https://bugzilla.mozilla.org/show_bug.cgi?id=734965

Comment on attachment 604219 [details] mockup: feedback + suggestions for tentative 0.2 screenshot I really like splitting out the inherited rules. I've heard a web dev express that at first they just want to see the rules that match the element, and this would make that much easier. I also like the style editor links to the side. Aside from the color scheme which we'll update soon, I like this rule view quite a bit.

Comment on attachment 605677 [details] [diff] [review] patch v0.1 Mike, can you take a look at how I add the inherited source?

removed the border for the computed style

Comment on attachment 604219 [details] mockup: feedback + suggestions for tentative 0.2 screenshot I like it. Obviously we should be using the same name / value throughout all tools (object inspector etc.) but I guess that can come later.

(In reply to Michael Ratcliffe from comment #43) > Comment on attachment 604219 [details] > mockup: feedback + suggestions for tentative 0.2 screenshot > > I like it. Cool! > Obviously we should be using the same name / value throughout all > tools (object inspector etc.) but I guess that can come later. bug 715472

Comment on attachment 605701 [details] [diff] [review] patch v0.1.1 > .ruleview-property > span > .ruleview-propertyname, > .ruleview-property > .ruleview-propertyvalue { > border-bottom: 1px dashed #cddae5; > } Can you not use the following ... it would be much more efficient: .ruleview-propertyname, .ruleview-propertyvalue { border-bottom: 1px dashed #cddae5; } With that change f+ ... I especially like the grouping of inherited rules.

(In reply to Michael Ratcliffe from comment #45) > Comment on attachment 605701 [details] [diff] [review] > patch v0.1.1 > > > .ruleview-property > span > .ruleview-propertyname, > > .ruleview-property > .ruleview-propertyvalue { > > border-bottom: 1px dashed #cddae5; > > } > > Can you not use the following ... it would be much more efficient: > .ruleview-propertyname, > .ruleview-propertyvalue { > border-bottom: 1px dashed #cddae5; > } > > With that change f+ ... I especially like the grouping of inherited rules. Nope, because I don't want to use a border for the expanded values.

We need to do the same design update for the Computed View: bug 735629

(In reply to Paul Rouget [:paul] from comment #46) > (In reply to Michael Ratcliffe from comment #45) > > Comment on attachment 605701 [details] [diff] [review] > > patch v0.1.1 > > > > > .ruleview-property > span > .ruleview-propertyname, > > > .ruleview-property > .ruleview-propertyvalue { > > > border-bottom: 1px dashed #cddae5; > > > } > > > > Can you not use the following ... it would be much more efficient: > > .ruleview-propertyname, > > .ruleview-propertyvalue { > > border-bottom: 1px dashed #cddae5; > > } > > > > With that change f+ ... I especially like the grouping of inherited rules. > > Nope, because I don't want to use a border for the expanded values. Expanded values are not editable. And the border is supposed to mean "editable".

Comment on attachment 605702 [details] [diff] [review] patch v0.1.2 Try-builds: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/prouget@mozilla.com-023b9dafb24d/ Note that in this try build, even the non-editable properties (once a declaration is expanded) have the dashed border. This is fixed in patcvh v0.1.2.

suggestion: because some rules might be sequentially overrided by other css styles, we could add a "link" to each overrided rule in the Style panel so that upon clicking on it, it will "highlight" (or something similar) the style that is overriding it. Thus, the you will be able to tell the hierarchy of the rules.

The web console also plans to have a "dark" color theme. would it be possible to be in sync with that and also offer such a dark theme (matching the one of the console)? screenshot v0.1.2 is great.

(In reply to sys.sgx from comment #52) > The web console also plans to have a "dark" color theme. would it be > possible to be in sync with that and also offer such a dark theme (matching > the one of the console)? screenshot v0.1.2 is great. sys.sgx: Could you please refrain from advocating for and commenting on these bugs with your suggestions? The place for this kind of discussion is not bugzilla, but we do follow the news/google group: mozilla.dev.apps.firefox. Comments of this type or not really helpful here, though we do appreciate feedback in the appropriate forum. thanks.

rc: i think giving in sync suggestions for current specific bugs was part of the bugzilla process. so, the right way was to add a topic to the newsgroup (talking about pretty much everything mozilla related) suggesting to this specific bug to be in sync with another bug currently on its way to land to the next version?.. well, ok, i respect that.

triage, filter on centaur

I've had a request to make the line height a bit tighter to get more information into the view. I just wanted to pass that along for consideration as you're styling this view.

@jgrlicky ui-review ping? Also, I need help to make sure that the editable values look editable.

Hey Paul, Thank you so much for your patience on this bug. I do think the dotted-underlined values look editable! In addition to this, it might be with it to change the cursor to a pointer when hovering over the editable boxes to indicate that the item is actionable. I realize this is not the convention with text boxes, but these don't look like typical text-boxes, so some additional cues that activity is available seems appropriate to me. After using it, I've also got some other ideas about how to make it more polished. Split any of these out into other bugs as you see fit, of course! Can be addressed without a mockup: - Hovering behavior on the properties seems to show the checkboxes only after a delay. It would make the system more understandable and appear less sluggish to show the checkbox immediately (the current nightly behavior). - It seems like clicking on the whitespace to add a rule is broken in the patch. - The next lines of rules should wrap to their indentation level, like the expanded rules do. This http://cl.ly/042B393x1t0a0e1W0S2z stops the rules from being scannable. There might be even more we could do here, but this improvement is the low-hanging-fruit. - Expanded rules should not appear editable (via the mouse cursor or underlining), since they are not. May need a mockup: - The colors are not right on the blue "Inherited from" line dividers. The important thing is that the text in the dividers be more prominent than the borders. Some colors you could try would be text: #57819E, border: #B4C4D1. - Expanders are too prominent. I would make them the same visual prominence as the blue "Inherited from" dividers. A color to try would be #CDDAE5. - Indentation on expanded rules can be reduced. Thanks for the great work you've been doing for web developers everywhere, Paul. If you need clarification on anything, feel free to ask!

> - It seems like clicking on the whitespace to add a rule is broken in the patch. If clicking on whitespace allowed you to add a property then it would not be possible to copy text.

Ignore comment 59, I was wrong.

Note for later: the expanded properties (from shorthand properties) should have a different look (even if subtle) to reflect the fact they are not editable.

For the record, here is a recent mockup for the Rule View: https://bug715472.bugzilla.mozilla.org/attachment.cgi?id=622375 And a mockup (a little older) for the Computed View: https://bug749628.bugzilla.mozilla.org/attachment.cgi?id=619038

rebased patch v0.1.2, with some style fixes.

Rebased patch with and all tests passing. Style is a bit different than Paul's original patch, mainly I kept the background white to match the computed view for the time being.

Comment on attachment 639363 [details] [diff] [review] rebased patch with fixed test Thanks for rebasing all this. Since your work is based on my work, it's possible that I'm the one to blame for some of these remarks :) - the pseudo indentation doesn't work. And for some reason, I don't see the "ruleview-property-name-and-colon" span in the DOM. - for hsl colors, don't use spaces hsl(121,42%,43%) - can you explain why we need `_clearRules()` in nodeChanged now? - can we make the filename:linenumber block unselectable? - for transitions, don't use the -moz- prefix

Comment on attachment 639363 [details] [diff] [review] rebased patch with fixed test Review of attachment 639363 [details] [diff] [review]: ----------------------------------------------------------------- Looks great.

Updated patch to Paul's comments: (In reply to Paul Rouget [:paul] from comment #65) > - the pseudo indentation doesn't work. And for some reason, I don't see the > "ruleview-property-name-and-colon" span in the DOM. Discussed this on IRC. I think it's easier to read without aligning the property values. Aligning the property values makes it easier to compare values, but they're unrelated and don't really need to be compared. At the same time it makes it a bit slower to associate a property name with it's value, which is important. > - can you explain why we need `_clearRules()` in nodeChanged now? Discussed with dcamp. A recent change was made to save unnecessary UI updates on node changes, but this was too complicated with the new structure of laying out the rules. So we're reverting back to clearing and rebuilding the rule list on a node change. > - can we make the filename:linenumber block unselectable? Done. For the "inherited from" headers also.

Comment on attachment 641080 [details] [diff] [review] rebased patch with fix for comments Steven, can you take a look at this new layout for the rule view? Try builds here: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/fayearthur@gmail.com-d051ad30bfe5/

Comment on attachment 641080 [details] [diff] [review] rebased patch with fix for comments Review of attachment 641080 [details] [diff] [review]: ----------------------------------------------------------------- I'm happy with the code if UX is happy with the behavior/look.

Comment on attachment 641080 [details] [diff] [review] rebased patch with fix for comments Review of attachment 641080 [details] [diff] [review]: ----------------------------------------------------------------- This looks good! The only thing I noticed is that selecting the property changes the height of the section causing everything to shift vertically. This doesn't happen with the values. r+ with that fixed.

http://hg.mozilla.org/integration/fx-team/rev/2149897d05a0 (In reply to Stephen Horlander from comment #70) > The only thing I noticed is that selecting the property changes the height > of the section causing everything to shift vertically. This doesn't happen > with the values. r+ with that fixed. fixed. nice catch!

-# e.g "Inherited from body#bodyID (styles.css:20)" -rule.inheritedSource=Inherited from %S (%S) +# e.g "Inherited from body#bodyID" +rule.inheritedSource=Inherited from %S I know I should probably open a new bug for this, but: a) don't change a string like this (removing a variable) without changing its name as well b) you should update its l10n comment (last line) # LOCALIZATION NOTE (rule.inheritedSource): Shown for CSS rules # that were inherited from a parent node. Will be passed a node # identifier and a source location.

(In reply to Francesco Lodolo [:flod] from comment #73) > -# e.g "Inherited from body#bodyID (styles.css:20)" > -rule.inheritedSource=Inherited from %S (%S) > +# e.g "Inherited from body#bodyID" > +rule.inheritedSource=Inherited from %S > > I know I should probably open a new bug for this, but: > > a) don't change a string like this (removing a variable) without changing > its name as well > b) you should update its l10n comment (last line) > > # LOCALIZATION NOTE (rule.inheritedSource): Shown for CSS rules > # that were inherited from a parent node. Will be passed a node > # identifier and a source location. You're very right. Sorry, we missed that. Filed bug 775692.

(In reply to Paul Rouget [:paul] from comment #74) > You're very right. Sorry, we missed that. Filed bug 775692. Thanks Paul. Next I'll be less lazy and I'll open the bug myself ;-)