Polish the layout panel

RESOLVED FIXED in Firefox 55

Status

P3
normal
RESOLVED FIXED
2 years ago
9 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
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
(Reporter)

Updated

2 years ago
Duplicate of this bug: 1359761

Updated

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