Merge pull request #2213 from concord-consortium/186610168-manual-points

Add points by hand toolbar button

cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js

@@ -468,5 +468,28 @@ context('XYPlot Tool Tile', function () {
       // Only the unlink remove button should remain
       xyTile.getRemoveVariablesButtons().should("have.length", 1);
     });
+
+    it("Test points by hand", () => {
+      beforeTest(queryParamsMultiDataset);
+      cy.log("Add XY Plot Tile");
+      cy.collapseResourceTabs();
+      clueCanvas.addTile("graph");
+      xyTile.getTile().should('be.visible');
+
+      clueCanvas.clickToolbarButton("graph", "add-points-by-hand");
+      xyTile.getXAttributesLabel().should('have.length', 1).should("contain.text", "X Variable");
+      xyTile.getYAttributesLabel().should('have.length', 1).should("contain.text", "Y Variable 1");
+      xyTile.getLayerName().should('have.length', 1).should("contain.text", "Added by hand");
+      xyTile.getLayerNameInput().should('not.be.visible');
+
+      xyTile.getLayerNameEditButton().click();
+      xyTile.getLayerNameEditButton().should('have.length', 0);
+      xyTile.getLayerNameInput().should('be.visible').type('Renamed{enter}');
+      xyTile.getLayerNameInput().should('not.be.visible');
+      xyTile.getLayerName().should('have.length', 1).should("contain.text", "Renamed");
+    });
+
   });
+
+
 });

cypress/support/elements/tile/XYPlotToolTile.js

@@ -51,6 +51,15 @@ class XYPlotToolTile {
   getYAxisInput(workspaceClass) {
     return this.getAxisInput("left", workspaceClass);
   }
+  getLayerName(workspaceClass) {
+    return cy.get(`${wsClass(workspaceClass)} .canvas-area .multi-legend .legend-row .layer-name`);
+  }
+  getLayerNameEditButton(workspaceClass) {
+    return cy.get(`${wsClass(workspaceClass)} .canvas-area .multi-legend .legend-row .layer-name button`);
+  }
+  getLayerNameInput(workspaceClass) {
+    return cy.get(`${wsClass(workspaceClass)} .canvas-area .multi-legend .legend-row .layer-name input`);
+  }
   getXAttributesLabel(workspaceClass) {
     return cy.get(`${wsClass(workspaceClass)} .canvas-area .multi-legend .legend-row .bottom .simple-attribute-label`);
   }

src/clue/app-config.json

@@ -269,6 +269,7 @@
         "disableAttributeDnD": true,
         "tools": [
           "link-tile",
+          "add-points-by-hand",
           "|",
           "fit-all",
           "toggle-lock"

src/components/utilities/editable-label-with-button.scss

@@ -0,0 +1,23 @@
+@import "../vars.sass";
+
+.chakra-editable {
+  .chakra-editable__preview[hidden] {
+    display: none;
+  }
+
+  button {
+    background: inherit;
+    border: none;
+    cursor: pointer;
+
+    &:hover {
+      background-color: $workspace-teal-light-6;
+    }
+
+    svg {
+      height: 1em;
+      width: 1em;
+      vertical-align: baseline;
+    }
+  }
+}

src/components/utilities/editable-label-with-button.tsx

@@ -0,0 +1,40 @@
+import React from "react";
+import { observer } from "mobx-react";
+import { Editable, EditableInput, EditablePreview, useEditableControls } from "@chakra-ui/react";
+
+import EditIcon from "../../assets/edit.svg";
+
+import "./editable-label-with-button.scss";
+
+interface IProps {
+  defaultValue: string|undefined;
+  onSubmit: (value:string) => void;
+}
+
+export const EditableLabelWithButton = observer(function EditableDataSetName({defaultValue, onSubmit}: IProps) {
+
+  function EditButton() {
+    const { isEditing, getEditButtonProps } = useEditableControls();
+    if (!isEditing) {
+      return (
+        <button aria-label="Edit name" {...getEditButtonProps()}>
+          <EditIcon/>
+        </button>
+      );
+    } else {
+      return null;
+    }
+  }
+
+  return (
+    <Editable
+      defaultValue={defaultValue}
+      isPreviewFocusable={false}
+      onSubmit={onSubmit}
+    >
+      <EditablePreview />
+      <EditableInput />
+      <EditButton />
+    </Editable>
+  );
+});

src/models/data/data-set.ts

@@ -639,6 +639,7 @@ export const DataSet = types.model("DataSet", {
         for (let i = attribute.values.length; i < self.cases.length; ++i) {
           attribute.values.push("");
         }
+        return attribute;
       },
 
       setAttributeName(attributeID: string, name: string) {
@@ -882,7 +883,7 @@ export function addAttributeToDataSet(dataset: IDataSet, snapshot: IAttributeSna
   if (!snapshot.id) {
     snapshot.id = uniqueId();
   }
-  dataset.addAttributeWithID(snapshot, beforeID);
+  return dataset.addAttributeWithID(snapshot, beforeID);
 }
 
 export function addCasesToDataSet(dataset: IDataSet, cases: ICaseCreation[], beforeID?: string | string[]) {

src/plugins/graph/components/graph-toolbar-registration.tsx

@@ -13,6 +13,7 @@ import AddIcon from "../../../assets/icons/add-data-graph-icon.svg";
 import FitAllIcon from "../assets/fit-all-icon.svg";
 import LockAxesIcon from "../assets/lock-axes-icon.svg";
 import UnlockAxesIcon from "../assets/unlock-axes-icon.svg";
+import AddPointsByHandIcon from "../assets/add-points-by-hand-icon.svg";
 
 function LinkTileButton(name: string, title: string, allowMultiple: boolean) {
 
@@ -88,6 +89,34 @@ const ToggleLockAxesButton = observer(function ToggleLockAxesButton({name}: IToo
   );
 });
 
+const AddPointsByHandButton = observer(function AddPointsByHandButton({name}: IToolbarButtonComponentProps) {
+  const graph = useGraphModelContext();
+
+  const hasEditableLayers = graph.getEditableLayers().length > 0;
+
+  // Enable button if axes are numeric or undefined.
+  const isNumeric = (graph.attributeType("x")||"numeric") === "numeric"
+    && (graph.attributeType("y")||"numeric") === "numeric";
+
+  const enabled = isNumeric && !hasEditableLayers;
+
+  function handleClick() {
+    graph.createEditableLayer();
+  }
+
+  return (
+    <TileToolbarButton
+      name={name}
+      title="Add points by hand"
+      onClick={handleClick}
+      disabled={!enabled}
+    >
+      <AddPointsByHandIcon/>
+    </TileToolbarButton>
+  );
+
+});
+
 registerTileToolbarButtons("graph",
 [
   {
@@ -105,5 +134,9 @@ registerTileToolbarButtons("graph",
   {
     name: 'toggle-lock',
     component: ToggleLockAxesButton
+  },
+  {
+    name: 'add-points-by-hand',
+    component: AddPointsByHandButton
   }
 ]);

src/plugins/graph/components/legend/layer-legend.tsx

@@ -12,13 +12,15 @@ import { DataConfigurationContext, useDataConfigurationContext } from "../../hoo
 import { IGraphLayerModel } from "../../models/graph-layer-model";
 import { LegendDropdown } from "./legend-dropdown";
 import { LegendIdListFunction, ILegendHeightFunctionProps, ILegendPartProps } from "./legend-types";
-import RemoveDataIcon from "../../assets/remove-data-icon.svg";
-import XAxisIcon from "../../assets/x-axis-icon.svg";
-import YAxisIcon from "../../assets/y-axis-icon.svg";
 import { logSharedModelDocEvent } from "../../../../models/document/log-shared-model-document-event";
 import { LogEventName } from "../../../../lib/logger-types";
 import { useTileModelContext } from "../../../../components/tiles/hooks/use-tile-model-context";
+import { EditableLabelWithButton } from "../../../../components/utilities/editable-label-with-button";
+import { GraphLayerContext, useGraphLayerContext } from "../../hooks/use-graph-layer-context";
 
+import RemoveDataIcon from "../../assets/remove-data-icon.svg";
+import XAxisIcon from "../../assets/x-axis-icon.svg";
+import YAxisIcon from "../../assets/y-axis-icon.svg";
 
 export const layerLegendType = "layer-legend";
 
@@ -46,6 +48,7 @@ function ColorKey({ color }: IColorKeyProps) {
 const SingleLayerLegend = observer(function SingleLayerLegend(props: ILegendPartProps) {
   let legendItems = [] as React.ReactNode[];
   const graphModel = useGraphModelContext();
+  const layer = useGraphLayerContext();
   const dataConfiguration = useDataConfigurationContext();
   const readOnly = useReadOnlyContext();
   const { tile } = useTileModelContext();
@@ -72,6 +75,23 @@ const SingleLayerLegend = observer(function SingleLayerLegend(props: ILegendPart
     }
   }
 
+
+  const dataSetName = dataConfiguration?.dataset?.name || "Unknown";
+
+  function handleSetDataSetName (value: string) {
+    if (value) {
+      dataConfiguration?.dataset?.setName(value);
+    }
+  }
+
+  const layerName =
+  <span className="layer-name">
+    {layer.editable
+      ? <EditableLabelWithButton defaultValue={dataSetName} onSubmit={handleSetDataSetName}/>
+      : dataSetName
+    }
+  </span>;
+
   if (dataConfiguration) {
     const yAttributes = dataConfiguration.yAttributeDescriptions;
 
@@ -136,7 +156,7 @@ const SingleLayerLegend = observer(function SingleLayerLegend(props: ILegendPart
               </div>
             }
             <div className="legend-title">
-              Data from: <strong>{dataConfiguration.dataset.name || "Unknown"}</strong>&nbsp;
+              Data from: {layerName}
             </div>
           </div>
           <div className="legend-cell-2">
@@ -170,11 +190,13 @@ export const LayerLegend = observer(function LayerLegend(props: ILegendPartProps
       {
         graphModel.layers.map((layer) => {
           return (
-            <DataConfigurationContext.Provider key={layer.id} value={layer.config}>
-              <SingleLayerLegend {...props} />
-            </DataConfigurationContext.Provider>);
-          }
-        )
+            <GraphLayerContext.Provider key={layer.id} value={layer}>
+              <DataConfigurationContext.Provider value={layer.config}>
+                <SingleLayerLegend {...props} />
+              </DataConfigurationContext.Provider>
+            </GraphLayerContext.Provider>
+          );
+        })
       }
     </>
   );

src/plugins/graph/components/legend/multi-legend.scss

@@ -25,6 +25,20 @@
 
     .legend-title {
       padding-left: 6px;
+
+      .layer-name {
+        font-weight: bold;
+      }
+
+      .chakra-editable {
+        display: inline-block;
+        margin-left: 6px;
+
+        input {
+          font-weight: normal;
+        }
+      }
+
     }
   }
 

src/plugins/graph/models/graph-layer-model.ts

@@ -15,7 +15,9 @@ export const GraphLayerModel = types
   .model('GraphLayerModel')
   .props({
     id: types.optional(types.identifier, () => typedId("LAYR")),
-    config: types.optional(DataConfigurationModel, () => DataConfigurationModel.create())
+    config: types.optional(DataConfigurationModel, () => DataConfigurationModel.create()),
+    // Whether this layer contains "points by hand" that can be edited in the graph
+    editable: false
   })
   .volatile(self => ({
     autoAssignedAttributes: [] as Array<{ place: GraphPlace, role: GraphAttrRole, dataSetID: string, attrID: string }>,
@@ -90,6 +92,7 @@ export const GraphLayerModel = types
     configureUnlinkedLayer() {
       if (!self.config.isEmpty) {
         self.config.clearAttributes();
+        self.editable = false;
       }
     },
     setDataSetListener() {

src/plugins/graph/models/graph-model.test.ts

@@ -126,34 +126,50 @@ describe('GraphModel', () => {
       expect(graphModel.layers[1].config.dataset).toEqual(sharedDataSet2.dataSet);
     });
 
+    it('supports adding an editable layer', () => {
+      if (!graphModel) fail('No graph model'); // reuses data from previous test
+      expect(graphModel.layers.length).toBe(2);
+      expect(graphModel.layers[0].editable).toBe(false);
+      graphModel.createEditableLayer();
+      expect(graphModel.layers.length).toBe(3);
+      const layer = graphModel.layers[2];
+      expect(layer.editable).toBe(true);
+      expect(layer.config.attributeDescriptions.x.type).toEqual("numeric");
+      expect(layer.config.attributeDescriptions.y.type).toEqual("numeric");
+      expect(layer.config.dataset?.name).toEqual("Added by hand");
+      expect(layer.config.dataset?.attributes.map(a => a.name)).toEqual(["X Variable", "Y Variable 1"]);
+    });
+
     it('supports removing layers', () => {
-      if (!graphModel) fail('No graph model');
+      if (!graphModel) fail('No graph model'); // reuses data from previous test
       const smm = getSharedModelManager(graphModel);
       smm?.removeTileSharedModel(graphModel, sharedDataSet2);
       graphModel.updateAfterSharedModelChanges(sharedDataSet);
       // Currently Metadata remains attached - doesn't seem like correct behavior longer term though
-      expect(getTileSharedModels(graphModel)).toHaveLength(3);
-      expect(graphModel.layers.length).toBe(1);
+      expect(getTileSharedModels(graphModel)).toHaveLength(5);
+      expect(graphModel.layers.length).toBe(2);
       expect(graphModel.layers[0].isLinked).toBe(true);
       expect(graphModel.layers[0].config.dataset).toEqual(sharedDataSet.dataSet);
     });
 
     it("re-uses existing metadata if present", () => {
-      if (!graphModel) fail('No graph model');
+      if (!graphModel) fail('No graph model'); // reuses data from previous test
       const smm = getSharedModelManager(graphModel);
       smm?.addSharedModel(sharedDataSet2);
       smm?.addTileSharedModel(graphModel, sharedDataSet2);
       graphModel.updateAfterSharedModelChanges(sharedDataSet2);
-      expect(getTileSharedModels(graphModel)).toHaveLength(4);
-      expect(graphModel.layers.length).toBe(2);
+      expect(getTileSharedModels(graphModel)).toHaveLength(6);
+      expect(graphModel.layers.length).toBe(3);
       expect(graphModel.layers[0].isLinked).toBe(true);
       expect(graphModel.layers[0].config.dataset).toEqual(sharedDataSet.dataSet);
       expect(graphModel.layers[1].isLinked).toBe(true);
-      expect(graphModel.layers[1].config.dataset).toEqual(sharedDataSet2.dataSet);
+      expect(graphModel.layers[1].editable).toEqual(true);
+      expect(graphModel.layers[2].isLinked).toBe(true);
+      expect(graphModel.layers[2].config.dataset).toEqual(sharedDataSet2.dataSet);
     });
 
     it("cycles through colors properly", () => {
-      if (!graphModel) fail("No graph model");
+      if (!graphModel) fail("No graph model"); // reuses data from previous test
       function getUniqueColorIndices() {
         const uniqueColorIndices: number[] = [];
         graphModel._idColors.forEach(colorIndex => {