Merge pull request #2239 from concord-consortium/186908262-autoscale-…

…changes

autoscale changes

cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js

@@ -117,14 +117,17 @@ context('XYPlot Tool Tile', function () {
         tableToolTile.getTableCell().eq(6).should('contain', '6');
       });
 
-      cy.log("verify graph dot is updated");
+      cy.log("verify graph dot is added");
       xyTile.getGraphDot().should('have.length', 2);
 
       // X axis should have scaled to fit 5 and 7.
       xyTile.getEditableAxisBox("bottom", "min").invoke('text').then(parseFloat).should("be.within", -1, 5);
       xyTile.getEditableAxisBox("bottom", "max").invoke('text').then(parseFloat).should("be.within", 7, 12);
 
-      cy.log("add another data point");
+      cy.log("add another data point with axes locked");
+      xyTile.getTile().click();
+      clueCanvas.clickToolbarButton("graph", "toggle-lock");
+      clueCanvas.toolbarButtonIsSelected("graph", "toggle-lock");
       cy.get(".primary-workspace").within((workspace) => {
         tableToolTile.typeInTableCell(9, '15');
         tableToolTile.getTableCell(8).should('contain', '15');
@@ -150,6 +153,10 @@ context('XYPlot Tool Tile', function () {
       xyTile.getEditableAxisBox("bottom", "min").invoke('text').then(parseFloat).should("be.within", -1, 5);
       xyTile.getEditableAxisBox("bottom", "max").invoke('text').then(parseFloat).should("be.within", 15, 20);
 
+      // Turn Lock axes off
+      clueCanvas.clickToolbarButton("graph", "toggle-lock");
+      clueCanvas.toolbarButtonIsNotSelected("graph", "toggle-lock");
+
       cy.log("add y2 column to table and show it");
       tableToolTile.getTableTile().click();
       tableToolTile.getAddColumnButton().click();
@@ -560,14 +567,21 @@ context('XYPlot Tool Tile', function () {
       // Drag a point to reposition.  Should start out where we initially clicked
       xAttributeOfTransform(xyTile.getGraphDot().eq(0)).should("be.closeTo", 150, 10);
       yAttributeOfTransform(xyTile.getGraphDot().eq(0)).should("be.closeTo", 50, 10);
-      // {force: true} seems to be necessary, not sure why
-      xyTile.getGraphDot().eq(0).children('circle').eq(1)
-        .trigger("mousedown", 150, 50, { force: true })
-        .trigger("drag", 175, 75, { force: true })
-        .trigger("mouseup", 175, 75, { force: true });
-      cy.wait(1000);
-      xAttributeOfTransform(xyTile.getGraphDot().eq(0)).should("be.closeTo", 175, 10);
-      yAttributeOfTransform(xyTile.getGraphDot().eq(0)).should("be.closeTo", 75, 10);
+      clueCanvas.clickToolbarButton('graph', 'toggle-lock'); // so that we can test position without rescale happening
+
+      xyTile.getGraphDot().eq(0).then(elt => {
+        const currentPos = elt[0].getBoundingClientRect();
+        cy.window().then(win => {
+          xyTile.getGraphDot().eq(0).children('circle').eq(1)
+            .trigger("mousedown", { force: true, view: win })
+            .trigger("mousemove", { force: true, view: win, clientX: currentPos.x+25, clientY: currentPos.y+25 })
+            .trigger("mouseup", { force: true, view: win, clientX: currentPos.x+25, clientY: currentPos.y+25 });
+        });
+        cy.wait(500); // animation
+        xAttributeOfTransform(xyTile.getGraphDot().eq(0)).should("be.closeTo", 175, 10);
+        yAttributeOfTransform(xyTile.getGraphDot().eq(0)).should("be.closeTo", 75, 10);
+        clueCanvas.clickToolbarButton('graph', 'toggle-lock'); // unlock
+      });
 
       // Click toolbar button again to leave edit mode
       clueCanvas.clickToolbarButton('graph', 'move-points');

src/plugins/graph/components/graph-axis.tsx

@@ -17,23 +17,21 @@ import {axisPlaceToAttrRole, kGraphClassSelector} from "../graph-types";
 import {GraphPlace} from "../imports/components/axis-graph-shared";
 import {AttributeLabel} from "./attribute-label";
 import {useDropHintString} from "../imports/hooks/use-drop-hint-string";
-import { isAddCasesAction, isSetCaseValuesAction } from "../../../models/data/data-set-actions";
 import { DroppableAxis } from "./droppable-axis";
 import { useGraphSettingsContext } from "../hooks/use-graph-settings-context";
 import { GraphController } from "../models/graph-controller";
 
 interface IProps {
   place: AxisPlace;
   enableAnimation: MutableRefObject<boolean>;
-  autoAdjust?: React.MutableRefObject<boolean>;
   controller: GraphController;
   onDropAttribute?: (place: GraphPlace, dataSet: IDataSet, attrId: string) => void;
   onRemoveAttribute?: (place: GraphPlace, attrId: string) => void;
   onTreatAttributeAs?: (place: GraphPlace, attrId: string, treatAs: AttributeType) => void;
 }
 
 export const GraphAxis = observer(function GraphAxis({
-  place, enableAnimation, autoAdjust, controller, onDropAttribute, onRemoveAttribute, onTreatAttributeAs
+  place, enableAnimation, controller, onDropAttribute, onRemoveAttribute, onTreatAttributeAs
 }: IProps) {
   const dataConfig = useDataConfigurationContext(), // FIXME mult-dataset.
     isDropAllowed = dataConfig?.graphPlaceCanAcceptAttributeIDDrop ?? (() => true),
@@ -87,24 +85,6 @@ export const GraphAxis = observer(function GraphAxis({
     });
   }, [layout, place, wrapperElt]);
 
-  useEffect(() => {
-    if (autoAdjust?.current) {
-      // Set up listener on each layer for changes that require a rescale
-      for (const layer of graphModel.layers) {
-        layer.config?.onAction(action => {
-          if (isAlive(graphModel) &&
-            !graphModel.lockAxes &&
-            !graphModel.interactionInProgress &&
-            (isAddCasesAction(action) || isSetCaseValuesAction(action))) {
-            controller.autoscaleAllAxes();
-          }
-        });
-      }
-    }
-  // we just want this to run once to set up the handlers, not every time something changes.
-  // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [autoAdjust]);
-
   useEffect(function cleanup () {
     return () => {
       // This gets called when the component is unmounted, which happens when the graph is closed.

src/plugins/graph/components/graph-component.tsx

@@ -29,9 +29,8 @@ export const GraphComponent = observer(
   const graphRef = useRef<HTMLDivElement | null>(null);
   const {width, height} = useResizeDetector<HTMLDivElement>({ targetRef: graphRef });
   const enableAnimation = useRef(true);
-  const autoAdjustAxes = useRef(true);
   const graphController = useMemo(
-    () => new GraphController({layout, enableAnimation, instanceId, autoAdjustAxes}),
+    () => new GraphController({layout, enableAnimation, instanceId}),
     [layout, instanceId]
   );
 

src/plugins/graph/components/graph.tsx

@@ -47,7 +47,7 @@ export const Graph = observer(
     function Graph({ graphController, readOnly, graphRef, onRequestRowHeight }: IProps) {
 
   const graphModel = useGraphModelContext(),
-    {autoAdjustAxes, enableAnimation} = graphController,
+    {enableAnimation} = graphController,
     {plotType} = graphModel,
     instanceId = useInstanceIdContext(),
     marqueeState = useMemo<MarqueeState>(() => new MarqueeState(), []),
@@ -185,7 +185,6 @@ export const Graph = observer(
                         place={place}
                         controller={graphController}
                         enableAnimation={enableAnimation}
-                        autoAdjust={autoAdjustAxes}
                         onDropAttribute={handleChangeAttribute}
                         onRemoveAttribute={handleRemoveAttribute}
                         onTreatAttributeAs={handleTreatAttrAs}

src/plugins/graph/components/scatterdots.tsx

@@ -1,15 +1,14 @@
-import {ScaleBand, ScaleLinear, select} from "d3";
-import React, {useCallback, useRef, useState} from "react";
+import {D3DragEvent, drag, ScaleBand, ScaleLinear, select} from "d3";
+import React, {useCallback, useRef} from "react";
 import {ScaleNumericBaseType} from "../imports/components/axis/axis-types";
-import {CaseData, inGraphDot} from "../d3-types";
-import {PlotProps} from "../graph-types";
-import {useDragHandlers, usePlotResponders} from "../hooks/use-plot";
+import {CaseData, inGraphDot, selectGraphDots} from "../d3-types";
+import {PlotProps, Point} from "../graph-types";
+import { usePlotResponders} from "../hooks/use-plot";
 import {useDataConfigurationContext} from "../hooks/use-data-configuration-context";
 import {useGraphLayoutContext} from "../models/graph-layout";
 import {ICase} from "../../../models/data/data-set-types";
 import { useGraphModelContext } from "../hooks/use-graph-model-context";
 import {
-  // getScreenCoord,
   handleClickOnDot,
   setPointCoordinates,
   setPointSelection
@@ -29,8 +28,6 @@ export const ScatterDots = function ScatterDots(props: PlotProps) {
     layout = useGraphLayoutContext(),
     legendAttrID = dataConfiguration?.attributeID('legend') as string,
     yScaleRef = useRef<ScaleNumericBaseType>(),
-    [dragID, setDragID] = useState(''),
-    currPos = useRef({x: 0, y: 0}),
     target = useRef<any>(),
     plotNumRef = useRef(0);
 
@@ -40,85 +37,67 @@ export const ScatterDots = function ScatterDots(props: PlotProps) {
   dragPointRadiusRef.current = graphModel.getPointRadius('hover-drag');
   yScaleRef.current = layout.getAxisScale("left") as ScaleNumericBaseType;
 
-  const onDragStart = useCallback((event: MouseEvent) => {
-    if (graphModel.editingMode !== "none") {
-      const targetDot = event.target && inGraphDot(event.target as SVGSVGElement);
+  const
+    onDragStart = useCallback((event: D3DragEvent<SVGGElement,CaseData,Point>, datum) => {
+      const targetDot = event.sourceEvent.target && inGraphDot(event.sourceEvent.target as SVGSVGElement);
       if (!targetDot) return;
       target.current = select(targetDot as SVGSVGElement);
-      const aCaseData: CaseData = target.current.node().__data__;
-      if (!aCaseData || !dataConfiguration || aCaseData.dataConfigID !== dataConfiguration.id) return;
-      event.stopPropagation();
+      if (!datum || !dataConfiguration || datum.dataConfigID !== dataConfiguration.id) return;
+      handleClickOnDot(event.sourceEvent, datum, dataConfiguration);
       graphModel.setInteractionInProgress(true);
       dataset?.beginCaching();
       secondaryAttrIDsRef.current = dataConfiguration.yAttributeIDs || [];
       enableAnimation.current = false; // We don't want to animate points until end of drag
-      const tItsID = aCaseData.caseID;
-      plotNumRef.current = target.current.datum()?.plotNum ?? 0;
-      // TODO the radius transition doesn't actually do anything
-      target.current
-        .property('isDragging', true)
-        .transition()
-        .attr('r', dragPointRadiusRef.current);
-      setDragID(tItsID);
-      currPos.current = { x: event.clientX, y: event.clientY };
-
-      handleClickOnDot(event, aCaseData, dataConfiguration);
-    }
-  }, [dataConfiguration, dataset, enableAnimation, graphModel]),
-
-    updatePositions = useCallback((event: MouseEvent, forceUpdate: boolean) => {
-      if (dragID !== '') {
-        event.stopPropagation();
-        event.preventDefault();
-        const xAxisScale = layout.getAxisScale('bottom') as ScaleLinear<number, number>,
-          xAttrID = dataConfiguration?.attributeID('x') ?? '';
-        const newPos = {x: event.clientX, y: event.clientY},
-          dx = newPos.x - currPos.current.x,
-          dy = newPos.y - currPos.current.y;
-        currPos.current = newPos;
-        if (forceUpdate || dx !== 0 || dy !== 0) {
-          const deltaX = Number(xAxisScale.invert(dx)) - Number(xAxisScale.invert(0)),
-            deltaY = Number(yScaleRef.current?.invert(dy)) - Number(yScaleRef.current?.invert(0)),
-            caseValues: ICase[] = [];
-            const cellSelection = dataConfiguration?.dataset?.selectedCells;
-            cellSelection?.forEach(cell => {
-            const currX = Number(dataset?.getNumeric(cell.caseId, xAttrID)),
-              currY = Number(dataset?.getNumeric(cell.caseId, cell.attributeId));
-            if (isFinite(currX) && isFinite(currY)) {
-              caseValues.push({
-                __id__: cell.caseId,
-                [xAttrID]: currX + deltaX,
-                [cell.attributeId]: currY + deltaY
-              });
-            }
+      plotNumRef.current = datum.plotNum ?? 0;
+    }, [dataConfiguration, dataset, enableAnimation, graphModel]),
+
+    updatePositions = useCallback((dx: number, dy: number) => {
+      const
+        xAxisScale = layout.getAxisScale('bottom') as ScaleLinear<number, number>,
+        xAttrID = dataConfiguration?.attributeID('x') ?? '',
+        deltaX = Number(xAxisScale.invert(dx)) - Number(xAxisScale.invert(0)),
+        deltaY = Number(yScaleRef.current?.invert(dy)) - Number(yScaleRef.current?.invert(0)),
+        caseValues: ICase[] = [],
+        cellSelection = dataConfiguration?.dataset?.selectedCells;
+      cellSelection?.forEach(cell => {
+        const
+          currX = Number(dataset?.getNumeric(cell.caseId, xAttrID)),
+          currY = Number(dataset?.getNumeric(cell.caseId, cell.attributeId));
+        if (isFinite(currX) && isFinite(currY)) {
+          caseValues.push({
+            __id__: cell.caseId,
+            [xAttrID]: currX + deltaX,
+            [cell.attributeId]: currY + deltaY
           });
-          caseValues.length &&
-            dataset?.setCanonicalCaseValues(caseValues);
         }
-      }
-    }, [layout, dataConfiguration, dataset, dragID]),
+      });
+      caseValues.length &&
+        dataset?.setCanonicalCaseValues(caseValues);
+    }, [layout, dataConfiguration, dataset]),
 
-    onDrag = useCallback((event: MouseEvent) => {
-      updatePositions(event, false);
+    onDrag = useCallback((event: D3DragEvent<SVGGElement,CaseData,Point>) => {
+      if (event.dx !== 0 || event.dy !== 0) {
+        updatePositions(event.dx, event.dy);
+      }
     }, [updatePositions]),
 
-    onDragEnd = useCallback((event: MouseEvent) => {
-      if (dragID !== '') {
+    onDragEnd = useCallback((event: D3DragEvent<SVGGElement,CaseData,Point>) => {
         graphModel.setInteractionInProgress(false);
         // Final update does a rescale if appropriate
-        updatePositions(event, true);
-        // TODO the radius transition doesn't actually do anything
-        target.current
-          .classed('dragging', false)
-          .property('isDragging', false)
-          .transition()
-          .attr('r', selectedPointRadiusRef.current);
-        setDragID(() => '');
+        updatePositions(event.dx, event.dy);
         target.current = null;
-      }
-    }, [dragID, graphModel, updatePositions]);
-
-  useDragHandlers(window, {start: onDragStart, drag: onDrag, end: onDragEnd});
+    }, [graphModel, updatePositions]),
+
+    refreshDragHandlers = useCallback(() => {
+      selectGraphDots(dotsRef.current)
+        ?.call(drag<SVGGElement,CaseData,Point>()
+          .filter(() => graphModel.editingMode !== "none")
+          .subject((event) => ({ x: event.x, y: event.y }))
+          .on('start', onDragStart)
+          .on('drag', onDrag)
+          .on('end', onDragEnd)
+        );
+    }, [dotsRef, graphModel, onDrag, onDragEnd, onDragStart]);
 
   const refreshPointSelection = useCallback(() => {
     const { pointColor, pointStrokeColor } = graphModel;
@@ -169,8 +148,9 @@ export const ScatterDots = function ScatterDots(props: PlotProps) {
       selectedOnly, getScreenX, getScreenY, getLegendColor,
       getColorForId: graphModel.getColorForId, enableAnimation, pointColor, pointStrokeColor
     });
+    refreshDragHandlers();
   }, [dataConfiguration, dataset, dotsRef, layout, legendAttrID,
-    enableAnimation, graphModel, yScaleRef]);
+    enableAnimation, graphModel, yScaleRef, refreshDragHandlers]);
 
   // const refreshPointPositionsSVG = useCallback((selectedOnly: boolean) => {
   //   const xAttrID = dataConfiguration?.attributeID('x') ?? '',

src/plugins/graph/d3-types.ts

@@ -12,7 +12,7 @@ export type DotsElt = SVGSVGElement | null;
 //  CaseData: type of data attached to selected element
 //  SVGSVGElement: type of parent element selected by initial/global select
 //  unknown: type of data attached to parent element (none in this case)
-export type DotSelection = Selection<SVGCircleElement, CaseData, SVGSVGElement, unknown>;
+export type DotSelection = Selection<SVGGElement, CaseData, SVGSVGElement, unknown>;
 
 export const graphDotSelector = "g.graph-dot";
 

src/plugins/graph/hooks/use-graph-model.ts

@@ -12,39 +12,12 @@ interface IProps {
 export function useGraphModel(props: IProps) {
   const { graphModel, enableAnimation, instanceId } = props;
 
-  // const callMatchCirclesToData = useCallback((layer) => {
-  //   matchCirclesToData({
-  //     dataConfiguration: layer.config,
-  //     pointRadius: graphModel.getPointRadius(),
-  //     pointColor: graphModel.pointColor,
-  //     pointStrokeColor: graphModel.pointStrokeColor,
-  //     dotsElement: layer.dotsElt,
-  //     enableAnimation, instanceId
-  //   });
-  // }, [graphModel, enableAnimation, instanceId]);
-
   const callMatchAllCirclesToData = useCallback(() => {
     matchAllCirclesToData({
       graphModel, enableAnimation, instanceId
     });
   }, [graphModel, enableAnimation, instanceId]);
 
-  // respond to added/removed cases
-  // TODO seems redundant with use-plot.ts handleAddRemoveCases
-  // useEffect(function installAddRemoveCaseHandler() {
-  //   const disposers: (()=>void)[] = [];
-  //   for (const layer of graphModel.layers) {
-  //     console.log("registering layer responder");
-  //     disposers.push(layer.config.onAction(action => {
-  //       console.log('layer responder examining', action);
-  //       if (isAddCasesAction(action) || isRemoveCasesAction(action)) {
-  //         callMatchCirclesToData(layer);
-  //       }
-  //     }));
-  //   }
-  //   return () => { disposers.forEach((d) => { d(); }); };
-  // }, [graphModel.layers, callMatchCirclesToData]);
-
   // respond to change in plotType
   useEffect(function installPlotTypeAction() {
     const disposer = onAnyAction(graphModel, action => {