1468402 - Indent subgrids in the grid inspector sidebar

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Patrick Brosset <:pbro>]

Today in a design discussion about subgrid we discussed about this potential enhancement.

Instead of showing all the grids on a page in a flat list like we do today:

□ div#parent ○
□ div.wrapper ○
□ div.some-child ○
□ div.some-child ○

We want to show the parent-child relationship between parent grids and subgrids.
So assuming that the 2 .some-child elements are defined as subgrids of the .wrapper element, we would show the list as such:

□ div#parent ○
□ div.wrapper ○
   □ div.some-child ○
   □ div.some-child ○

On top of this, selecting a parent grid would also select its subgrids:

□ div#parent ○
☑ div.wrapper ○
   ☑ div.some-child ○
   ☑ div.some-child ○

Obviously you'd be able to unselect any grid. The auto-selection would just be the default:

□ div#parent ○
□ div.wrapper ○
   ☑ div.some-child ○
   □ div.some-child ○

This therefore depends on bug 1317102.

Comment 1

[Sebastian Zartner [:sebo]]

From a short IRC chat:

> Sebo  Ok, according to the spec. nesting is allowed. See https://drafts.csswg.org/css-grid-2/#example-3ffc0434. So, the indentation needs to take that into account.
> Sebo	Though highlighting all individual subgrid items may be overkill, visually and regarding performance.
> Sebo	So, maybe only highlight the areas they span like in the spec. examples, e.g. https://drafts.csswg.org/css-grid-2/images/subgrid-gap-normal.png.
> pbro	Sebo: yes, something like this would be good. Something that highlight the subgrid, and gives an indication that there's a parent cell/area around it that's coming from a parent grid, and some indication that its rows or columns relate to that of the parent

Sebastian

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1468402 - Part 2: Indent the subgrids in the grid list. r=pbro

Comment 3

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1468402 - Part 1: Add isSubgrid to the grid actor form and getParentGridNode to the walker. r=pbro

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1468402 - Part 3: Add test for subgrids in the grid list. r=pbro

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/567911d83bda
Part 1: Add isSubgrid and subgridParentNode to the grid actor form. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/63f40ac370c6
Part 2: Indent the subgrids in the grid list. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/6685fd957302
Part 3: Add test for subgrids in the grid list. r=pbro

Comment 6

[Razvan Maries]

Backed out for causing perma fails on browser_grids_grid-list-subgrids.js

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=6685fd95730257d69b3e9169a1d84c7336a8ce74&selectedJob=246154194

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=246154194&repo=mozilla-inbound&lineNumber=1907

[task 2019-05-13T16:53:07.112Z] 16:53:07 INFO - TEST-START | devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js
[task 2019-05-13T16:53:10.301Z] 16:53:10 INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js | One grid container is listed. - Got 5, expected 1
[task 2019-05-13T16:53:10.301Z] 16:53:10 INFO - Stack trace:
[task 2019-05-13T16:53:10.301Z] 16:53:10 INFO - chrome://mochikit/content/browser-test.js:test_is:1325
[task 2019-05-13T16:53:10.302Z] 16:53:10 INFO - chrome://mochitests/content/browser/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js:null:23
[task 2019-05-13T16:53:10.302Z] 16:53:10 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1116
[task 2019-05-13T16:53:10.302Z] 16:53:10 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1144
[task 2019-05-13T16:53:10.302Z] 16:53:10 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1005
[task 2019-05-13T16:53:10.302Z] 16:53:10 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-05-13T16:53:10.303Z] 16:53:10 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-05-13T16:53:10.303Z] 16:53:10 INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js | Got the correct number of subgrids in div.container - Got 0, expected 2
[task 2019-05-13T16:53:10.303Z] 16:53:10 INFO - Stack trace:
[task 2019-05-13T16:53:10.304Z] 16:53:10 INFO - chrome://mochikit/content/browser-test.js:test_is:1325
[task 2019-05-13T16:53:10.304Z] 16:53:10 INFO - chrome://mochitests/content/browser/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js:null:24
[task 2019-05-13T16:53:10.304Z] 16:53:10 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1116
[task 2019-05-13T16:53:10.304Z] 16:53:10 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1144
[task 2019-05-13T16:53:10.304Z] 16:53:10 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1005
[task 2019-05-13T16:53:10.305Z] 16:53:10 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-05-13T16:53:10.305Z] 16:53:10 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-05-13T16:53:10.305Z] 16:53:10 INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js:60 - TypeError: gridListEl is null
[task 2019-05-13T16:53:10.305Z] 16:53:10 INFO - Stack trace:
[task 2019-05-13T16:53:10.306Z] 16:53:10 INFO - getGridItemElements@chrome://mochitests/content/browser/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js:60:11
[task 2019-05-13T16:53:10.306Z] 16:53:10 INFO - @chrome://mochitests/content/browser/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js:26:6
[task 2019-05-13T16:53:10.306Z] 16:53:10 INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1116:34
[task 2019-05-13T16:53:10.306Z] 16:53:10 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:1144:12
[task 2019-05-13T16:53:10.306Z] 16:53:10 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:1005:14
[task 2019-05-13T16:53:10.307Z] 16:53:10 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:803:59
[task 2019-05-13T16:53:10.307Z] 16:53:10 INFO - Leaving test bound
[task 2019-05-13T16:53:10.307Z] 16:53:10 INFO - GECKO(1099) | console.warn: "Error while detaching the thread front: 'detach' request packet to 'server1.conn13.child1/context18' can't be sent as the connection is closed."
[task 2019-05-13T16:53:10.593Z] 16:53:10 INFO - Removing tab.
[task 2019-05-13T16:53:10.595Z] 16:53:10 INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2019-05-13T16:53:10.623Z] 16:53:10 INFO - Got event: 'TabClose' on [object XULElement].
[task 2019-05-13T16:53:10.639Z] 16:53:10 INFO - Tab removed and finished closing
[task 2019-05-13T16:53:10.687Z] 16:53:10 INFO - GECKO(1099) | MEMORY STAT | vsize 20975234MB | residentFast 1284MB
[task 2019-05-13T16:53:10.687Z] 16:53:10 INFO - TEST-OK | devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js | took 3580ms
[task 2019-05-13T16:53:10.747Z] 16:53:10 INFO - checking window state

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/0647eae6a6e6abd91ebd206b2db15a6766d2d3ef

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

Clearing needinfo, already fixing this up.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/11edf789501b
Part 1: Add isSubgrid to the grid actor form and getParentGridNode to the walker. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/3431614b512a
Part 2: Indent the subgrids in the grid list. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/a06aa97ec04f
Part 3: Add test for subgrids in the grid list. r=pbro

Comment 9

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/11edf789501b
https://hg.mozilla.org/mozilla-central/rev/3431614b512a
https://hg.mozilla.org/mozilla-central/rev/a06aa97ec04f