Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

[Editor] Jumping to function definitions should go to right location #6028

Closed
jasonLaster opened this issue Apr 19, 2018 · 4 comments
Closed
Assignees
Milestone

Comments

@jasonLaster
Copy link
Contributor

jasonLaster commented Apr 19, 2018

STR:

  1. navigate to http://firefox-dev.tools/debugger-examples/examples/increment/
  2. add a breakpoint in increment.js:4
  3. click the increment button
  4. when you're paused hover on "add" and click the link

Expected: you jump to "add" in math.js
Actual: you jump into the bundle

diff --git a/src/actions/pause/paused.js b/src/actions/pause/paused.js
index fb6bcea..677cdee 100644
--- a/src/actions/pause/paused.js
+++ b/src/actions/pause/paused.js
@@ -75,6 +75,10 @@ export function paused(pauseInfo: Pause) {
     const selectedFrame = getSelectedFrame(getState());
 
     if (selectedFrame) {
+      // this code is basically doing the same thing
+      // if we're smart we can replace this code with the new function
+
+      // instead of using getVisibleSelectedFrame we'd use getSelectedLocation
       const visibleFrame = getVisibleSelectedFrame(getState());
       const location = isGeneratedId(visibleFrame.location.sourceId)
         ? selectedFrame.generatedLocation
diff --git a/src/actions/sources/select.js b/src/actions/sources/select.js
index 4f592ef..130ccce 100644
--- a/src/actions/sources/select.js
+++ b/src/actions/sources/select.js
@@ -76,6 +76,17 @@ export function selectSourceURL(
   };
 }
 
+// naming is tough
+function selectLocation2(location) {
+  return async ({ dispatch }: ThunkArgs) => {
+    const selectedLocation = getSelectedLocation(getState());
+    const newLocation = isGeneratedId(selectedLocation.sourceId)
+      ? location.generatedLocation
+      : location.location;
+    return await dispatch(selectSpecificLocation(newLocation));
+  };
+}
+
 /**
  * @memberof actions/sources
  * @static
diff --git a/src/components/Editor/Preview/Popup.js b/src/components/Editor/Preview/Popup.js
index 8363330..3b8f8ae 100644
--- a/src/components/Editor/Preview/Popup.js
+++ b/src/components/Editor/Preview/Popup.js
@@ -139,6 +139,9 @@ export class Popup extends Component<Props> {
     return (
       <div
         className="preview-popup"
+        // this is currently what we do...
+        // what we should do is have a new action that looks at whether you're currently in an original source and if so
+        // jumps to the original location
         onClick={() => selectSourceURL(location.url, { line: location.line })}
       >
         <PreviewFunction func={value} />
diff --git a/src/components/Editor/index.js b/src/components/Editor/index.js
index eaa1c34..28556df 100644
--- a/src/components/Editor/index.js
+++ b/src/components/Editor/index.js
@@ -50,6 +50,7 @@ import {
   shouldShowFooter,
   createEditor,
   clearEditor,
+  getEditor,
   getCursorLine,
   toSourceLine,
   getDocument,
@@ -128,7 +129,7 @@ class Editor extends PureComponent<Props, State> {
   }
 
   setupEditor() {
-    const editor = createEditor();
+    let editor = getEditor() || createEditor();
 
     // disables the default search shortcuts
     // $FlowIgnore
diff --git a/src/components/shared/VisibilityHandler.js b/src/components/shared/VisibilityHandler.js
index 283644b..0d97530 100644
--- a/src/components/shared/VisibilityHandler.js
+++ b/src/components/shared/VisibilityHandler.js
@@ -25,6 +25,7 @@ class VisibilityHandler extends Component {
     super(props);
     this.isVisible = true;
     this.onVisibilityChange = this.onVisibilityChange.bind(this);
+    window.foo = this.toggleVisible;
   }
 
   componentDidMount() {
@@ -39,13 +40,18 @@ class VisibilityHandler extends Component {
     window.removeEventListener("visibilitychange", this.onVisibilityChange);
   }
 
-  onVisibilityChange() {
+  toggleVisible = () => {
+    this.isVisible = !this.isVisible;
+    this.forceUpdate();
+  };
+
+  onVisibilityChange = () => {
     this.isVisible = false;
     if (document.visibilityState == "visible") {
       this.isVisible = true;
     }
     this.forceUpdate();
-  }
+  };
 
   render() {
     return this.isVisible ? this.props.children : null;
@ghost
Copy link

ghost commented May 6, 2018

This seems a little above my skill level, but I'll give it a go.

@jasonLaster
Copy link
Contributor Author

how's it going @Ch4r13s

@lukaszsobek
Copy link
Contributor

It seems the tooltip gets passed the values of the root file which in turn causes it to navigate to the wrong place.

@darkwing
Copy link
Contributor

darkwing commented Jun 5, 2018

I can confirm this is still an issue. Also should mention it's a bit hard to click on the tooltip because it disappears so quickly; I had to make the following change to make working with the problem easier:

diff --git a/src/components/Editor/Preview/index.js b/src/components/Editor/Preview/index.js
index 8b340c74a..025a935cb 100644
--- a/src/components/Editor/Preview/index.js
+++ b/src/components/Editor/Preview/index.js
@@ -119,7 +119,7 @@ class Preview extends PureComponent<Props, State> {

   onTokenLeave = e => {
     if (!inPopup(e)) {
-      this.props.clearPreview();
+      //this.props.clearPreview();
     }
   };

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants