[Breakpoints] show breakpoint on hover

[jasonLaster]

I recently was looking at "Continue to Line" for time travel debugging. CtL got me thinking it about two things:

1. it would be nice to have a hover effect in the gutter. For instance, hovering on a line where you can add a breakpoint shows a light breakpoint. With continue to here, hovering on a line that you can jump to with the alt key pressed could show a light background color indicating you can jump there.

2. the way we currently show empty lines (greying out the line number). Can create a zebra effect that is quite distracting. Most of time, the user doesn't care which lines are empty or not, bu the color difference is always visible.

Lastly, I think disabling some lines so that you can't add a breakpoint or jump is helpful, but we just need a soft nudge at the right moment. The hover effect should validate whether it's possible to jump or set the breakpoint, without being distracting at other times.

Breakpoints On Hover

In this example, the user is hovering on 127 and can add a breakpoint. The user is also hovering on line 133 with the alt key pressed and can jump to 133.

Empty Lines

---

Other options:

• remove the opacity, perhaps the user doesn't need a visual indicator that the line is valid
• remove the opacity, and use a different visual indicator like bold the line number on hover

[darkwing]

Maybe a light grey when you hover, so it's not confused with disabled breakpoint? I love the idea though.

[jasonLaster]

yeah - that's a good point. i was worried about disabled bp state too

[violasong]

Love this idea! More informative than the empty lines, and lol that's so true re: zebra effect.

Sounds like the same line could have either a breakpoint added or CtL activated, so there may be a need for a combined indicator?

[jasonLaster]

ooh good point. You could jump to a line with a breakpoint.

I kinda was lazy when i chose the background color for CtL maybe we need breakpoint like icon that we can show?

[violasong]

Yes, maybe we could incorporate the little arrow from "jump to debugger." Now I'm also wondering if CtL should allow you to alt-click anywhere in the line of code to surface this feature more (though we wouldn't want to conflict with the all-important 2-D text selection :)). In that case we could do an effect with the entire row. Not sure how surfaced we want this feature to be - that might be too much.

[jasonLaster]

Yeah, Continue to Function (CtF) or Jump to Function (JtF) is definitely a thing. We discussed it with @bomsy in the fall.

Technically we basically are there, but have held off because the UX can be awkward and we've been a bit busy 😉

Here is a GIF of a minimal CtF that feels a lot like an IDE.

By the way, I used alt click earlier. I think editors use cmd+click to jump, so it might be good to switch there.

[violasong]

Ah, interesting. I noticed that in Chrome debugger, holding down Cmd causes a few keywords to be highlighted inside the function, and clicking them seems to activate CtF. Seems nice to be able to see the clickable targets upon holding Cmd, if there aren't going to be too many of them.

[jasonLaster]

yeah, very similar!

and yes, we want to converge on that shortcut

[CharlesStocky]

/claim (if it's still open)

[claim]

Thanks for claiming the issue! 👋

Here are some links for getting setup, contributing, and developing. We're always happy to answer questions in slack! If you become busy, feel free to /unclaim it.

🦊 Debugger team!

[darkwing]

Is this still on your radar @CharlesStocky ? Would you like to unclaim? How can we help?

[CharlesStocky]

@darkwing I've been working on other stuff, I'll start focusing on it more now.

[jasonLaster]

Alright, so this is definitely a tricky problem technically. I just played with a bunch of solutions and kinda stumbled on doing it the way we handle the preview popup:

• add a codemirror onmouseover handler
• try really hard to only look at gutter elements
• try really hard to know when we change lines

Things to do:

• clear the breakpoint when you move off of the gutter
• dont clobber lines that have a breakpoint
• improve the styling...

diff --git a/src/components/Editor/Breakpoints.js b/src/components/Editor/Breakpoints.js
index 14578af..20dab9b 100644
--- a/src/components/Editor/Breakpoints.js
+++ b/src/components/Editor/Breakpoints.js
@@ -7,11 +7,16 @@ import { connect } from "react-redux";
 import React, { Component } from "react";
 
 import Breakpoint from "./Breakpoint";
+import { getDocument, toEditorLine } from "../../utils/editor";
 
 import { getSelectedSource, getVisibleBreakpoints } from "../../selectors";
 import { makeLocationId } from "../../utils/breakpoint";
 import { isLoaded } from "../../utils/source";
 
+import Svg from "../shared/Svg";
+import ReactDOM from "react-dom";
+import classnames from "classnames";
+
 import type { BreakpointsMap } from "../../reducers/types";
 import type { SourceRecord } from "../../types";
 
@@ -21,7 +26,28 @@ type Props = {
   editor: Object
 };
 
+const breakpointSvg = document.createElement("div");
+ReactDOM.render(<Svg name="breakpoint" />, breakpointSvg);
+
+function makeMarker(isDisabled: boolean) {
+  const bp = breakpointSvg.cloneNode(true);
+  bp.className = classnames("editor new-breakpoint", {
+    "breakpoint-disabled": isDisabled
+  });
+
+  return bp;
+}
+
 class Breakpoints extends Component<Props> {
+  currentEl = null;
+
+  componentDidMount() {
+    const { codeMirror } = this.props.editor;
+    const codeMirrorWrapper = codeMirror.getWrapperElement();
+    // debugger;
+    codeMirrorWrapper.addEventListener("mouseover", this.onMouseOver);
+  }
+
   shouldComponentUpdate(nextProps: Props) {
     if (nextProps.selectedSource && !isLoaded(nextProps.selectedSource)) {
       return false;
@@ -30,6 +56,45 @@ class Breakpoints extends Component<Props> {
     return true;
   }
 
+  onMouseOver = e => {
+    const { editor, selectedSource } = this.props;
+    const { codeMirror } = editor;
+
+    if (!e.target.classList.contains("CodeMirror-linenumber")) {
+      return;
+    }
+
+    if (!selectedSource) {
+      return;
+    }
+
+    const el = e.target;
+    const line = parseInt(el.innerText, 10);
+    const sourceId = selectedSource.get("id");
+    const doc = getDocument(sourceId);
+
+    if (this.currentEl && el.innerText == this.currentEl.innerText) {
+      // console.log(`same el`);
+      return;
+    }
+
+    if (this.currentEl) {
+      const oldLine = parseInt(this.currentEl.innerText, 10);
+      // console.log("clearing old line", oldLine);
+      doc.setGutterMarker(oldLine - 1, "breakpoints", null);
+    }
+
+    // console.log(`new el`, el);
+    this.currentEl = el;
+
+    const marker = codeMirror.setGutterMarker(
+      line - 1,
+      "breakpoints",
+      makeMarker(true)
+    );
+    // console.log(`Entering ${el.innerText}`);
+  };
+
   render() {
     const { breakpoints, selectedSource, editor } = this.props;
 

[CharlesStocky]

Thanks for looking into it @jasonLaster. It would have taken me a while to come up with a solution resembling that, if ever.

[CharlesStocky]

@jasonLaster how is it determined if a line can or can't have a breakpoint set?

[jasonLaster]

it has an emptyLine class. We also use the empty lines data from redux

[Sekhmet]

I'm working on CSS solution for this which should be easier to implement (with some CSS voodoo).