1349691 - Polish the layout panel

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Gabriel Luong [:gl] (ΦωΦ)]

In this bug, we need to solve a couple of layout issues with the grid inspector. 
1) We need to add some padding around the grid panel
2) We need to align the checkbox items between the list of grid container and the highlighter settings
3) The individual GridItems with the color picker overflows poorly when there is a small width.

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

4) We need to fix the font used in this panel.

Comment 2

[Stanford Lockhart [:lockhart]]

Attachment: Bug 1349691: Polish the grid inspector and layout tab.

Review commit: https://reviewboard.mozilla.org/r/124858/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/124858/

Comment 3

[Stanford Lockhart [:lockhart]]

Comment on attachment 8852673 [details]
Bug 1349691: Polish the grid inspector and layout tab.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124858/diff/1-2/

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1349691 - Polish the layout panel.

Review commit: https://reviewboard.mozilla.org/r/130930/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/130930/

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8858934 [details]
Bug 1349691 - Polish the layout panel.

https://reviewboard.mozilla.org/r/130930/#review133630

LGTM.
I guess this should not have any impact on existing tests?

For a potential follow up:
When the panel gets small enough so that the element stack on top of each other, the centering feels weird. 
I think the display would be slightly better if they were aligned-start.

::: devtools/client/inspector/grids/components/GridDisplaySettings.js:60
(Diff revision 1)
>        ),
>        dom.ul(
>          {},
>          dom.li(
> -          {},
> +          {
> +            className: "grid-settings-item"

nit: comma

::: devtools/client/inspector/grids/components/GridDisplaySettings.js:76
(Diff revision 1)
>              getStr("layout.extendGridLinesInfinitely")
>            )
>          ),
>          dom.li(
> -          {},
> +          {
> +            className: "grid-settings-item"

nit: comma

::: devtools/client/inspector/layout/components/App.js:55
(Diff revision 1)
>  
>    render() {
>      return dom.div(
>        {
>          id: "layout-container",
> +        className: "devtools-monospace"

nit: add trailing comma

::: devtools/client/themes/layout.css:27
(Diff revision 1)
>  }
>  
>  .grid-container > span {
> -  font-weight: bold;
> -  margin-bottom: 3px;
> +  font-weight: 600;
> +  margin-bottom: 5px;
> +  pointer-events: none;

is this needed / doing anything?

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

We set the pointer-events: none because otherwise we get the "I" cursor on mouseover of the "Overlay Grids" and "Grid Display Setting" headers

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c3c2c81e6d7
Polish the grid inspector and layout tab. r=jdescottes

Comment 8

[Carsten Book [:Tomcat]]

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