720980 - Quick rule view ... quick layout fix until we have mockups

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P2)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

At the moment the layout of the style inspector's rule view is not optimal. I made a few simple tweaks when working on another bug. If we have a patch that is better than the current implementation we should land it.

Of course, when bug 712469 is complete we will be able to use that ... this is more of a 5 minute step in the right direction.

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Simple patch

Just a simple patch in case the mockups take a long time.

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Patch

Comment 3

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 591410 [details] [diff] [review]
Patch

I think this looks pretty good. Will try it out and report back.

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 591410 [details] [diff] [review]
Patch

>--- a/browser/devtools/styleinspector/styleinspector.css
>+++ b/browser/devtools/styleinspector/styleinspector.css

>+.ruleview-firstline {
>+  display: inline-block;
>+}

Replace this with overflow:hidden ...

>--- a/browser/themes/gnomestripe/devtools/csshtmltree.css
>+++ b/browser/themes/gnomestripe/devtools/csshtmltree.css

>+.ruleview-firstline {
>+  width: 100%;
> }

... and drop this?

> .ruleview-rule-source {
>-  background-color: -moz-dialog;
>   padding: 2px 5px;
>+  float: right;
> }

The vertical padding makes this element misaligned. Not sure if this is new due to float:right.

> .ruleview-code {
>-  padding: 2px 5px;
>+  padding: 2px 5px 10px;;
>+}

double semicolon

> .ruleview-enableproperty {
>   height: 10px;
>   width: 10px;
>   -moz-margin-start: 2px;
>   -moz-margin-end: 0;
>+  visibility: hidden;
>+}
>+
>+.ruleview-property:hover > .ruleview-enableproperty {
>+  visibility: visible;
> }

.ruleview-property:not(:hover) > .ruleview-enableproperty {
  visibility: hidden;
}

Comment 5

[Dão Gottwald [:dao]]

> > .ruleview-enableproperty {
> >   height: 10px;
> >   width: 10px;
> >   -moz-margin-start: 2px;
> >   -moz-margin-end: 0;
> >+  visibility: hidden;
> >+}
> >+
> >+.ruleview-property:hover > .ruleview-enableproperty {
> >+  visibility: visible;
> > }
> 
> .ruleview-property:not(:hover) > .ruleview-enableproperty {
>   visibility: hidden;
> }

This could also be put in the content stylesheet.

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Simple patch 2

(In reply to Dão Gottwald [:dao] from comment #4)
> Comment on attachment 591410 [details] [diff] [review]
> Patch
> 
> >--- a/browser/devtools/styleinspector/styleinspector.css
> >+++ b/browser/devtools/styleinspector/styleinspector.css
> 
> >+.ruleview-firstline {
> >+  display: inline-block;
> >+}
> 
> Replace this with overflow:hidden ...
> 

Done

> >--- a/browser/themes/gnomestripe/devtools/csshtmltree.css
> >+++ b/browser/themes/gnomestripe/devtools/csshtmltree.css
> 
> >+.ruleview-firstline {
> >+  width: 100%;
> > }
> 
> ... and drop this?
> 

Done

> > .ruleview-rule-source {
> >-  background-color: -moz-dialog;
> >   padding: 2px 5px;
> >+  float: right;
> > }
> 
> The vertical padding makes this element misaligned. Not sure if this is new
> due to float:right.
> 

It is better without any padding. Also, because there is only a float I have moved it to the content stylesheet.

> > .ruleview-code {
> >-  padding: 2px 5px;
> >+  padding: 2px 5px 10px;;
> >+}
> 
> double semicolon
> 

Fixed

> > .ruleview-enableproperty {
> >   height: 10px;
> >   width: 10px;
> >   -moz-margin-start: 2px;
> >   -moz-margin-end: 0;
> >+  visibility: hidden;
> >+}
> >+
> >+.ruleview-property:hover > .ruleview-enableproperty {
> >+  visibility: visible;
> > }
> 
> .ruleview-property:not(:hover) > .ruleview-enableproperty {
>   visibility: hidden;
> }

Changed

(In reply to Dão Gottwald [:dao] from comment #5)
> > > .ruleview-enableproperty {
> > >   height: 10px;
> > >   width: 10px;
> > >   -moz-margin-start: 2px;
> > >   -moz-margin-end: 0;
> > >+  visibility: hidden;
> > >+}
> > >+
> > >+.ruleview-property:hover > .ruleview-enableproperty {
> > >+  visibility: visible;
> > > }
> > 
> > .ruleview-property:not(:hover) > .ruleview-enableproperty {
> >   visibility: hidden;
> > }
> 
> This could also be put in the content stylesheet.

True, done.

Comment 7

[Dave Camp (:dcamp)]

This wordwraps really badly for me when the rule view is thinner, I get the css location (filename:line) *inside* the rule.

Comment 8

[Dave Camp (:dcamp)]

Comment on attachment 591829 [details] [diff] [review]
Simple patch 2

(f- on that basis, but rcampbell can override me)

Comment 9

[Rob Campbell [:rc] (:robcee)]

yeah, this was a little weird for me during playtesting, but overall I think the improvements are "better" overall. Really long selectors can get a little funky in a tiny sidebar. Maybe change the min-width on the sidebar to force more space? Maybe start hiding filenames if below a certain threshold?

Comment 10

[Dave Camp (:dcamp)]

If the selectors could just wrap under the filename instead of above, that would be nice.

Comment 11

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

I can always move the layout into a table ... that is by far the best way to solve the problem.

Comment 12

[Dave Camp (:dcamp)]

Let's save large reworkings for bug 712496.

Comment 13

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Fixed wrapping

(In reply to Dave Camp (:dcamp) from comment #10)
> If the selectors could just wrap under the filename instead of above, that
> would be nice.

It did look a little funky so I have fixed it.

Comment 14

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Only hide checkboxes

We decided against moving the filenames to the right because of text mangling.

This is just a simple patch to hide the checkboxes ... Dão has already checked it.

Comment 15

[Paul Rouget [:paul]]

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

Comment 16

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/f5709d1da2ae