[Breakpoints] Group Breakpoints (#5845)

Author: darkwing

src/components/SecondaryPanes/Breakpoints.css

@@ -10,20 +10,32 @@
   user-select: none;
 }
 
+.breakpoints-list .breakpoint-heading {
+  text-overflow: ellipsis;
+  overflow: hidden;
+}
+
+.breakpoints-list .breakpoint-heading,
 .breakpoints-list .breakpoint {
   font-size: 12px;
   color: var(--theme-content-color1);
-  padding: 0.5em 1em 0.5em 0.5em;
   line-height: 1em;
   position: relative;
   transition: all 0.25s ease;
+  padding: 0.5em 1em 0.5em 0.5em;
 }
 
-html[dir="rtl"] .breakpoints-list .breakpoint {
+.breakpoints-list .breakpoint {
+  display: flex;
+}
+
+html[dir="rtl"] .breakpoints-list .breakpoint,
+html[dir="rtl"] .breakpoints-list .breakpoint-heading {
   border-right: 4px solid transparent;
 }
 
-html:not([dir="rtl"]) .breakpoints-list .breakpoint {
+html:not([dir="rtl"]) .breakpoints-list .breakpoint,
+html:not([dir="rtl"]) .breakpoints-list .breakpoint-heading {
   border-left: 4px solid transparent;
 }
 
@@ -53,6 +65,27 @@ html .breakpoints-list .breakpoint.paused {
   background-color: var(--search-overlays-semitransparent);
 }
 
+.breakpoints-list .breakpoint .breakpoint-line,
+.breakpoints-list .breakpoint-label {
+  font-family: var(--monospace-font-family);
+}
+
+.breakpoints-list .breakpoint .breakpoint-line {
+  font-size: 11px;
+  color: var(--theme-comment);
+  padding-top: 3px;
+  min-width: 14px;
+  text-align: right;
+}
+
+html[dir="rtl"] .breakpoints-list .breakpoint .breakpoint-line {
+  text-align: left;
+}
+
+.breakpoints-list .breakpoint:hover .breakpoint-line {
+  display: none;
+}
+
 .breakpoints-list .breakpoint.paused:hover {
   border-color: var(--breakpoint-active-color-hover);
 }
@@ -62,6 +95,10 @@ html .breakpoints-list .breakpoint.paused {
   display: inline-block;
   padding-inline-start: 2px;
   cursor: default;
+  flex-grow: 1;
+  text-overflow: ellipsis;
+  overflow: hidden;
+  padding-top: 3px;
 }
 
 .breakpoint-label .breakpoint-checkbox {
@@ -88,6 +125,11 @@ html .breakpoints-list .breakpoint.paused {
   offset-inline-end: 13px;
   offset-inline-start: auto;
   top: 9px;
+  display: none;
+}
+
+.breakpoint:hover .close-btn {
+  display: flex;
 }
 
 .breakpoint .close {

src/components/SecondaryPanes/Breakpoints.js

@@ -9,11 +9,10 @@ import { connect } from "react-redux";
 import * as I from "immutable";
 import classnames from "classnames";
 import { createSelector } from "reselect";
-import { sortBy } from "lodash";
+import { groupBy, sortBy } from "lodash";
 
 import actions from "../../actions";
 import CloseButton from "../shared/Button/Close";
-import { endTruncateStr } from "../../utils/utils";
 import { features } from "../../utils/prefs";
 import { getFilename } from "../../utils/source";
 import {
@@ -68,23 +67,14 @@ function getBreakpointFilename(source) {
   return source && source.toJS ? getFilename(source.toJS()) : "";
 }
 
-function renderSourceLocation(source, line, column) {
-  const filename = getBreakpointFilename(source);
+function getBreakpointLocation(source, line, column) {
   const isWasm = source && source.isWasm;
   const columnVal = features.columnBreakpoints && column ? `:${column}` : "";
   const bpLocation = isWasm
     ? `0x${line.toString(16).toUpperCase()}`
     : `${line}${columnVal}`;
 
-  if (!filename) {
-    return null;
-  }
-
-  return (
-    <div className="location">
-      {`${endTruncateStr(filename, 30)}: ${bpLocation}`}
-    </div>
-  );
+  return bpLocation;
 }
 
 class Breakpoints extends Component<Props> {
@@ -149,32 +139,53 @@ class Breakpoints extends Component<Props> {
           onClick={ev => ev.stopPropagation()}
         />
         <label className="breakpoint-label" title={breakpoint.text}>
-          {renderSourceLocation(breakpoint.location.source, line, column)}
+          {breakpoint.text}
         </label>
-        <CloseButton
-          handleClick={ev => this.removeBreakpoint(ev, breakpoint)}
-          tooltip={L10N.getStr("breakpoints.removeBreakpointTooltip")}
-        />
+        <div className="breakpoint-line-close">
+          <div className="breakpoint-line">
+            {getBreakpointLocation(breakpoint.location.source, line, column)}
+          </div>
+          <CloseButton
+            handleClick={ev => this.removeBreakpoint(ev, breakpoint)}
+            tooltip={L10N.getStr("breakpoints.removeBreakpointTooltip")}
+          />
+        </div>
       </div>
     );
   }
 
+  renderEmpty() {
+    return <div className="pane-info">{L10N.getStr("breakpoints.none")}</div>;
+  }
+
+  renderBreakpoints() {
+    const { breakpoints } = this.props;
+
+    const groupedBreakpoints = groupBy(
+      sortBy([...breakpoints.valueSeq()], bp => bp.location.line),
+      bp => getBreakpointFilename(bp.location.source)
+    );
+
+    return [
+      ...Object.keys(groupedBreakpoints).map(filename => {
+        return [
+          <div className="breakpoint-heading" title={filename} key={filename}>
+            {filename}
+          </div>,
+          ...groupedBreakpoints[filename].map(bp => this.renderBreakpoint(bp))
+        ];
+      })
+    ];
+  }
+
   render() {
     const { breakpoints } = this.props;
-    const children =
-      breakpoints.size === 0 ? (
-        <div className="pane-info">{L10N.getStr("breakpoints.none")}</div>
-      ) : (
-        sortBy(
-          [...breakpoints.valueSeq()],
-          [
-            bp => getBreakpointFilename(bp.location.source),
-            bp => bp.location.line
-          ]
-        ).map(bp => this.renderBreakpoint(bp))
-      );
-
-    return <div className="pane breakpoints-list">{children}</div>;
+
+    return (
+      <div className="pane breakpoints-list">
+        {breakpoints.size ? this.renderBreakpoints() : this.renderEmpty()}
+      </div>
+    );
   }
 }
 

src/test/mochitest/browser_dbg-breakpoints-toggle.js

@@ -1,12 +1,12 @@
 function toggleBreakpoint(dbg, index) {
-  const bp = findElement(dbg, "breakpointItem", index);
+  const bp = findAllElements(dbg, "breakpointItems")[index];
   const input = bp.querySelector("input");
   input.click();
 }
 
 async function removeBreakpoint(dbg, index) {
   const removed = waitForDispatch(dbg, "REMOVE_BREAKPOINT");
-  const bp = findElement(dbg, "breakpointItem", index);
+  const bp = findAllElements(dbg, "breakpointItems")[index];
   bp.querySelector(".close-btn").click();
   await removed;
 }
@@ -80,8 +80,8 @@ add_task(async function() {
   is(bp2.disabled, false, "second breakpoint is enabled");
 
   // Remove the breakpoints
-  await removeBreakpoint(dbg, 1);
-  await removeBreakpoint(dbg, 1);
+  await removeBreakpoint(dbg, 0);
+  await removeBreakpoint(dbg, 0);
 
   const bps = findBreakpoints(dbg);
 

src/test/mochitest/browser_dbg-breakpoints.js

@@ -2,18 +2,11 @@
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 function toggleBreakpoint(dbg, index) {
-  const bp = findElement(dbg, "breakpointItem", index);
+  const bp = findAllElements(dbg, "breakpointItems")[index];
   const input = bp.querySelector("input");
   input.click();
 }
 
-async function removeBreakpoint(dbg, index) {
-  const removed = waitForDispatch(dbg, "REMOVE_BREAKPOINT");
-  const bp = findElement(dbg, "breakpointItem", index);
-  bp.querySelector(".close-btn").click();
-  await removed;
-}
-
 async function disableBreakpoint(dbg, index) {
   const disabled = waitForDispatch(dbg, "DISABLE_BREAKPOINT");
   toggleBreakpoint(dbg, index);
@@ -62,15 +55,15 @@ add_task(async function() {
   await addBreakpoint(dbg, "simple2", 5);
 
   // Disable the first one
-  await disableBreakpoint(dbg, 1);
+  await disableBreakpoint(dbg, 0);
   let bp1 = findBreakpoint(dbg, "simple2", 3);
   let bp2 = findBreakpoint(dbg, "simple2", 5);
   is(bp1.disabled, true, "first breakpoint is disabled");
   is(bp2.disabled, false, "second breakpoint is enabled");
 
   // Disable and Re-Enable the second one
-  await disableBreakpoint(dbg, 2);
-  await enableBreakpoint(dbg, 2);
+  await disableBreakpoint(dbg, 1);
+  await enableBreakpoint(dbg, 1);
   bp2 = findBreakpoint(dbg, "simple2", 5);
   is(bp2.disabled, false, "second breakpoint is enabled");
 });

src/test/mochitest/browser_dbg-browser-content-toolbox.js

@@ -14,7 +14,7 @@ const {
 } = require("devtools/client/framework/devtools-browser");
 
 function toggleBreakpoint(dbg, index) {
-  const bp = findElement(dbg, "breakpointItem", index);
+  const bp = findAllElements(dbg, "breakpointItems")[index];
   const input = bp.querySelector("input");
   input.click();
 }
@@ -57,12 +57,12 @@ add_task(async function() {
   await addBreakpoint(dbg, "simple2", 3);
 
   info("Disable the breakpoint");
-  await disableBreakpoint(dbg, 1);
+  await disableBreakpoint(dbg, 0);
   let bp = findBreakpoint(dbg, "simple2", 3);
   is(bp.disabled, true, "breakpoint is disabled");
 
   info("Enable the breakpoint");
-  await enableBreakpoint(dbg, 1);
+  await enableBreakpoint(dbg, 0);
   bp = findBreakpoint(dbg, "simple2", 3);
   is(bp.disabled, false, "breakpoint is enabled");
 

src/test/mochitest/head.js

@@ -936,7 +936,8 @@ const selectors = {
     `.expressions-list .expression-container:nth-child(${i}) .close`,
   expressionNodes: ".expressions-list .tree-node",
   scopesHeader: ".scopes-pane ._header",
-  breakpointItem: i => `.breakpoints-list .breakpoint:nth-child(${i})`,
+  breakpointItem: i => `.breakpoints-list .breakpoint:nth-of-type(${i})`,
+  breakpointItems: `.breakpoints-list .breakpoint`,
   scopes: ".scopes-list",
   scopeNode: i => `.scopes-list .tree-node:nth-child(${i}) .object-label`,
   scopeValue: i =>