1266716 - Firebug theme: Improve storage inspector

Status: RESOLVED FIXED

(DevTools :: General, enhancement)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

There are some inconsistencies in the Storage inspector panel when Firebug theme is active. E.g. the selected item in the Storage inspector has white text on white background.

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1266716.patch

- Fixed vertical centering of hader labels
- Fixed color and bk-color of selected row

Honza

Comment 2

[Tim Nguyen :ntim]

Comment on attachment 8744322 [details] [diff] [review]
bug1266716.patch

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

::: devtools/client/themes/widgets.css
@@ +1377,2 @@
>    background: #FFFFFF;
>  }

Instead of these 2 rules, you can simply override --table-zebra-background at the top of this file to #efefef, since the background is already #fff by default.

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Tim Nguyen [:ntim] (busy, email me instead) from comment #2)
> Comment on attachment 8744322 [details] [diff] [review]
> bug1266716.patch
> 
> Review of attachment 8744322 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: devtools/client/themes/widgets.css
> @@ +1377,2 @@
> >    background: #FFFFFF;
> >  }
> 
> Instead of these 2 rules, you can simply override --table-zebra-background
> at the top of this file to #efefef, since the background is already #fff by
> default.
I don't understand what you mean. The background should
*not* be #fff if the row is selected.

Honza

Comment 4

[Tim Nguyen :ntim]

(In reply to Jan Honza Odvarko [:Honza] from comment #3)
> (In reply to Tim Nguyen [:ntim] (busy, email me instead) from comment #2)
> > Comment on attachment 8744322 [details] [diff] [review]
> > bug1266716.patch
> > 
> > Review of attachment 8744322 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: devtools/client/themes/widgets.css
> > @@ +1377,2 @@
> > >    background: #FFFFFF;
> > >  }
> > 
> > Instead of these 2 rules, you can simply override --table-zebra-background
> > at the top of this file to #efefef, since the background is already #fff by
> > default.
> I don't understand what you mean. The background should
> *not* be #fff if the row is selected.
> 
> Honza

The background is already #fff for unstriped rows.

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1266716.patch

Hm..., I am not fully sure what changed (my feeling is that it could be inline-editing for the Storage panel), but it looks like these two rows aren't necessary anymore. One change to the splitter though.

Honza

Comment 6

[Tim Nguyen :ntim]

Comment on attachment 8745323 [details] [diff] [review]
bug1266716.patch

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

r=me with this comment addressed.

::: devtools/client/themes/widgets.css
@@ +1368,2 @@
>    position: absolute;
>  }

You might want to remove this whole block (.theme-firebug .table-widget-body .devtools-side-splitter) alltogether. It seems to be an improvement for the firebug theme (since it makes it look more consistent with the netmonitor by adding separators to the header).

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1266716.patch

(In reply to Tim Nguyen :ntim from comment #6)
> Comment on attachment 8745323 [details] [diff] [review]
> bug1266716.patch
> 
> Review of attachment 8745323 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with this comment addressed.
Done, it looks great now!

Honza

Comment 8

[Pulsebot]

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

Comment 9

[Tim Nguyen :ntim]

Comment on attachment 8745980 [details] [diff] [review]
bug1266716.patch

Approval Request Comment
[Feature/regressing bug #]: New Firebug theme
[User impact if declined]: Unreadable text for selected rows in Storage inspector
[Describe test coverage new/current, TreeHerder]: soon in Nightly, locally tested
[Risks and why]: Low, CSS code removal
[String/UUID change made/needed]: none

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7b4589f13fca

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8745980 [details] [diff] [review]
bug1266716.patch

Firebug theme related, Aurora48+

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/469db9271b52