Status

enhancement
P3
normal
RESOLVED FIXED
2 years ago
11 months ago

People

(Reporter: gl, Assigned: lockhart)

Tracking

(Blocks 1 bug)

unspecified
Firefox 55
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

2 years ago
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.
Reporter

Comment 1

2 years ago
4) We need to fix the font used in this panel.
Reporter

Updated

2 years ago
Assignee: nobody → lockhart
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Reporter

Updated

2 years ago
Attachment #8852673 - Flags: review?(gl)
Comment hidden (mozreview-request)
Reporter

Updated

2 years ago
Summary: Polish the grid inspector → Polish the layout panel

Comment 5

2 years ago
mozreview-review
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+
Reporter

Comment 6

2 years ago
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

2 years ago
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c3c2c81e6d7
Polish the grid inspector and layout tab. r=jdescottes

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7c3c2c81e6d7
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Reporter

Updated

2 years ago
Duplicate of this bug: 1359761

Updated

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