Repository: vgteam/sequenceTubeMap (js)

Issue:
Make the left and right buttons go to the previous/next BED region when a BED file is selected
If a BED subregion contains a `tracks.json` that doesn't point at files the server is allowed to use (because e.g. it was extracted from a larger graph that is not available on the web), we should figure this out and stop the user from trying to use that file.

For example, we should turn off the left and right buttons, because we know we won't be able to fetch a different set of coordinates (or we could make them go left/right to the next/previous region *in the BED file*).

We should also stop the user from going to coordinates not represented in the BED with a premade chunk, if they don't actually have a set of tracks that we could fetch the region from.

(Maybe we want to handle this by allowing empty/null track files in `tracks.json` in a BED region? Then we can use null by default in the extraction scripts, and you could make sure to fill in the paths manually if you want a BED with pre-extracted regions but also a real backing graph.) 

Pull request description:
Fix left right buttons
Fixes #354 

Patch:
diff --git a/src/common.mjs b/src/common.mjs
index 4b429d1..e350fa2 100644
--- a/src/common.mjs
+++ b/src/common.mjs
@@ -26,6 +26,8 @@ const removeCommas = (input) => {
 // or
 // { contig, start, distance }
 //
+// a region string could look like: "17:1-100"
+// 
 // For distance, + is used as the coordinate separator. For start/end ranges, - is used.
 // The coordinates are set off from the contig by the last colon.
 // Commas in coordinates are removed.
diff --git a/src/components/DataPositionFormRow.js b/src/components/DataPositionFormRow.js
index d0f278c..627e1af 100644
--- a/src/components/DataPositionFormRow.js
+++ b/src/components/DataPositionFormRow.js
@@ -84,6 +84,7 @@ class DataPositionFormRow extends Component {
           color="primary"
           id="goLeftButton"
           onClick={this.props.handleGoLeft}
+          disabled={this.props.uploadInProgress || !this.props.canGoLeft}
         >
           <FontAwesomeIcon icon={faStepBackward} size="lg" />
         </Button>
@@ -97,6 +98,7 @@ class DataPositionFormRow extends Component {
           color="primary"
           id="goRightButton"
           onClick={this.props.handleGoRight}
+          disabled={this.props.uploadInProgress || !this.props.canGoRight}
         >
           <FontAwesomeIcon icon={faStepForward} size="lg" />
         </Button>
@@ -125,6 +127,8 @@ DataPositionFormRow.propTypes = {
   uploadInProgress: PropTypes.bool.isRequired,
   getCurrentViewTarget: PropTypes.func.isRequired,
   viewTargetHasChange: PropTypes.bool.isRequired,
+  canGoLeft: PropTypes.bool.isRequired,
+  canGoRight: PropTypes.bool.isRequired,
 };
 
 export default DataPositionFormRow;
diff --git a/src/components/HeaderForm.js b/src/components/HeaderForm.js
index 5a1b837..14bf7b8 100644
--- a/src/components/HeaderForm.js
+++ b/src/components/HeaderForm.js
@@ -44,8 +44,25 @@ const CLEAR_STATE = {
   // Description for the selected region, is not displayed when empty
   desc: "",
 
-  // This tracks several arrays of BED region data, stored by data type, with
+  // This tracks several arrays (desc, chr, start, end) of BED region data, with
   // one entry in each array per region.
+  // desc: description of region, i.e. "region with no source graph available"
+  // chr: path in graph where the region is on, i.e. in ref:2000-3000, "ref" is the chr
+  // start: start of the region, i.e. in ref:2000-3000, 2000 is the start
+  // end: end of the region, i.e. in ref:2000-3000, 3000 is the end
+  // chunk: url/directory for preexisting cached chunk, or empty string if not available
+  // tracks: object full of tracks to apply when user selects region, or null
+  // so regionInfo might look like: 
+  /*
+  {
+    chr: [ '17', '17' ],
+    start: [ '1', '1000' ],
+    end: [ '100', '1200' ],
+    desc: [ '17_1_100', '17_1000_1200' ],
+    chunk: [ '', '' ],
+    tracks: [ null, null ]
+  }
+  */
   regionInfo: {},
 
   pathNames: [],
@@ -171,6 +188,77 @@ function viewTargetsEqual(currViewTarget, nextViewTarget) {
   return true;
 }
 
+/* determine the current region: accepts a region string and returns the region index
+
+  example of regionInfo:
+  {
+    chr: [ '17', '17' ],
+    start: [ '1', '1000' ],
+    end: [ '100', '1200' ],
+    desc: [ '17_1_100', '17_1000_1200' ],
+    chunk: [ '', '' ],
+    tracks: [ null, null ]
+  }
+
+  examples: 
+  if the regionString is "17:1-100", it would be parsed into {contig: "17", start: 1, end: 100} -> 0
+  if the regionString is "17:1000-1200", it would be parsed into {contig: "17", start: 1000, end: 1200} -> 1
+  if the regionString is "17:2000-3000", it cannot be found - return null
+
+  The function uses this approach to find the regionIndex given regionString and regionInfo:
+  function (region string){
+    parse(region string) -> return {contig, start, end}
+    loop over chr in region info
+      determine if contig, start, end are present at the current index
+      if present: return index
+    return null
+  }
+*/
+export const determineRegionIndex = (regionString, regionInfo) => {
+  let parsedRegion;
+  try {
+    parsedRegion = parseRegion(regionString);
+  } catch(error) {
+    return null;
+  }
+  if (!regionInfo["chr"]){
+    return null;
+  }
+  for (let i = 0; i < regionInfo["chr"].length; i++){
+    if ((parseInt(regionInfo["start"][i]) === parsedRegion.start) 
+        && (parseInt(regionInfo["end"][i]) === parsedRegion.end)
+        && (regionInfo["chr"][i] === parsedRegion.contig)){
+          return i;
+    }
+  }
+  return null;
+}
+
+/*
+  This function takes in a regionIndex and regionInfo, and reconstructs a regionString from them
+  assumes that index is valid in regionInfo
+
+  example of regionInfo:
+  {
+    chr: [ '17', '17' ],
+    start: [ '1', '1000' ],
+    end: [ '100', '1200' ],
+    desc: [ '17_1_100', '17_1000_1200' ],
+    chunk: [ '', '' ],
+    tracks: [ null, null ]
+  }
+
+  example of regionIndex: 0
+
+  example of regionString: "17:1-100"
+*/
+export const regionStringFromRegionIndex = (regionIndex, regionInfo) => {
+  let regionStart = regionInfo["start"][regionIndex];
+  let regionEnd = regionInfo["end"][regionIndex];
+  let regionContig = regionInfo["chr"][regionIndex];
+  return regionContig + ":" + regionStart + "-" + regionEnd;
+}
+
 class HeaderForm extends Component {
   state = EMPTY_STATE;
   componentDidMount() {
@@ -504,6 +592,9 @@ class HeaderForm extends Component {
     return regionString;
   };
 
+  
+  
+  
   // In addition to a new region value, also takes tracks and chunk associated with the region
   // Update current track if the new tracks are valid
   // Otherwise check if the current bed file is a url, and if tracks can be fetched from said url
@@ -627,12 +718,57 @@ class HeaderForm extends Component {
     );
   }
 
+
+  /* Offset the region left or right by the given negative or positive fraction*/
+  // offset: +1 or -1
+  jumpRegion(offset) {
+    let regionIndex = determineRegionIndex(this.state.region, this.state.regionInfo) ?? 0;
+    if ((offset === -1 && this.canGoLeft(regionIndex)) || (offset === 1 && this.canGoRight(regionIndex))){
+      regionIndex += offset;
+    }
+    let regionString = regionStringFromRegionIndex(regionIndex, this.state.regionInfo);
+    this.setState(
+      (state) => ({
+        region: regionString,
+      }),
+      () => this.handleGoButton()
+    );
+  }
+
+  canGoLeft = (regionIndex) => {
+    if (this.state.bedFile){
+      return (regionIndex > 0);
+    } else {
+      return true;
+    }
+  }
+
+  canGoRight = (regionIndex) => {
+    if (this.state.bedFile){
+      if (!this.state.regionInfo["chr"]){
+        return false;
+      }
+      return (regionIndex < ((this.state.regionInfo["chr"].length) - 1));
+    } else {
+      return true;
+    }
+  }
+
+
   handleGoRight = () => {
-    this.budgeRegion(0.5);
+    if (this.state.bedFile){
+      this.jumpRegion(1);
+    } else {
+      this.budgeRegion(0.5);
+    }
   };
 
   handleGoLeft = () => {
-    this.budgeRegion(-0.5);
+    if (this.state.bedFile){
+      this.jumpRegion(-1);
+    } else {
+      this.budgeRegion(-0.5);
+    }
   };
 
   showFileSizeAlert = () => {
@@ -736,9 +872,13 @@ class HeaderForm extends Component {
         uploadInProgress={this.state.uploadInProgress}
         getCurrentViewTarget={this.props.getCurrentViewTarget}
         viewTargetHasChange={viewTargetHasChange}
+        canGoLeft={this.canGoLeft(determineRegionIndex(this.state.region, this.state.regionInfo))}
+        canGoRight={this.canGoRight(determineRegionIndex(this.state.region, this.state.regionInfo))}
       />
     );
 
+    
+
     return (
       <div>
         <Container>
diff --git a/src/util/tubemap.js b/src/util/tubemap.js
index 4ae9fef..c4a36de 100644
--- a/src/util/tubemap.js
+++ b/src/util/tubemap.js
@@ -674,23 +674,14 @@ function placeReads() {
   reads.forEach((read, idx) => {
     read.path.forEach((element, pathIdx) => {
       if (!element.hasOwnProperty("y")) {
-        // previous y value from pathIdx - 1 might not exist yet if that segment is also without node 
-        // use previous y value from last segment with node instead 
-        let previousValidY = null;
-        let lastIndex = pathIdx - 1;
-        while (previousValidY === null && lastIndex >= 0) {
-          previousValidY = reads[idx].path[lastIndex].y;
-          lastIndex = lastIndex - 1;
-        }
         elementsWithoutNode.push({
           readIndex: idx,
           pathIndex: pathIdx,
-          previousY: previousValidY,
+          previousY: reads[idx].path[pathIdx - 1].y,
         });
       }
     });
   });
-
   elementsWithoutNode.sort(compareNoNodeReadsByPreviousY);
   elementsWithoutNode.forEach((element) => {
     const segment = reads[element.readIndex].path[element.pathIndex];


Test patch:
diff --git a/src/components/HeaderForm.test.js b/src/components/HeaderForm.test.js
new file mode 100644
index 0000000..a3e570e
--- /dev/null
+++ b/src/components/HeaderForm.test.js
@@ -0,0 +1,64 @@
+import { determineRegionIndex, regionStringFromRegionIndex } from "./HeaderForm.js";
+
+
+// test for determineRegionIndex and regionStringFromRegionIndex
+describe("determine regionIndex and corresponding region strings for various region inputs", () => {
+  // TEST #1
+  it("determine regionIndex and regionString for smaller regionInfo lists", async () => {
+    let regionInfo = {
+      chr: [ 'ref', 'ref' ],
+      start: [ '1000', '2000' ],
+      end: [ '2000', '3000' ],
+      desc: [ '17_1_100', 'ref_2000_3000' ],
+      chunk: [ '', '' ],
+      tracks: [ null, null ]
+    }
+    let regionString = "ref:2000-3000";
+    let regionIndex = determineRegionIndex(regionString, regionInfo);
+    expect(regionIndex).toBe(1);
+    expect(regionStringFromRegionIndex(regionIndex, regionInfo)).toBe("ref:2000-3000");
+  })
+  // TEST #2
+  it("determine regionIndex and regionString for larger regionInfo lists", async () => {
+    let regionInfo = {
+      chr: [ '17', 'ref', '17', 'ref', '17', 'ref' ],
+      start: [ '100', '200', '2000', '3000', '4000', '5000' ],
+      end: [ '200', '300', '3000', '4000', '5000', '6000' ],
+      desc: [ '17_100_200', '17_200_300', 'ref_2000_3000', 'ref_3000_4000', 'ref_4000_5000', 'ref_5000_6000' ],
+      chunk: [ '', '', '', '', '', '', '' ],
+      tracks: [ null, null, null, null, null, null ]
+    }
+    let regionString = "17:4000-5000";
+    let regionIndex = determineRegionIndex(regionString, regionInfo);
+    expect(regionIndex).toBe(4);
+    expect(regionStringFromRegionIndex(regionIndex, regionInfo)).toBe("17:4000-5000");
+  })
+  // TEST #3
+  it("determine regionIndex to be null for input of region not found in regionInfo", async () => {
+    let regionInfo = {
+      chr: [ '17', 'ref', '17', 'ref', '17', 'ref' ],
+      start: [ '100', '200', '2000', '3000', '4000', '5000' ],
+      end: [ '200', '300', '3000', '4000', '5000', '6000' ],
+      desc: [ '17_100_200', '17_200_300', 'ref_2000_3000', 'ref_3000_4000', 'ref_4000_5000', 'ref_5000_6000' ],
+      chunk: [ '', '', '', '', '', '', '' ],
+      tracks: [ null, null, null, null, null, null ]
+    }
+    let regionString = "17:5000-7000";
+    let regionIndex = determineRegionIndex(regionString, regionInfo);
+    expect(regionIndex).toBe(null);
+  })
+  // TEST #4
+  it("determine regionIndex and regionString to be null given empty regionInfo", async () => {
+    let regionInfo = {
+      chr: [],
+      start: [],
+      end: [],
+      desc: [],
+      chunk: [],
+      tracks: []
+    }
+    let regionString = "17:5000-7000";
+    let regionIndex = determineRegionIndex(regionString, regionInfo);
+    expect(regionIndex).toBe(null);
+  })
+});