We need to implement mouseover interactions for the grid outline. This bug should: 1. Figure out the row/col of the mouseover using the grid fragment data. 2. Figure out if there is a grid area, if yes then show the grid highlight.

Comment on attachment 8840275 [details] Bug 1337235 - Mouseover interaction for grid outline. https://reviewboard.mozilla.org/r/114762/#review116294

Comment on attachment 8840322 [details] Bug1337235 - Mouseover interactions for grid outline. https://reviewboard.mozilla.org/r/114828/#review116972 ::: devtools/client/inspector/layout/components/GridOutline.js:63 (Diff revision 1) > } > ); > }, > > /** > + * getGridAreaName - Determines if grid cell is part of a grid area and Apply the JSDoc formatting that you should be used to from the grid outline patch. ::: devtools/client/inspector/layout/components/GridOutline.js:63 (Diff revision 1) > } > ); > }, > > /** > + * getGridAreaName - Determines if grid cell is part of a grid area and Comment should be: Returns the grid area name if the given grid cell is part of a grid area. ::: devtools/client/inspector/layout/components/GridOutline.js:101 (Diff revision 1) > - highlightedGrids.push(grid.gridFragments[0]); > } > return highlightedGrids; > }, > > - renderCell(fragment, columnNumber, rowNumber) { > + renderCell(fragment, columnNumber, rowNumber, areas, nodeId) { This is mostly a rebasing thing since it differs from the latest grid outline patch, but I don't think we need the fragment here. ::: devtools/client/inspector/layout/components/GridOutline.js:102 (Diff revision 1) > } > return highlightedGrids; > }, > > - renderCell(fragment, columnNumber, rowNumber) { > + renderCell(fragment, columnNumber, rowNumber, areas, nodeId) { > + const name = this.getGridAreaName(fragment, areas); The grid fragment object is different from {row, col} as an object. I would just pass in the individual row and col to not confuse things. ::: devtools/client/inspector/layout/components/GridOutline.js:107 (Diff revision 1) > + const name = this.getGridAreaName(fragment, areas); > return dom.rect( > { > className: "grid-outline-cell", > + "data-nodeId": nodeId, > + "data-name": name, Put data-name before data-nodeId ::: devtools/client/inspector/layout/components/GridOutline.js:119 (Diff revision 1) > ); > }, > > - renderFragment(gridFragment) { > - const { rows, cols } = gridFragment; > + renderFragment({gridFragments, id}) { > + // TODO: We are drawing the first fragment since only one is currently being stored. > + // In future we will need to iterate over all fragments of a grid. In the future,* ::: devtools/client/inspector/layout/components/GridOutline.js:151 (Diff revision 1) > > return rectangles; > }, > > onMouseOverCell({ target }) { > - // TODO: We will want onMouseOver interactions with the grid cells, which is addressed > + const { grids, onShowGridAreaHighlight } = this.props; const { grids, ... } = this.props; ::: devtools/client/inspector/layout/components/GridOutline.js:153 (Diff revision 1) > }, > > onMouseOverCell({ target }) { > - // TODO: We will want onMouseOver interactions with the grid cells, which is addressed > - // in Bug 1337235 > + const { grids, onShowGridAreaHighlight } = this.props; > + const nodeId = target.getAttribute("data-nodeId"); > + const name = target.getAttribute("data-name"); Move this above nodeId ::: devtools/client/inspector/layout/layout.js:229 (Diff revision 1) > > toolbox.highlighterUtils.highlightNodeFront(nodeFront, options); > }, > > /** > + * Handler for a change in the grid area being highlighted. We first describe when this handler is called: Handler for a mouseover in the grid outline of a grid area. Highlights the grid area in the CSS Grid Highlighter for the given grid area. ::: devtools/client/inspector/layout/layout.js:231 (Diff revision 1) > }, > > /** > + * Handler for a change in the grid area being highlighted. > + * Shows the grid area currently being highlighted. > + * @param {NodeFront} node Format the JSDoc similar to what we have done before.

Comment on attachment 8840275 [details] Bug 1337235 - Mouseover interaction for grid outline. https://reviewboard.mozilla.org/r/114762/#review117004 ::: devtools/client/inspector/layout/components/GridOutline.js:44 (Diff revision 4) > selectedGrids: grids.filter(grid => grid.highlighted), > }); > }, > > - renderGridCell(columnNumber, rowNumber, color) { > + /** > + * Returns the grid area name if the given grid cell is part of a grid area. Each * needs 1 more space indentation. Should align with /* ::: devtools/client/inspector/layout/components/GridOutline.js:52 (Diff revision 4) > + * The column number of the grid cell. > + * @param {Number} row > + * The row number of the grid cell. > + * @param {Array} areas > + * List of grid areas stored on grid fragment > + * @returns {String} s/@returns/@return. {String} should align with {Array} so remove one space. after @return. ::: devtools/client/inspector/layout/components/GridOutline.js:73 (Diff revision 4) > + * > + * @param {Number} x > + * The x-position of the grid cell. > + * @param {Number} y > + * The y-position of the grid cell. > + * @param {Number} col s/col/columnNumber ::: devtools/client/inspector/layout/components/GridOutline.js:75 (Diff revision 4) > + * The x-position of the grid cell. > + * @param {Number} y > + * The y-position of the grid cell. > + * @param {Number} col > + * The column number of the grid cell. > + * @param {Number} row s/row/rowNumber ::: devtools/client/inspector/layout/components/GridOutline.js:78 (Diff revision 4) > + * @param {Number} col > + * The column number of the grid cell. > + * @param {Number} row > + * The row number of the grid cell. > + * @param {Array} areas > + * List of grid areas stored on grid fragment Add with a period. s/List/Array s/grid areas/grid areas data s/on/in the ::: devtools/client/inspector/layout/components/GridOutline.js:82 (Diff revision 4) > + * @param {Array} areas > + * List of grid areas stored on grid fragment > + */ > + renderGridCell(x, y, columnNumber, rowNumber, color, areas) { > + const name = this.getGridAreaName(columnNumber, rowNumber, areas); > return dom.rect( I think we have a grid key that we can store in a data-id attribute to make getting the grid and its nodefront easier in onShowGridAreaHighlight so we don't have to traverse through the list of grids as well. ::: devtools/client/inspector/layout/components/GridOutline.js:85 (Diff revision 4) > + renderGridCell(x, y, columnNumber, rowNumber, color, areas) { > + const name = this.getGridAreaName(columnNumber, rowNumber, areas); > return dom.rect( > { > className: "grid-outline-cell", > - x: columnNumber, > + "data-name": name, s/data-name/data-grid-area-name ::: devtools/client/inspector/layout/components/GridOutline.js:164 (Diff revision 4) > + const name = target.getAttribute("data-name"); > + const color = target.getAttribute("stroke"); > + > + target.setAttribute("fill", color); > + > + onShowGridAreaHighlight(name, color); We need to make sure we aren't still highlighting the grid area when we leave a cell that has a grid area to a cell that doesn't have a grid area. ::: devtools/client/inspector/layout/layout.js:105 (Diff revision 4) > return this.swatchColorPickerTooltip; > }, > > /** > * Set the inspector selection. > * @param {NodeFront} nodeFront Add a new line before @param here while you are making fixes. ::: devtools/client/inspector/layout/layout.js:153 (Diff revision 4) > > onShowBoxModelEditor, > onShowBoxModelHighlighter, > > /** > * Shows the box-model highlighter on the element corresponding to the provided Ah, I see the indentation for this is also messed up. We need to remove a space before the *. Can you fix this while we are at it. ::: devtools/client/inspector/layout/layout.js:167 (Diff revision 4) > let toolbox = this.inspector.toolbox; > toolbox.highlighterUtils.highlightNodeFront(nodeFront, options); > }, > > /** > + * Handler for a mouseover in the grid outline of a grid area. * needs 1 more space indented. ::: devtools/client/inspector/layout/layout.js:167 (Diff revision 4) > let toolbox = this.inspector.toolbox; > toolbox.highlighterUtils.highlightNodeFront(nodeFront, options); > }, > > /** > + * Handler for a mouseover in the grid outline of a grid area. This isn't really a handler for any typical events. So, we should just be describing its functionality which is stated in the second sentence of the comments. We can remove this handler sentence.

Comment on attachment 8840275 [details] Bug 1337235 - Mouseover interaction for grid outline. https://reviewboard.mozilla.org/r/114762/#review117246

Comment on attachment 8840275 [details] Bug 1337235 - Mouseover interaction for grid outline. https://reviewboard.mozilla.org/r/114762/#review117556

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0a15dfecc9fa Mouseover interaction for grid outline. r=gl