Closed Bug 1266716 Opened 8 years ago Closed 8 years ago

Firebug theme: Improve storage inspector

Categories

(DevTools :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox48 fixed, firefox49 fixed)

RESOLVED FIXED
Firefox 49
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: Honza, Assigned: Honza)

References

Details

Attachments

(1 file, 2 obsolete files)

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
Assignee: nobody → odvarko
Attached patch bug1266716.patch (obsolete) — Splinter Review
- Fixed vertical centering of hader labels
- Fixed color and bk-color of selected row

Honza
Attachment #8744322 - Flags: review?(ntim.bugs)
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.
Attachment #8744322 - Flags: review?(ntim.bugs)
(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
Flags: needinfo?(ntim.bugs)
(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.
Flags: needinfo?(ntim.bugs)
Attached patch bug1266716.patch (obsolete) — Splinter Review
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
Attachment #8744322 - Attachment is obsolete: true
Attachment #8745323 - Flags: review?(ntim.bugs)
Severity: normal → enhancement
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).
Attachment #8745323 - Flags: review?(ntim.bugs) → review+
Attached patch bug1266716.patchSplinter Review
(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
Attachment #8745323 - Attachment is obsolete: true
Attachment #8745980 - Flags: review+
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
Attachment #8745980 - Flags: approval-mozilla-aurora?
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/7b4589f13fca
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment on attachment 8745980 [details] [diff] [review]
bug1266716.patch

Firebug theme related, Aurora48+
Attachment #8745980 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.