Closed Bug 1464442 Opened 6 years ago Closed 6 years ago

Make the display-type badge in the markup-view say "subgrid" if the element is a subgrid

Categories

(DevTools :: Inspector, enhancement, P2)

enhancement

Tracking

(firefox62 fixed)

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: pbro, Assigned: gl)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

If a grid is a subgrid, then its display-type badge in the markup-view should say subgrid instead of grid.
subgrid is *not* a correct CSS display type, but it's just more useful this way I believe.

Steps:
- have a build of Firefox that supports subgrids (might be in nighly soon, but ask me if not)
- open https://codepen.io/anon/pen/vjqQJP?editors=1100
- open the inspector and find the <main> element in the page

I believe if an element is a grid and has one of its grid-template-* properties defined as subgrid, then we should consider the element as a subgrid.
Assignee: nobody → gl
Status: NEW → ASSIGNED
Comment on attachment 8980822 [details]
Bug 1464442 - Make the display-type badge in the markup-view say "subgrid" if the element is a subgrid.

https://reviewboard.mozilla.org/r/247002/#review253246

A couple of suggestions to do this differently.
Also, subgrids are currently behind a flag and although the plan seems to be shipping in 63, I don't know that that's a final plan. So you'll need to set the flag to true in the test.

::: devtools/client/inspector/markup/views/element-editor.js:279
(Diff revision 1)
> -    this.displayNode.textContent = this.node.displayType;
> -    this.displayNode.dataset.display = showDisplayNode ? this.node.displayType : "";
> +    let displayType = this.node.isSubgrid ? "subgrid" : this.node.displayType;
> +    this.displayNode.textContent = displayType;
> +    this.displayNode.dataset.display = displayType;
>      this.displayNode.style.display = showDisplayNode ? "inline-block" : "none";
> -    this.displayNode.title = showDisplayNode ? DISPLAY_TYPES[this.node.displayType] : "";
> +    this.displayNode.title = DISPLAY_TYPES[displayType];

I think I'd prefer to avoid all of these changes by moving the logic actor-side instead.
The node.displayType property could, very well, just be `subgrid`. So, none of the client-side changes (apart from adding the new localized string) would be needed.
The only change would reside in the node actor's displayType getter.
Now, subgrid is *not* really a displayType, I get it, but I don't think it's a big problem making this getter return it still.
In fact it could return subgrid and inline-subgrid.

::: devtools/client/locales/en-US/inspector.properties:61
(Diff revision 1)
>  # LOCALIZATION NOTE (markupView.display.inlineGrid.tooltiptext)
>  # Used in a tooltip that appears when the user hovers over the display type button in
>  # the markup view.
>  markupView.display.inlineGrid.tooltiptext=This element behaves like an inline element and lays out its content according to the grid model.
>  
> +markupView.display.subgrid.tooltiptiptext=If the parent element has display:grid, the element itself and its content are laid out according to the grid model.

Please add a `LOCALIZATION NOTE` comment for this new key.
Also, some suggestion for rephrasing:

This element lays out its content according to the grid model but defers the definition of its rows and/or columns to its parent grid container.
Attachment #8980822 - Flags: review?(pbrosset)
Comment on attachment 8980822 [details]
Bug 1464442 - Make the display-type badge in the markup-view say "subgrid" if the element is a subgrid.

https://reviewboard.mozilla.org/r/247002/#review255372
Attachment #8980822 - Flags: review?(pbrosset) → review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/95cb0fb8017a
Make the display-type badge in the markup-view say "subgrid" if the element is a subgrid. r=pbro
https://hg.mozilla.org/mozilla-central/rev/95cb0fb8017a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: