Bug 1526975: Filter out implicit grid line names in text-property-editor's autocomplete. r=gl,bradwerth

Differential Revision: https://phabricator.services.mozilla.com/D21308

--HG--
extra : moz-landing-system : lando

devtools/client/inspector/rules/test/browser.ini

@@ -17,6 +17,7 @@ support-files =
   doc_filter.html
   doc_frame_script.js
   doc_grid_names.html
+  doc_grid_area_gridline_names.html
   doc_inline_sourcemap.html
   doc_invalid_sourcemap.css
   doc_invalid_sourcemap.html
@@ -190,6 +191,8 @@ skip-if = (os == "win" && debug) # bug 963492: win.
 [browser_rules_grid-toggle_04.js]
 [browser_rules_grid-toggle_05.js]
 [browser_rules_gridline-names-autocomplete.js]
+[browser_rules_gridline-names-are-shown-correctly.js]
+skip-if = os == 'linux' # focusEditableField times out consistently on linux.
 [browser_rules_guessIndentation.js]
 [browser_rules_highlight-element-rule.js]
 [browser_rules_highlight-property.js]

devtools/client/inspector/rules/test/browser_rules_gridline-names-are-shown-correctly.js

@@ -0,0 +1,106 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests that the text editor correctly calculates the grid line names shown in the
+// autocomplete popup. We generally want to show all the grid line names for a grid
+// container, except for implicit line names created by an implicitly named area.
+
+const TEST_URL = URL_ROOT + "doc_grid_area_gridline_names.html";
+
+add_task(async function() {
+  await addTab(TEST_URL);
+  const { inspector, view } = await openRuleView();
+
+  info("Test that implicit grid line names from explicit grid areas are shown.");
+  await testExplicitNamedAreas(inspector, view);
+
+  info("Test that explicit grid line names creating implicit grid areas are shown.");
+  await testImplicitNamedAreasWithExplicitGridLineNames(inspector, view);
+
+  info("Test that implicit grid line names creating implicit grid areas are not shown.");
+  await testImplicitAreasWithImplicitGridLineNames(inspector, view);
+  await testImplicitNamedAreasWithReversedGridLineNames(inspector, view);
+});
+
+async function testExplicitNamedAreas(inspector, view) {
+  await selectNode(".header", inspector);
+
+  const gridColLines = ["header-start", "header-end", "main-start", "main-end"];
+
+  const propertyName = view.styleDocument.querySelectorAll(".ruleview-propertyvalue")[0];
+  const gridLineNamesUpdated = inspector.once("grid-line-names-updated");
+  const editor = await focusEditableField(view, propertyName);
+  const onPopupShown = once(editor.popup, "popup-opened");
+  await gridLineNamesUpdated;
+
+  EventUtils.synthesizeKey("VK_DOWN", { shiftKey: true }, view.styleWindow);
+
+  await onPopupShown;
+
+  info("Check that the expected grid line column names are shown in the editor popup.");
+  for (const lineName of gridColLines) {
+    ok(editor.gridLineNames.cols.indexOf(lineName) > -1,
+      `${lineName} is a suggested implicit grid line`);
+  }
+}
+
+async function testImplicitNamedAreasWithExplicitGridLineNames(inspector, view) {
+  await selectNode(".contentArea", inspector);
+
+  const gridRowLines = ["main-start", "main-end", "content-start", "content-end"];
+
+  const propertyName = view.styleDocument.querySelectorAll(".ruleview-propertyvalue")[1];
+  const gridLineNamesUpdated = inspector.once("grid-line-names-updated");
+  const editor = await focusEditableField(view, propertyName);
+  const onPopupShown = once(editor.popup, "popup-opened");
+  await gridLineNamesUpdated;
+
+  EventUtils.synthesizeKey("VK_DOWN", { shiftKey: true }, view.styleWindow);
+
+  await onPopupShown;
+
+  info("Check that the expected grid line row names are shown in the editor popup.");
+  for (const lineName of gridRowLines) {
+    ok(editor.gridLineNames.rows.indexOf(lineName) > -1,
+      `${lineName} is a suggested explicit grid line`);
+  }
+}
+
+async function testImplicitAreasWithImplicitGridLineNames(inspector, view) {
+  await selectNode(".a", inspector);
+
+  const propertyName = view.styleDocument.querySelectorAll(".ruleview-propertyvalue")[0];
+  const gridLineNamesUpdated = inspector.once("grid-line-names-updated");
+  const editor = await focusEditableField(view, propertyName);
+  const onPopupShown = once(editor.popup, "popup-opened");
+  await gridLineNamesUpdated;
+
+  EventUtils.synthesizeKey("VK_DOWN", { shiftKey: true }, view.styleWindow);
+
+  await onPopupShown;
+
+  info("Check that the implicit grid lines created by implicit areas are not shown.");
+  ok(!(editor.gridLineNames.cols.indexOf("a-end") > -1),
+    "a-end is not shown because it is created by an implicit named area.");
+}
+
+async function testImplicitNamedAreasWithReversedGridLineNames(inspector, view) {
+  await selectNode(".b", inspector);
+
+  const propertyName = view.styleDocument.querySelectorAll(".ruleview-propertyvalue")[0];
+  const gridLineNamesUpdated = inspector.once("grid-line-names-updated");
+  const editor = await focusEditableField(view, propertyName);
+  const onPopupShown = once(editor.popup, "popup-opened");
+  await gridLineNamesUpdated;
+
+  EventUtils.synthesizeKey("VK_DOWN", { shiftKey: true }, view.styleWindow);
+
+  await onPopupShown;
+
+  info("Test that reversed implicit grid line names from implicit areas are not shown");
+  ok(!(editor.gridLineNames.cols.indexOf("b-start") > -1),
+    "b-start is not shown because it is created by an implicit named area.");
+}

devtools/client/inspector/rules/test/doc_grid_area_gridline_names.html

@@ -0,0 +1,59 @@
+<!doctype html>
+<style type='text/css'>
+  /* Implicit gridlines created from explicit grid areas. */
+  .wrapperA {
+    display: grid;
+    grid-template-columns: repeat(3, 1fr);
+    grid-auto-rows: minmax(100px, auto);
+    grid-template-areas:
+      "header header header"
+      "main   main   main";
+  }
+
+  .header {
+    grid-column: header-start / header-end;
+    grid-row: header-start / header-end;
+  }
+
+  .main {
+    grid-area: main;
+  }
+
+  /* Implicit grid areas created from explicit gridlines */
+  .wrapperB {
+    display: grid;
+    grid-template-columns: [main-start] 1fr [content-start] 1fr [content-end main-end];
+    grid-template-rows: [main-start] 100px [content-start] 100px [content-end main-end];
+  }
+
+  .contentArea {
+    grid-column: content-start / content-end;
+    grid-row: content-start / content-end;
+  }
+
+  .wrapperC {
+    display: grid;
+    grid-template-columns: [a-start b-end] 1fr [c];
+  }
+
+  .a {
+    grid-column: a-start / a-end;
+  }
+
+  .b {
+    grid-column: b-start / b-end;
+  }
+</style>
+<div>
+  <div class="wrapperA">
+    <div class="header">Header</div>
+    <div class="main">Content</div>
+  </div>
+  <div class="wrapperB">
+    <div class="contentArea">Implicit area named "content".</div>
+  </div>
+  <div class="wrapperC">
+    <div class="a">A.</div>
+    <div class="b">B.</div>
+  </div>
+</div>

devtools/client/inspector/rules/views/text-property-editor.js

@@ -330,10 +330,39 @@ TextPropertyEditor.prototype = {
 
       for (const gridFragment of gridFragments) {
         for (const rowLine of gridFragment.rows.lines) {
-          gridLineNames.rows = gridLineNames.rows.concat(rowLine.names);
+          // We specifically ignore implicit line names created from implicitly named
+          // areas. This is because showing implicit line names can be confusing for
+          // designers who may have used a line name with "-start" or "-end" and created
+          // an implicitly named grid area without meaning to.
+          let gridArea;
+
+          for (const name of rowLine.names) {
+            const rowLineName = name.substring(0, name.lastIndexOf("-start")) ||
+              name.substring(0, name.lastIndexOf("-end"));
+            gridArea = gridFragment.areas.find(area => area.name === rowLineName);
+
+            if (rowLine.type === "implicit" && gridArea &&
+                gridArea.type === "implicit") {
+              continue;
+            }
+            gridLineNames.rows.push(name);
+          }
         }
+
         for (const colLine of gridFragment.cols.lines) {
-          gridLineNames.cols = gridLineNames.cols.concat(colLine.names);
+          let gridArea;
+
+          for (const name of colLine.names) {
+            const colLineName = name.substring(0, name.lastIndexOf("-start")) ||
+              name.substring(0, name.lastIndexOf("-end"));
+            gridArea = gridFragment.areas.find(area => area.name === colLineName);
+
+            if (colLine.type === "implicit" && gridArea &&
+                gridArea.type === "implicit") {
+              continue;
+            }
+            gridLineNames.cols.push(name);
+          }
         }
       }
     }

devtools/server/actors/utils/css-grid-utils.js

@@ -58,8 +58,8 @@ function getStringifiableArea({ columnEnd, columnStart, name, rowEnd, rowStart,
   return { columnEnd, columnStart, name, rowEnd, rowStart, type };
 }
 
-function getStringifiableLine({ breadth, names, number, start }) {
-  return { breadth, names, number, start };
+function getStringifiableLine({ breadth, names, number, start, type }) {
+  return { breadth, names, number, start, type };
 }
 
 function getStringifiableTrack({ breadth, start, state, type }) {

devtools/server/tests/browser/browser_layout_getGrids.js

@@ -40,18 +40,21 @@ const GRID_FRAGMENT_DATA = {
         names: ["col-1", "col-start-1", "sidebar-start"],
         number: 1,
         start: 0,
+        type: "explicit",
       },
       {
         breadth: 0,
         names: ["col-2", "header-start", "sidebar-end", "content-start"],
         number: 2,
         start: 100,
+        type: "explicit",
       },
       {
         breadth: 0,
         names: ["header-end", "content-end"],
         number: 3,
         start: 200,
+        type: "explicit",
       },
     ],
     tracks: [
@@ -76,18 +79,21 @@ const GRID_FRAGMENT_DATA = {
         names: ["header-start"],
         number: 1,
         start: 0,
+        type: "explicit",
       },
       {
         breadth: 0,
         names: ["header-end", "sidebar-start", "content-start"],
         number: 2,
         start: 100,
+        type: "explicit",
       },
       {
         breadth: 0,
         names: ["sidebar-end", "content-end"],
         number: 3,
         start: 200,
+        type: "explicit",
       },
     ],
     tracks: [