fix view stack bug (#1640)

client/src/actions/index.js

@@ -1,10 +1,5 @@
 import * as globals from "../globals";
-import {
-  AnnoMatrixLoader,
-  AnnoMatrixObsCrossfilter,
-  clip,
-  isubsetMask,
-} from "../annoMatrix";
+import { AnnoMatrixLoader, AnnoMatrixObsCrossfilter } from "../annoMatrix";
 import {
   catchErrorsWrap,
   doJsonRequest,
@@ -16,6 +11,7 @@ import {
 import { loadUserColorConfig } from "../util/stateManager/colorHelpers";
 import * as selnActions from "./selection";
 import * as annoActions from "./annotation";
+import * as viewActions from "./viewStack";
 
 /*
 return promise fetching user-configured colors
@@ -176,75 +172,6 @@ const requestDifferentialExpression = (set1, set2, num_genes = 10) => async (
   }
 };
 
-const clipAction = (min, max) => (dispatch, getState) => {
-  /*
-  apply a clip to the current annoMatrix.  By convention, the clip
-  view is ALWAYS the top view.
-  */
-  const { annoMatrix: prevAnnoMatrix } = getState();
-  const annoMatrix = prevAnnoMatrix.isClipped
-    ? clip(prevAnnoMatrix.viewOf, min, max)
-    : clip(prevAnnoMatrix, min, max);
-  const obsCrossfilter = new AnnoMatrixObsCrossfilter(annoMatrix);
-  dispatch({
-    type: "set clip quantiles",
-    clipQuantiles: { min, max },
-    annoMatrix,
-    obsCrossfilter,
-  });
-};
-
-const subsetAction = () => (dispatch, getState) => {
-  /*
-  Subset the annoMatrix to the current crossfilter selection
-  */
-  const {
-    annoMatrix: prevAnnoMatrix,
-    obsCrossfilter: prevObsCrossfilter,
-  } = getState();
-
-  const annoMatrix = isubsetMask(
-    prevAnnoMatrix,
-    prevObsCrossfilter.allSelectedMask()
-  );
-  const obsCrossfilter = new AnnoMatrixObsCrossfilter(annoMatrix);
-  dispatch({
-    type: "subset to selection",
-    annoMatrix,
-    obsCrossfilter,
-  });
-};
-
-const resetSubsetAction = () => (dispatch, getState) => {
-  /*
-  Reset the annoMatrix to all data.  Because we may have multiple views
-  stacked, we pop them all.  By convention, any clip transformation will
-  be the top of the stack, and must be preserved.
-  */
-
-  const { annoMatrix: prevAnnoMatrix } = getState();
-
-  const clipRange = prevAnnoMatrix.isClipped ? prevAnnoMatrix.clipRange : null;
-
-  /* pop all views */
-  let annoMatrix = prevAnnoMatrix;
-  while (annoMatrix.isView) {
-    annoMatrix = annoMatrix.viewOf;
-  }
-
-  /* re-apply the clip, if any */
-  if (clipRange !== null) {
-    annoMatrix = clip(annoMatrix, ...clipRange);
-  }
-
-  const obsCrossfilter = new AnnoMatrixObsCrossfilter(annoMatrix);
-  dispatch({
-    type: "reset subset",
-    annoMatrix,
-    obsCrossfilter,
-  });
-};
-
 function fetchJson(pathAndQuery) {
   return doJsonRequest(
     `${globals.API.prefix}${globals.API.version}${pathAndQuery}`
@@ -270,9 +197,9 @@ export default {
   graphLassoEndAction: selnActions.graphLassoEndAction,
   graphLassoCancelAction: selnActions.graphLassoCancelAction,
   graphLassoDeselectAction: selnActions.graphLassoDeselectAction,
-  clipAction,
-  subsetAction,
-  resetSubsetAction,
+  clipAction: viewActions.clipAction,
+  subsetAction: viewActions.subsetAction,
+  resetSubsetAction: viewActions.resetSubsetAction,
   annotationCreateCategoryAction: annoActions.annotationCreateCategoryAction,
   annotationRenameCategoryAction: annoActions.annotationRenameCategoryAction,
   annotationDeleteCategoryAction: annoActions.annotationDeleteCategoryAction,

client/src/actions/viewStack.js

@@ -0,0 +1,100 @@
+/*
+The following actions manage the view stack for annoMatrix.
+
+Conventions used and assumed elsewhere in the code base:
+  * there will be zero or one clip view, and it will be the TOP view always.
+  * there will be zero or more subset views
+
+In other words, in our current use, we do not stack multiple clip views but we do
+stack multiple subsets.
+
+If these conventions change, code elsewhere (eg. menubar/clip.js) will need to
+change as well.
+*/
+import { AnnoMatrixObsCrossfilter, clip, isubsetMask } from "../annoMatrix";
+
+export const clipAction = (min, max) => (dispatch, getState) => {
+  /*
+  apply a clip to the current annoMatrix.  By convention, the clip
+  view is ALWAYS the top view.
+  */
+  const { annoMatrix: prevAnnoMatrix } = getState();
+  const annoMatrix = prevAnnoMatrix.isClipped
+    ? clip(prevAnnoMatrix.viewOf, min, max)
+    : clip(prevAnnoMatrix, min, max);
+  const obsCrossfilter = new AnnoMatrixObsCrossfilter(annoMatrix);
+  dispatch({
+    type: "set clip quantiles",
+    clipQuantiles: { min, max },
+    annoMatrix,
+    obsCrossfilter,
+  });
+};
+
+export const subsetAction = () => (dispatch, getState) => {
+  /*
+  Subset the annoMatrix to the current crossfilter selection by pushing a 
+  subset view.
+
+  By convention, a clip view is ALWAYS the top view, so if present, pop
+  off and re-apply
+  */
+  const {
+    annoMatrix: prevAnnoMatrix,
+    obsCrossfilter: prevObsCrossfilter,
+  } = getState();
+
+  let annoMatrix;
+  if (prevAnnoMatrix.isClipped) {
+    // if there is a clip view, pop it and reapply after we subset
+    const { clipRange } = prevAnnoMatrix;
+    annoMatrix = isubsetMask(
+      prevAnnoMatrix.viewOf,
+      prevObsCrossfilter.allSelectedMask()
+    );
+    annoMatrix = clip(annoMatrix, ...clipRange);
+  } else {
+    // else just push a subset view.
+    annoMatrix = isubsetMask(
+      prevAnnoMatrix,
+      prevObsCrossfilter.allSelectedMask()
+    );
+  }
+
+  const obsCrossfilter = new AnnoMatrixObsCrossfilter(annoMatrix);
+  dispatch({
+    type: "subset to selection",
+    annoMatrix,
+    obsCrossfilter,
+  });
+};
+
+export const resetSubsetAction = () => (dispatch, getState) => {
+  /*
+  Reset the annoMatrix to all data.  Because we may have multiple views
+  stacked, we pop them all.  By convention, any clip transformation will
+  be the top of the stack, and must be preserved.
+  */
+
+  const { annoMatrix: prevAnnoMatrix } = getState();
+
+  const clipRange = prevAnnoMatrix.isClipped ? prevAnnoMatrix.clipRange : null;
+
+  /* pop all views */
+  let annoMatrix = prevAnnoMatrix;
+  while (annoMatrix.isView) {
+    annoMatrix = annoMatrix.viewOf;
+  }
+
+  /* re-apply the clip, if any */
+  if (clipRange !== null) {
+    annoMatrix = clip(annoMatrix, ...clipRange);
+  }
+
+  const obsCrossfilter = new AnnoMatrixObsCrossfilter(annoMatrix);
+  dispatch({
+    type: "reset subset",
+    annoMatrix,
+    obsCrossfilter,
+  });
+};