Display a list of grid container elements in the css grid layout panel

RESOLVED FIXED in Firefox 53

Status

DevTools
Inspector
P3
normal
RESOLVED FIXED
2 years ago
a month ago

People

(Reporter: gl, Assigned: gl)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 53
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(7 attachments, 22 obsolete attachments)

3.66 KB, patch
pbro
: review+
Details | Diff | Splinter Review
4.67 KB, patch
Details | Diff | Splinter Review
5.23 KB, patch
Details | Diff | Splinter Review
8.87 KB, patch
Details | Diff | Splinter Review
3.77 KB, patch
jryans
: review+
Details | Diff | Splinter Review
35.71 KB, patch
Details | Diff | Splinter Review
9.47 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

2 years ago
Adds a checkbox list of the grid containers for the current page in the css grid layout and creates a new css grid highlighter when a grid container is checked. We won't worry about having different colours for each highlighter for this bug.
(Assignee)

Updated

2 years ago
Depends on: 1308257
(Assignee)

Updated

2 years ago
Assignee: nobody → gl
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
Created attachment 8809725 [details] [diff] [review]
1308260.patch [WIP1]
(Assignee)

Comment 2

2 years ago
Created attachment 8810294 [details] [diff] [review]
Part 1: Emit an event with the new grid layout actors on page navigations from the LayoutActor [1.0]

Breaking down my current big WIP patch into reviewable and landable pieces. Patrick, can you comment on what is the best way to go about filtering out the reflows? I saw that for nodes we have a wasDisplayed attribute that we check when the display visibility is changed. Should we be doing the same thing here when checking for reflows?
Attachment #8810294 - Flags: review?(pbrosset)
Comment on attachment 8810294 [details] [diff] [review]
Part 1: Emit an event with the new grid layout actors on page navigations from the LayoutActor [1.0]

Review of attachment 8810294 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/server/actors/layout.js
@@ +72,5 @@
>      this.tabActor = tabActor;
>      this.walker = walker;
> +
> +    this.onNavigate = this.onNavigate.bind(this);
> +    this.onReflows = this.onReflows.bind(this);

This callback isn't defined or used in the code that you changed in this patch.

@@ +84,3 @@
>      this.tabActor = null;
>      this.walker = null;
> +    this.layoutChangeObserver = null;

this.layoutChangeObserver does not seem to exist in this file. Either this needs to be removed, or you forgot to import and use it.

@@ +131,5 @@
>        grids = [...grids, ...this.getGrids(document.documentElement)];
>      }
>      return grids;
>    },
> +  onNavigate: function () {

Splinter says that there's no new line between the getAllGrids method and the onNavigate method. I'm sure there is one because this already happened in other patches I reviewed for you in the past, but maybe your editor is configured to enter new lines in a way that splinter doesn't recognizes them.
We recently made a change to our .eslintrc config to enforce unix-style line breaks: http://searchfox.org/mozilla-central/source/devtools/.eslintrc.js#148-149
Attachment #8810294 - Flags: review?(pbrosset)
(Assignee)

Comment 4

2 years ago
Created attachment 8810345 [details] [diff] [review]
Part 1: Emit an event with the new grid layout actors on page navigations from the LayoutActor [2.0]
Attachment #8810294 - Attachment is obsolete: true
Attachment #8810345 - Flags: review?(pbrosset)
Attachment #8810345 - Flags: review?(pbrosset) → review+
(Assignee)

Updated

2 years ago
Keywords: leave-open
(Assignee)

Comment 5

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/fef1eecca5bf6e3afa3b379974c4fe14c0342289
Bug 1308260 - Part 1: Emit an event with the new grid layout actors on page navigations from the LayoutActor. r=pbro
(Assignee)

Comment 6

2 years ago
Patrick, can you comment on what is the best way to go about filtering out the reflows? I saw that for nodes we have a wasDisplayed attribute that we check when the display visibility is changed. Should we be doing the same thing here when checking for reflows?
Flags: needinfo?(pbrosset)
(In reply to Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ) from comment #6)
> Patrick, can you comment on what is the best way to go about filtering out
> the reflows? I saw that for nodes we have a wasDisplayed attribute that we
> check when the display visibility is changed. Should we be doing the same
> thing here when checking for reflows?
There is a important difference between the 2 things. In the Walker, we only care about the nodes we know about to check whether their visibility has changed on reflow, because only the nodes we know about in the Walker are visible in the inspector front-end.

In contrast, the grid layout sidebar panel will display a list of nodes that may or may not be known yet. So we have no choice but to actually walk the DOM to find out the whole list of grids.

So, this is down to a performance optimization problem. Here, what we're saying is that we want to update the complete list of grids anytime that list changes in the page.

Our immediate solution is to listen to reflows (of which there could be many), and when that happens, walk the whole DOM (which could be deep), and on each node run getGridFragments() to test if a grid exist (which could be slow).

For the first problem (many reflows), I think we should debounce or throttle execution of the callback to avoid unnecessary work.

For the second problem (deep DOM), I don't see any way around this. I'm assuming that the treeWalker we use is the fastest thing we can use, and I don't see any way of filtering here. Obviously, if a node is inline or hidden, there's no need to walk down within it, but getting the node's computed style might make things slower than just walking its descendants. We should test this though.
One small optimization though, when calling getDocumentWalker, we should pass a second argument set to SHOW_ELEMENT from dom-node-filter-constants.js so at least the walker only goes into element nodes.

For the last problem (calling getGridFragments), my assumption so far was that this was really fast. The platform code seems to firs test the type of the first frame of the element. So I don't think there's any way to make this faster.
Flags: needinfo?(pbrosset)
(Assignee)

Comment 9

2 years ago
Created attachment 8811785 [details] [diff] [review]
Part 2: Handle grid layout changes and layout sidebar selection, and prepares the grid data to be disaptched. [1.0]
Attachment #8811785 - Flags: review?(pbrosset)
(Assignee)

Comment 10

2 years ago
Created attachment 8811787 [details] [diff] [review]
Part 3: Dispatch the new grid data and update the store with the new grid state.
Attachment #8811787 - Flags: review?(jryans)
(Assignee)

Comment 11

2 years ago
(In reply to Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ) from comment #10)
> Created attachment 8811787 [details] [diff] [review]
> Part 3: Dispatch the new grid data and update the store with the new grid
> state.

@jryans, main concern here is the duplication of enum.js from rdm in devtools/client/inspector/layout/utils. 
Otherwise, it is a very simple redux state update that can be reviewed by someone else if you don't have time. Feel free to pass the review to pbro or zer0 if that is the case.
(Assignee)

Comment 12

2 years ago
Created attachment 8811800 [details] [diff] [review]
Part 4: Pass the grids props into the layout components and only display the layout-no-grid element when no grids are present. [1.0]
Attachment #8811800 - Flags: review?(jryans)
Comment on attachment 8811787 [details] [diff] [review]
Part 3: Dispatch the new grid data and update the store with the new grid state.

Review of attachment 8811787 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/inspector/layout/utils/enum.js
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public

I'd say move the RDM one to devtools/client/shared/enum.js.

Seems unnecessary to have two copies.
Attachment #8811787 - Flags: review?(jryans) → review+
Comment on attachment 8811800 [details] [diff] [review]
Part 4: Pass the grids props into the layout components and only display the layout-no-grid element when no grids are present. [1.0]

Review of attachment 8811800 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/inspector/layout/components/App.js
@@ -10,1 @@
>  const App = createClass({

Consider adding

mixins: [ addons.PureRenderMixin ],

to all components that only change when props change to improve perf.
Attachment #8811800 - Flags: review?(jryans) → review+
(Assignee)

Comment 15

2 years ago
Created attachment 8812062 [details] [diff] [review]
Part 3: Dispatch the new grid data and update the store with the new grid state. [2.0]
Attachment #8811787 - Attachment is obsolete: true
Attachment #8812062 - Flags: review+
(Assignee)

Comment 16

2 years ago
Created attachment 8812063 [details] [diff] [review]
Part 2: Handle grid layout changes and layout sidebar selection, and prepares the grid data to be dispatched. [2.0]
Attachment #8811785 - Attachment is obsolete: true
Attachment #8811785 - Flags: review?(pbrosset)
Attachment #8812063 - Flags: review?(pbrosset)
(Assignee)

Updated

2 years ago
Attachment #8810345 - Flags: checkin+
(Assignee)

Comment 17

2 years ago
Created attachment 8812065 [details] [diff] [review]
Part 4: Pass the grids props into the layout components and only display the layout-no-grid element when no grids are present. [2.0]
Attachment #8811800 - Attachment is obsolete: true
Attachment #8812065 - Flags: review+
(Assignee)

Comment 18

2 years ago
Created attachment 8812089 [details] [diff] [review]
Part 4: Pass the grids props into the layout components and only display the layout-no-grid element when no grids are present. [3.0]
Attachment #8812065 - Attachment is obsolete: true
Attachment #8812089 - Flags: review+
Comment on attachment 8812063 [details] [diff] [review]
Part 2: Handle grid layout changes and layout sidebar selection, and prepares the grid data to be dispatched. [2.0]

Review of attachment 8812063 [details] [diff] [review]:
-----------------------------------------------------------------

Generally OK with this patch, but would like to maybe move the layout inspector construction to init.

::: devtools/client/inspector/layout/layout.js
@@ +101,5 @@
> +        console.error(e);
> +      }
> +
> +      grids.push({
> +        id: i,

Can you explain why an ID is needed here? Maybe that's for rendering a list with React later?
If it's not needed then you could use
for (let grid of gridFronts) above instead of defining i

@@ +129,5 @@
> +   * visible. Otherwise, stop listening for grid layout changes.
> +   */
> +  onSidebarSelect: Task.async(function* () {
> +    if (!this.layoutInspector) {
> +      this.layoutInspector = yield this.inspector.walker.getLayoutInspector();

I find it a little odd that the layout inspector would be created in this function. It shouldn't really have to deal with this. This is something init should really be doing instead.
I think you should move this to init, and after its done, call onSidebarSelect instead of calling it from the constructor.
Attachment #8812063 - Flags: review?(pbrosset)
(Assignee)

Comment 20

2 years ago
Created attachment 8812110 [details] [diff] [review]
Part 2: Handle grid layout changes and layout sidebar selection, and prepares the grid data to be dispatched. [3.0]
Attachment #8812063 - Attachment is obsolete: true
Attachment #8812110 - Flags: review?(pbrosset)
(Assignee)

Updated

2 years ago
Attachment #8812110 - Flags: review?(pbrosset)
(Assignee)

Comment 21

2 years ago
Created attachment 8812113 [details] [diff] [review]
Part 2: Handle grid layout changes and layout sidebar selection, and prepares the grid data to be dispatched. [4.0]
Attachment #8812110 - Attachment is obsolete: true
Attachment #8812113 - Flags: review?(pbrosset)
Comment on attachment 8812113 [details] [diff] [review]
Part 2: Handle grid layout changes and layout sidebar selection, and prepares the grid data to be dispatched. [4.0]

Review of attachment 8812113 [details] [diff] [review]:
-----------------------------------------------------------------

I have a few more comments, but don't need to re-review once you've addressed them.

::: devtools/client/inspector/layout/layout.js
@@ +32,5 @@
>  
>  LayoutView.prototype = {
>  
> +  init: Task.async(function* () {
> +    this.layoutInspector = yield this.inspector.walker.getLayoutInspector();

Since this is async, there might be cases where destroy was called before this resolved.
So below that line, you have no guaranty that the LayoutView (or the toolbox for that matter) still exists.
So after this line, you should test this.inspector and if falsy, then bail out.

@@ +60,1 @@
>      this.inspector = null;

You also need to nullify this.layoutInspector here.

@@ +88,5 @@
> +    // Get all the GridFront from the server if no gridFronts were provided.
> +    if (!gridFronts) {
> +      gridFronts = yield this.layoutInspector.getAllGrids(this.walker.rootNode);
> +    }
> +

Unfortunately, the fact that many things are async here (and in our tools in general) leads to races hiding in every corner. Here, refresh could be called as a result of a grid change on the server, but that grid change could happen while the toolbox is being closed. So there is a possibility where you're reaching this line but the panel has been destroyed already. So you'll also need to bail out here.

@@ +98,5 @@
> +      // Get the NodeFront of the grid container element.
> +      try {
> +        nodeFront = yield this.walker.getNodeFromActor(grid.actorID, ["containerEl"]);
> +      } catch (e) {
> +        console.error(e);

Why catch the error if all you're doing is raising it again with console.error?

Is there a case where calling getNodeFromActor may fail but we do want to continue?
If there is, then you should have:
try { ... } catch (e) { continue; }
instead, so you just skip this grid.

If there isn't, then just remove the try/catch altogether.
Attachment #8812113 - Flags: review?(pbrosset) → review+
(Assignee)

Comment 23

2 years ago
Created attachment 8812837 [details] [diff] [review]
Part 2: Handle grid layout changes and layout sidebar selection, and prepares the grid data to be dispatched. [5.0]
Attachment #8812113 - Attachment is obsolete: true
Attachment #8812837 - Flags: review+
(Assignee)

Comment 24

2 years ago
Created attachment 8812838 [details] [diff] [review]
Part 3: Dispatch the new grid data and update the store with the new grid state. [3.0]
Attachment #8812062 - Attachment is obsolete: true
Attachment #8812838 - Flags: review+
(Assignee)

Comment 25

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8423ee15c2f0d080a152454082863d8246d33adc
Bug 1308260 - Part 2: Handle grid layout changes and layout sidebar selection, and prepares the grid data to be dispatched. r=pbro

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca96b9d71ca33da24f09310c8df84125dbacd1b9
Bug 1308260 - Part 3: Dispatch the new grid data and update the store with the new grid state. r=jryans

https://hg.mozilla.org/integration/mozilla-inbound/rev/b5541adf77a4eeb1f237828e1bd6b8076bafd1f3
Bug 1308260 - Part 4: Pass the grids props into the layout components and only display the layout-no-grid element when no grids are present. r=jryans
(Assignee)

Comment 27

2 years ago
Created attachment 8814171 [details] [diff] [review]
1308260-5.patch [WIP1]
Attachment #8809725 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Depends on: 1318835
(Assignee)

Comment 28

2 years ago
Created attachment 8814306 [details] [diff] [review]
1308260.patch [WIP2]
Attachment #8814171 - Attachment is obsolete: true
(Assignee)

Comment 29

2 years ago
Created attachment 8814323 [details] [diff] [review]
1308260.patch [WIP3]
Attachment #8814306 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Blocks: 1308263
(Assignee)

Comment 30

2 years ago
Created attachment 8814524 [details] [diff] [review]
Part 5: Add a highlighted state to the grid state and a method to update it. [1.0]
Attachment #8814524 - Flags: review?(jryans)
(Assignee)

Comment 31

2 years ago
Created attachment 8814529 [details] [diff] [review]
Part 6: Toggling the grid highlighter should emit the node that the grid highlighter was shown or hidden for and update the grid's highlighted state. [1.0]

Part 6: Toggling the grid highlighter should emit the node that the grid highlighter was shown or hidden for and update the grid's highlighted state.

- Rearranged the order of the functions in the highlighters-overlay.js.
- Rewrote the grid highlighter functions using Task.
- _toggleRuleViewGridIcon does not need to be called unless the actual inspector selection is the actual grid container element since we would be able to highlight different grid containers from the LayoutView.
- Separated the grid highlighter show and hide functions so it can be called individually in different situations such as multiple overlay or from the text property editor edits.
Attachment #8814529 - Flags: review?(pbrosset)
(Assignee)

Updated

2 years ago
Attachment #8814323 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8812089 - Flags: checkin+
(Assignee)

Updated

2 years ago
Attachment #8812837 - Flags: checkin+
(Assignee)

Updated

2 years ago
Attachment #8812838 - Flags: checkin+
(Assignee)

Updated

2 years ago
Attachment #8814529 - Flags: review?(pbrosset)
(Assignee)

Comment 34

2 years ago
Created attachment 8814602 [details] [diff] [review]
Part 6: Toggling the grid highlighter should emit the node that the grid highlighter was shown or hidden for and update the grid's highlighted state. [2.0]
Attachment #8814529 - Attachment is obsolete: true
Attachment #8814602 - Flags: review?(pbrosset)
(Assignee)

Comment 35

2 years ago
Created attachment 8814610 [details] [diff] [review]
1308260-7.patch [WIP1]
(Assignee)

Comment 36

2 years ago
Created attachment 8814614 [details] [diff] [review]
Part 6: Toggling the grid highlighter should emit the node that the grid highlighter was shown or hidden for and update the grid's highlighted state. [3.0]
Attachment #8814602 - Attachment is obsolete: true
Attachment #8814602 - Flags: review?(pbrosset)
Attachment #8814614 - Flags: review?(pbrosset)
(Assignee)

Comment 38

2 years ago
Created attachment 8814656 [details] [diff] [review]
Part 5: Add a highlighted state to the grid state and a method to update it. [2.0]
Attachment #8814524 - Attachment is obsolete: true
Attachment #8814524 - Flags: review?(jryans)
Attachment #8814656 - Flags: review?(jryans)
(Assignee)

Comment 39

2 years ago
Created attachment 8814657 [details] [diff] [review]
Part 6: Toggling the grid highlighter should emit the node that the grid highlighter was shown or hidden for and update the grid's highlighted state. [4.0]
Attachment #8814614 - Attachment is obsolete: true
Attachment #8814614 - Flags: review?(pbrosset)
Attachment #8814657 - Flags: review?(pbrosset)
(Assignee)

Comment 40

2 years ago
Created attachment 8814706 [details] [diff] [review]
Part 7: Display a list of grid container elements and checkbox to toggle the grid highlighter for each grid in the CSS grid layout panel. [1.0]
Attachment #8814610 - Attachment is obsolete: true
Attachment #8814706 - Flags: review?(jryans)
(Assignee)

Updated

2 years ago
Attachment #8814706 - Attachment description: Part 7: Display a list of grid container elements and checkbox to toggle the grid highlighter for each grid in the CSS grid layout panel. → Part 7: Display a list of grid container elements and checkbox to toggle the grid highlighter for each grid in the CSS grid layout panel. [1.0]
(Assignee)

Comment 41

2 years ago
Created attachment 8814719 [details] [diff] [review]
Part 7: Display a list of grid container elements and checkbox to toggle the grid highlighter for each grid in the CSS grid layout panel. [2.0]
Attachment #8814706 - Attachment is obsolete: true
Attachment #8814706 - Flags: review?(jryans)
Attachment #8814719 - Flags: review?(jryans)
(Assignee)

Comment 42

2 years ago
Created attachment 8814725 [details] [diff] [review]
Part 6: Toggling the grid highlighter should emit the node that the grid highlighter was shown or hidden for and update the grid's highlighted state. [5.0]
Attachment #8814657 - Attachment is obsolete: true
Attachment #8814657 - Flags: review?(pbrosset)
Attachment #8814725 - Flags: review?(pbrosset)
Comment on attachment 8814725 [details] [diff] [review]
Part 6: Toggling the grid highlighter should emit the node that the grid highlighter was shown or hidden for and update the grid's highlighted state. [5.0]

Review of attachment 8814725 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/inspector/layout/layout.js
@@ +151,5 @@
> +   *         The NodeFront of the grid container element for which the grid highlighter
> +   *         is shown for.
> +   */
> +  onHighlighterChange(event, nodeFront) {
> +    let highlighted = event === "grid-highlighter-shown" ? true : false;

The === already evaluates to a boolean, so this can be simply:

let highlighted = event === "grid-hgihighter-shown";

::: devtools/client/inspector/shared/highlighters-overlay.js
@@ +159,5 @@
> +   * @param  {String} type
> +   *         The highlighter type. One of this.highlighters.
> +   * @return {Promise} that resolves to the highlighter
> +   */
> +  _getHighlighter: function (type) {

While you're at it, you could also make this one a Task.async.
Attachment #8814725 - Flags: review?(pbrosset) → review+
Comment on attachment 8814719 [details] [diff] [review]
Part 7: Display a list of grid container elements and checkbox to toggle the grid highlighter for each grid in the CSS grid layout panel. [2.0]

Review of attachment 8814719 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/inspector/layout/components/GridList.js
@@ +54,5 @@
> +          let gridName = displayName;
> +
> +          let idIndex = attributes.findIndex(({ name }) => name === "id");
> +          if (idIndex > -1 && attributes[idIndex].value) {
> +            gridName += "#" + attributes[idIndex].value;

Drive-by comment (as discussed on IRC): this is duplicated code from a few different places in DevTools:
- the markup-view has some code to preview DOM nodes
- the new console uses a Rep for this: devtools\client\shared\components\reps\element-node.js
- the animationinspector has a reusable component for this too: devtools\client\inspector\shared\dom-node-preview.js
- and the variable-views also has some code for this
So, while this is fine as a stopgap to start landing these patches, I'd like one of the your next parts or bugs to be about reusing one of these shareable components.
We should really be using the Rep everywhere we can. It's a good fit here because it's React.
Attachment #8814656 - Flags: review?(jryans) → review+
Comment on attachment 8814719 [details] [diff] [review]
Part 7: Display a list of grid container elements and checkbox to toggle the grid highlighter for each grid in the CSS grid layout panel. [2.0]

Review of attachment 8814719 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but please file a follow up for :pbro's comments.
Attachment #8814719 - Flags: review?(jryans) → review+
Comment on attachment 8814719 [details] [diff] [review]
Part 7: Display a list of grid container elements and checkbox to toggle the grid highlighter for each grid in the CSS grid layout panel. [2.0]

Review of attachment 8814719 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but please file a follow up for :pbro's comments.

::: devtools/client/inspector/layout/components/GridList.js
@@ +63,5 @@
> +            gridName += "." + attributes[classIndex].value.split(" ").join(".");
> +          }
> +
> +          return dom.li(
> +            {},

You should set a `key` attribute for better vdom diffing performance with lists.

https://facebook.github.io/react/docs/lists-and-keys.html
(Assignee)

Comment 47

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c54418718a5a6f41f60992a896e301cf1664270d
Bug 1308260 - Part 5: Add a highlighted state to the grid state and a method to update it. r=jryans

Comment 48

2 years ago
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c54418718a5a
Part 5: Add a highlighted state to the grid state and a method to update it. r=jryans
(Assignee)

Comment 49

2 years ago
Created attachment 8815204 [details] [diff] [review]
Part 6: Toggling the grid highlighter should emit the node that the grid highlighter was shown or hidden for and update the grid's highlighted state. [6.0]
Attachment #8814725 - Attachment is obsolete: true
Attachment #8815204 - Flags: review+
(Assignee)

Comment 50

2 years ago
Created attachment 8815228 [details] [diff] [review]
Part 7: Display a list of grid container elements and checkbox to toggle the grid highlighter for each grid in the CSS grid layout panel. [3.0]
Attachment #8814719 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Keywords: leave-open
(Assignee)

Comment 51

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/892a0318ffbe197aa1342c93b5ff78715fab2a7b
Bug 1308260 - Part 6: Toggling the grid highlighter should emit the node that the grid highlighter was shown or hidden for and update the grid's highlighted state. r=pbro

https://hg.mozilla.org/integration/mozilla-inbound/rev/7055689115a4ba815bdfca60af0cf1958939001b
Bug 1308260 - Part 7: Display a list of grid container elements and checkbox to toggle the grid highlighter for each grid in the CSS grid layout panel. r=jryans
(Assignee)

Updated

2 years ago
Attachment #8815228 - Flags: review+

Comment 52

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c54418718a5a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53

Updated

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