Style tweaks for Computed view

RESOLVED FIXED in Firefox 18

Status

RESOLVED FIXED
7 years ago
9 months ago

People

(Reporter: harth, Assigned: harth)

Tracking

unspecified
Firefox 18

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

7 years ago
We're making the computed view prettier in bug 735629, but in the meantime I suggest reducing the padding around each style property so you can scan more without scrolling and putting the search box on the left hand side to align with the properties.
(Assignee)

Comment 1

7 years ago
Patch for proposed tweaks.
For the toolbar holding the checkbox and the searchbox, could we use the devtools theme as well? Using class="devtools-toolbar" class="devtools-searchinput" should work (not sure if it should be part of this bug, and if it even looks good)
(Assignee)

Comment 3

7 years ago
Here's a screenshot with those two classes applied too.
(Assignee)

Comment 4

7 years ago
Comment on attachment 611597 [details] [diff] [review]
Reduce padding and put search box on left

We can do the dark themeing of the toolbar in a separate bug.
Attachment #611597 - Flags: review?(jwalker)
Comment on attachment 611597 [details] [diff] [review]
Reduce padding and put search box on left

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

r+ technically. I'd like to have a look but I'm travelling, so I'll try to have a look on monday.
Attachment #611597 - Flags: review?(jwalker) → review+
(In reply to Joe Walker from comment #5)
> r+ technically. I'd like to have a look but I'm travelling, so I'll try to
> have a look on monday.

It's monday still. In some part of the world. Maybe.
Time travel aside. it looks good to me.
(Assignee)

Comment 7

7 years ago
http://hg.mozilla.org/integration/fx-team/rev/e90f0d780ca9
Status: NEW → ASSIGNED
Whiteboard: [fixed-in-fx-team]
I mentioned this on IRC but so it's not forgotten: the test depends on the order of elements - it's checking that pressing <tab> from the search field moves focus to the style checkbox. Since those got reordered, the test needs to be updated to match (could probably just add a shift modifier to the keypress at https://mxr.mozilla.org/mozilla-central/source/browser/devtools/styleinspector/test/browser_bug589375_keybindings.js#71)
(Assignee)

Comment 10

7 years ago
Posted patch patch with updated test (obsolete) — Splinter Review
Thanks Paul for the note!

I updated the test, and it's passing on try server now.
Attachment #611597 - Attachment is obsolete: true
Attachment #646084 - Flags: review?(jwalker)

Updated

7 years ago
Attachment #646084 - Flags: review?(jwalker) → review+
Heather, my guess it that the SynthetizeMouse call fails:
http://mxr.mozilla.org/mozilla-central/source/browser/devtools/styleinspector/test/browser_styleinspector_bug_672744_search_filter.js#51

The CSS change might be the problem here (s/5,5/2,2/ might solve the problem).
(Assignee)

Comment 14

7 years ago
(In reply to Paul Rouget [:paul] from comment #13)
> Heather, my guess it that the SynthetizeMouse call fails:
> http://mxr.mozilla.org/mozilla-central/source/browser/devtools/
> styleinspector/test/browser_styleinspector_bug_672744_search_filter.js#51
> 
> The CSS change might be the problem here (s/5,5/2,2/ might solve the
> problem).

It does look like the synthesizeMouse() call is failing. I tried 2,2 and 8,8 but the test still fails.
(Assignee)

Comment 15

7 years ago
Okay, sod switching the search box and check box for now.

This patch tweaks the computed table to take up less room and reduce horizontal scrolling, it:

* reduces the font size of property names.
* makes property names <td>s so names are broken up if they're too long.
* reduces some padding/margin between rows and after expanders.

It will still scroll if you make the sidebar too small, and it's not a complete overhaul of the UI, but I think it's a good fix for now.

Tests pass on try server.
Attachment #614005 - Attachment is obsolete: true
Attachment #646084 - Attachment is obsolete: true
Attachment #653662 - Flags: review?(paul)
Comment on attachment 653662 [details] [diff] [review]
tweak styles to lessen horizontal scrolling

Optional: maybe you want to remove some padding/margin around the twisty. Up to you.
Attachment #653662 - Flags: review?(paul) → review+

Updated

7 years ago
Whiteboard: [land-in-fx-team]
(Assignee)

Comment 17

7 years ago
(In reply to Paul Rouget [:paul] from comment #16)
> Comment on attachment 653662 [details] [diff] [review]
> tweak styles to lessen horizontal scrolling
> 
> Optional: maybe you want to remove some padding/margin around the twisty. Up
> to you.

That would be nice. -moz-apperance: treetwisty was too magic for me to figure out what was going on immediately, maybe a follow-up?
(In reply to Heather Arthur [:harth] from comment #17)
> maybe a follow-up?

Yep.

Let's land that.
https://hg.mozilla.org/integration/fx-team/rev/59b7b6ae1d1d
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/59b7b6ae1d1d
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18

Updated

7 years ago
Depends on: 802837

Updated

9 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.