727867 - Rule view indicate when invalid values are entered

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P2)

Description

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

Attachment: This is the webkit implementation ... we will do something like this

Now that user entered values can be entered in the rule view we need a way to signify the validity of user entered property values.

Comment 1

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

Attachment: Patch

Much simpler than I expected.

Comment 2

[Dave Camp (:dcamp)]

Can you post a screenshot of this in action please?

Comment 3

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

Attachment: In Action

Sure, it looks almost identical to webkit's design. Hovering the warning symbol displays "Invalid property value" ... or at least it will do when bug 729539 is fixed (title & tooltips do not show tooltips in the sidebar).

The design may change when Jason has finished his mockup (bug 712469).

Comment 4

[Stephen Horlander [:shorlander]]

Comment on attachment 599950 [details]
In Action

I think this is perfectly fine for now. Knowing that you need to hover over the icon for the tooltip might not be immediately obvious. 

Probably worth some thought on if we can display this information in a less generic and more obvious way at some point.

Is that icon the right size for the height of the list items?

Comment 5

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

Good point, I will shrink it a little before we land it

Comment 6

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

Attachment: Shrunk icon as shorlander suggested

I have shrunk the icon a little as shorlander suggested.

Comment 7

[Dave Camp (:dcamp)]

Comment on attachment 600094 [details] [diff] [review]
Shrunk icon as shorlander suggested

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

I'm happy with it if shorlander's happy with the UI!

::: browser/devtools/styleinspector/CssRuleView.jsm
@@ +985,5 @@
>  
>      appendText(this.element, ";");
>  
> +    this.warning = createChild(this.element, "div", {
> +      // For an unknown reason the hidden attribute does not work here.

Is this comment still an issue?  It seems like it's working?

Comment 8

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

Attachment: Removed stray comment

(In reply to Dave Camp (:dcamp) from comment #7)
> Comment on attachment 600094 [details] [diff] [review]
> Shrunk icon as shorlander suggested
> 
> Review of attachment 600094 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm happy with it if shorlander's happy with the UI!
> 
> ::: browser/devtools/styleinspector/CssRuleView.jsm
> @@ +985,5 @@
> >  
> >      appendText(this.element, ";");
> >  
> > +    this.warning = createChild(this.element, "div", {
> > +      // For an unknown reason the hidden attribute does not work here.
> 
> Is this comment still an issue?  It seems like it's working?

Yes and no ... I added .ruleview-warning[hidden] { display: none; } to make it work but it should have worked without. Leaving this comment is confusing though so I have removed it.

Comment 9

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

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

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/399b98a618f3

Comment 11

[Eric Shepherd [:sheppy]]

Documentation updated:

https://developer.mozilla.org/en/Tools/Page_Inspector/Style_panel#New_features_in_Firefox_13

Listed on Firefox 13 for developers