Closed Bug 1349691 Opened 5 years ago Closed 5 years ago

Polish the layout panel

Categories

(DevTools :: Inspector, enhancement, P3)

enhancement

Tracking

(firefox55 fixed)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: gl, Assigned: lockhart)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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.
4) We need to fix the font used in this panel.
Assignee: nobody → lockhart
Status: NEW → ASSIGNED
Attachment #8852673 - Flags: review?(gl)
Summary: Polish the grid inspector → Polish the layout panel
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?
Attachment #8858934 - Flags: review?(jdescottes) → review+
We set the pointer-events: none because otherwise we get the "I" cursor on mouseover of the "Overlay Grids" and "Grid Display Setting" headers
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c3c2c81e6d7
Polish the grid inspector and layout tab. r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/7c3c2c81e6d7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Duplicate of this bug: 1359761
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.