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
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: