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)
DevTools
Inspector
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 | ||
Updated•6 years ago
|
Assignee: nobody → gl
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Reporter | ||
Comment 2•6 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Comment 6•6 years ago
|
||
mozreview-review |
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
Comment 8•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•