741576 - Style tweaks for Computed view

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Heather Arthur [:harth]]

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.

Comment 1

[Heather Arthur [:harth]]

Attachment: Reduce padding and put search box on left

Patch for proposed tweaks.

Comment 2

[Paul Rouget [:paul]]

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)

Comment 3

[Heather Arthur [:harth]]

Attachment: screenshot with tweaks and theme applied to toolbar

Here's a screenshot with those two classes applied too.

Comment 4

[Heather Arthur [:harth]]

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.

Comment 5

[Joe Walker [: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.

Comment 6

[Joe Walker [:jwalker]]

(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.

Comment 7

[Heather Arthur [:harth]]

http://hg.mozilla.org/integration/fx-team/rev/e90f0d780ca9

Comment 8

[Ed Morley [:emorley]]

Backed out for timeouts in browser_bug589375_keybindings.js:
https://tbpl.mozilla.org/?tree=Fx-Team&rev=e90f0d780ca9
https://tbpl.mozilla.org/php/getParsedLog.php?id=11470510&tree=Fx-Team

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

Comment 9

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

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)

Comment 10

[Heather Arthur [:harth]]

Attachment: patch with updated test

Thanks Paul for the note!

I updated the test, and it's passing on try server now.

Comment 11

[Heather Arthur [:harth]]

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

Comment 12

[Ed Morley [:emorley]]

Sorry backed out for Win7-only timeouts in browser_styleinspector_bug_672744_search_filter.js (and later tests):
https://tbpl.mozilla.org/?tree=Fx-Team&rev=6b2d5e37140e

eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=13892175&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=13898690&tree=Fx-Team

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

Comment 13

[Paul Rouget [:paul]]

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).

Comment 14

[Heather Arthur [:harth]]

(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.

Comment 15

[Heather Arthur [:harth]]

Attachment: tweak styles to lessen horizontal scrolling

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.

Comment 16

[Paul Rouget [: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.

Comment 17

[Heather Arthur [:harth]]

(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?

Comment 18

[Paul Rouget [:paul]]

(In reply to Heather Arthur [:harth] from comment #17)
> maybe a follow-up?

Yep.

Let's land that.

Comment 19

[Paul Rouget [:paul]]

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

Comment 20

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/mozilla-central/rev/59b7b6ae1d1d