diff --git a/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js b/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js index c120361061..dc391a5b1e 100644 --- a/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js @@ -40,6 +40,23 @@ function buildTable(data) { }); } +// Parse `transform` attributes (used for point positioning) +function xAttributeOfTransform(matcher) { + return attributeOfTransform(matcher, 1); +} +function yAttributeOfTransform(matcher) { + return attributeOfTransform(matcher, 2); +} +function attributeOfTransform(matcher, n) { + return matcher + .invoke('attr', 'transform') + .then(transform => { + return transform.match(/translate\((-?[0-9.]+), *(-?[0-9.]+)\)/)[n]; + }) + .then(parseFloat); +} + + function beforeTest(params) { cy.clearQAData('all'); cy.visit(params); @@ -80,6 +97,8 @@ context('XYPlot Tool Tile', function () { cy.log("Link Table"); clueCanvas.clickToolbarButton('graph', 'link-tile-multiple'); xyTile.linkTable("Table Data 1"); + cy.wait(1000); // Needs a little extra time, probably due to legend resizing. + // Otherwise the upcoming typeInTableCell fails. cy.log("shows edit boxes on axes"); xyTile.getEditableAxisBox("bottom", "min").should("exist"); @@ -469,27 +488,65 @@ context('XYPlot Tool Tile', function () { xyTile.getRemoveVariablesButtons().should("have.length", 1); }); - it("Test points by hand", () => { + it('Test points by hand', () => { beforeTest(queryParamsMultiDataset); - cy.log("Add XY Plot Tile"); + 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"); + clueCanvas.addTile('graph'); + xyTile.getTile('.primary-workspace').should('be.visible'); + clueCanvas.toolbarButtonIsDisabled('graph', 'move-points'); + clueCanvas.toolbarButtonIsNotSelected('graph', 'move-points'); + clueCanvas.toolbarButtonIsDisabled('graph', 'add-points'); + clueCanvas.toolbarButtonIsNotSelected('graph', 'add-points'); + + // Create manual layer + clueCanvas.clickToolbarButton('graph', 'add-points-by-hand'); + clueCanvas.toolbarButtonIsDisabled('graph', 'add-points-by-hand'); // only one manual set allowed + clueCanvas.toolbarButtonIsEnabled('graph', 'move-points'); + clueCanvas.toolbarButtonIsEnabled('graph', 'add-points'); + clueCanvas.toolbarButtonIsSelected('graph', 'add-points'); // automatically turns on "add" mode + 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'); + // Rename manual layer 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"); - }); + xyTile.getLayerName().should('have.length', 1).should('contain.text', 'Renamed'); - }); + // Add points + xyTile.getGraphDot().should('have.length', 0); + xyTile.getTile('.primary-workspace').should('have.length', 1); + xyTile.getGraphBackground().should('have.length', 1).click(150, 50); + xyTile.getGraphBackground().click(200, 100); + xyTile.getGraphDot().should('have.length', 2); + // Switch to 'select/move' mode + clueCanvas.clickToolbarButton('graph', 'move-points'); + clueCanvas.toolbarButtonIsSelected('graph', 'move-points'); + clueCanvas.toolbarButtonIsNotSelected('graph', 'add-points'); + xyTile.getGraphBackground().click(250, 100); // should not add a point + xyTile.getGraphDot().should('have.length', 2); + // 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); + + // Click toolbar button again to leave edit mode + clueCanvas.clickToolbarButton('graph', 'move-points'); + clueCanvas.toolbarButtonIsNotSelected('graph', 'move-points'); + clueCanvas.toolbarButtonIsNotSelected('graph', 'add-points'); + }); + }); }); diff --git a/cypress/support/elements/tile/TableToolTile.js b/cypress/support/elements/tile/TableToolTile.js index e293336f58..f415391897 100644 --- a/cypress/support/elements/tile/TableToolTile.js +++ b/cypress/support/elements/tile/TableToolTile.js @@ -64,14 +64,12 @@ class TableToolTile{ return cy.get('.rdg-row .rdg-cell .rdg-text-editor'); } typeInTableCellXY(row, col, text) { - this.getTableCellXY(row, col).dblclick().then(() => { - this.getTableCellEdit().type(`${text}{enter}`); - }); + this.getTableCellXY(row, col).dblclick(); + this.getTableCellEdit().type(`${text}{enter}`); } typeInTableCell(i, text) { - this.getTableCell().eq(i).dblclick().then(() => { - this.getTableCellEdit().type(`${text}{enter}`); - }); + this.getTableCell().eq(i).dblclick(); + this.getTableCellEdit().type(`${text}{enter}`); } getTableCellWithColIndex(colIndex, colValue){ return cy.get('.rdg-row').contains('.rdg-cell[aria-colindex="' + colIndex + '"]', colValue); diff --git a/cypress/support/elements/tile/XYPlotToolTile.js b/cypress/support/elements/tile/XYPlotToolTile.js index aa97dd1a87..f015a1e50c 100644 --- a/cypress/support/elements/tile/XYPlotToolTile.js +++ b/cypress/support/elements/tile/XYPlotToolTile.js @@ -6,6 +6,9 @@ class XYPlotToolTile { getTile(workspaceClass) { return cy.get(`${wsClass(workspaceClass)} .canvas-area .graph-tool-tile`); } + getGraphBackground(workspaceClass) { + return cy.get(`${wsClass(workspaceClass)} svg [data-testid=graph-background]`); + } getXYPlotTitle(workspaceClass) { return cy.get(`${wsClass(workspaceClass)} .canvas-area .editable-tile-title`); } diff --git a/docs/mst-detached-error.md b/docs/mst-detached-error.md index 77f6634297..40227591eb 100644 --- a/docs/mst-detached-error.md +++ b/docs/mst-detached-error.md @@ -12,7 +12,7 @@ Below is a strategy for tracking down the cause and a solution for one particula Hopefully from the message you can figure out the rough area of the code where the problem is happening. If you can pin point the UI action that causes the warning to be printed then you can use the following approach. -In your local code find `kEnableLivelinessChecking` located in `index.tsx` and set to be `true`. This will cause these console warnings to actually be thrown errors instead. In many cases these thrown errors will be caught and ignored so after you set this to true you probably won't see any messages in the console if you try to duplicate the problem again. +In your local code find `kEnableLivelinessChecking` located in `initialize-app.tsx` and set to be `true`. This will cause these console warnings to actually be thrown errors instead. In many cases these thrown errors will be caught and ignored so after you set this to true you probably won't see any messages in the console if you try to duplicate the problem again. Instead of looking in the console, what you should do is use the dev tools debugger to track it down. Setup the app to just before the error would normally be printed to the console. Then in the debug panel turn on "pause on exceptions" and check the box "pause on caught exceptions". Now trigger the error. The app should pause and you should be in the debugger. diff --git a/docs/unit-configuration.md b/docs/unit-configuration.md index f8c0107e22..b40648cc13 100644 --- a/docs/unit-configuration.md +++ b/docs/unit-configuration.md @@ -165,13 +165,16 @@ Not updated to common toolbar framework and does not support toolbar configurati - `emptyPlotIsNumeric`: boolean, default true - `scalePlotOnValueChange`: boolean, default true -Common toolbar framework; default toolbar buttons: +Uses the common toolbar framework. Default toolbar buttons: - `link-tile` (opens dialog to replace dataset with a new one) +- `add-points-by-hand` (creates a dataset owned by the graph) - `fit-all` (rescale axes to fit all points in view) - `toggle-lock` (lock axes so they won't automatically rescale) +- `move-points` (mode where points can be moved) +- `add-points` (mode where points can be added) -Also available: +Additional buttons available not in default set: - `link-tile-multiple` (opens dialog to add an additional dataset or link variables) diff --git a/src/clue/app-config.json b/src/clue/app-config.json index 170ff46412..167f4902a9 100644 --- a/src/clue/app-config.json +++ b/src/clue/app-config.json @@ -272,7 +272,10 @@ "add-points-by-hand", "|", "fit-all", - "toggle-lock" + "toggle-lock", + "|", + "move-points", + "add-points" ] }, "simulator": { diff --git a/src/components/utilities/editable-label-with-button.tsx b/src/components/utilities/editable-label-with-button.tsx index 5b107da8b8..b2508fe285 100644 --- a/src/components/utilities/editable-label-with-button.tsx +++ b/src/components/utilities/editable-label-with-button.tsx @@ -11,21 +11,14 @@ interface IProps { onSubmit: (value:string) => void; } +/** + * Generic component for an in-place editable label. + * + * Differs from stock in that there's an explicit edit button rather + * than invoking edit mode by clicking on the label text itself. + */ export const EditableLabelWithButton = observer(function EditableDataSetName({defaultValue, onSubmit}: IProps) { - function EditButton() { - const { isEditing, getEditButtonProps } = useEditableControls(); - if (!isEditing) { - return ( - - ); - } else { - return null; - } - } - return ( ); }); + +function EditButton() { + const { isEditing, getEditButtonProps } = useEditableControls(); + if (!isEditing) { + return ( + + ); + } else { + return null; + } +} diff --git a/src/models/data/data-set.ts b/src/models/data/data-set.ts index f504f6fbe8..d1dbebf7d5 100644 --- a/src/models/data/data-set.ts +++ b/src/models/data/data-set.ts @@ -221,6 +221,7 @@ export const DataSet = types.model("DataSet", { self.cases.push(newCase); beforeIndex = self.cases.length - 1; } + return newCase; } // `affectedAttributes` are not used in the function, but are present as a potential @@ -689,14 +690,16 @@ export const DataSet = types.model("DataSet", { }, addCanonicalCasesWithIDs(cases: ICase[], beforeID?: string | string[]) { + const newCases: ICase[] = []; cases.forEach((aCase, index) => { const beforeIndex = beforeIndexForInsert(index, beforeID); self.attributes.forEach((attr: IAttribute) => { const value = aCase[attr.id]; attr.addValue(value != null ? value : undefined, beforeIndex); }); - insertCaseIDAtIndex(aCase.__id__, beforeIndex); + newCases.push(insertCaseIDAtIndex(aCase.__id__, beforeIndex)); }); + return newCases; }, setCaseValues(cases: ICase[], affectedAttributes?: string[]) { @@ -904,7 +907,7 @@ export function addCanonicalCasesToDataSet(dataset: IDataSet, cases: ICaseCreati aCase.__id__ = newCaseId(); } }); - dataset.addCanonicalCasesWithIDs(newCases, beforeID); + return dataset.addCanonicalCasesWithIDs(newCases, beforeID); } export function getDataSetBounds(dataSet: IDataSet) { diff --git a/src/models/document/document-content-types.ts b/src/models/document/document-content-types.ts index a4fef9f9e8..f046fb7d1e 100644 --- a/src/models/document/document-content-types.ts +++ b/src/models/document/document-content-types.ts @@ -1,5 +1,4 @@ import { IArrowAnnotation } from "../annotations/arrow-annotation"; -import { SharedModelSnapshotType } from "../shared/shared-model"; import { IDragSharedModelItem } from "../shared/shared-model-manager"; import { IDragTileItem } from "../tiles/tile-model"; import { IDropRowInfo } from "./tile-row"; @@ -46,10 +45,3 @@ export interface IDragTilesData { annotations: IArrowAnnotation[]; } -export interface PartialTile { - id: string; -} -export interface PartialSharedModelEntry { - sharedModel: SharedModelSnapshotType; - tiles: PartialTile[]; -} diff --git a/src/models/document/document-content.ts b/src/models/document/document-content.ts index 3d8d9fbd8b..051af27fb3 100644 --- a/src/models/document/document-content.ts +++ b/src/models/document/document-content.ts @@ -1,7 +1,7 @@ import stringify from "json-stringify-pretty-compact"; import { applySnapshot, getSnapshot, Instance, SnapshotIn } from "mobx-state-tree"; import { cloneDeep, each } from "lodash"; -import { IDragTilesData, PartialSharedModelEntry, PartialTile, +import { IDragTilesData, IDocumentContentAddTileOptions } from "./document-content-types"; import { DocumentContentModelWithTileDragging } from "./drag-tiles"; import { IDropRowInfo, TileRowModelType, TileRowSnapshotOutType } from "./tile-row"; @@ -14,6 +14,7 @@ import { getTileContentInfo, IDocumentExportOptions } from "../tiles/tile-conten import { IDragTileItem, IDropTileItem, ITileModel, ITileModelSnapshotOut } from "../tiles/tile-model"; import { uniqueId } from "../../utilities/js-utils"; import { comma, StringBuilder } from "../../utilities/string-builder"; +import { SharedModelEntrySnapshotType } from "./shared-model-entry"; // Imports related to hard coding shared model duplication import { @@ -231,13 +232,13 @@ export const DocumentContentModel = DocumentContentModelWithTileDragging.named(" // Copies tiles and shared models into the specified row, giving them all new ids copyTiles( tiles: IDragTileItem[], - sharedModelEntries: PartialSharedModelEntry[], + sharedModelEntries: SharedModelEntrySnapshotType[], annotations: IArrowAnnotationSnapshot[], rowInfo: IDropRowInfo ) { // Update shared models with new names and ids const updatedSharedModelMap: Record = {}; - const newSharedModelEntries: PartialSharedModelEntry[] = []; + const newSharedModelEntries: SharedModelEntrySnapshotType[] = []; sharedModelEntries.forEach(sharedModelEntry => { // For now, only duplicate shared data sets if (isSharedDataSetSnapshot(sharedModelEntry.sharedModel)) { @@ -266,7 +267,7 @@ export const DocumentContentModel = DocumentContentModelWithTileDragging.named(" }); const findTileSharedModelEntries = (tileId: string) => { - return sharedModelEntries.filter(entry => entry.tiles?.map(t => t.id).includes(tileId)); + return sharedModelEntries.filter(entry => entry.tiles?.includes(tileId)); }; // Update tile content with new shared model ids @@ -302,7 +303,7 @@ export const DocumentContentModel = DocumentContentModelWithTileDragging.named(" // Update tile ids for shared models and add those references to document. // The shared datasets have already been added above. newSharedModelEntries.forEach(sharedModelEntry => { - const updatedTileIds: string[] = sharedModelEntry.tiles.map((oldTile: PartialTile) => tileIdMap[oldTile.id]) + const updatedTileIds: string[] = (sharedModelEntry.tiles||[]).map((oldTile) => tileIdMap[oldTile]) .filter((tileId: string | undefined) => tileId !== undefined); if (isSharedDataSetSnapshot(sharedModelEntry.sharedModel)) { const oldProvider = sharedModelEntry.sharedModel.providerId; @@ -332,7 +333,7 @@ export const DocumentContentModel = DocumentContentModelWithTileDragging.named(" }); // Update tile and object ids for annotations and add copies to document - const updateObject = (object?: IClueObjectSnapshot) => { + const updateAnnotationObject = (object?: IClueObjectSnapshot) => { if (object) { const tile = tiles.find(t => t.tileId === object?.tileId); if (tile) { @@ -344,11 +345,12 @@ export const DocumentContentModel = DocumentContentModelWithTileDragging.named(" } } }; + annotations.forEach(annotation => { if (isArrowAnnotationSnapshot(annotation)) { const newAnnotationSnapshot = cloneDeep(annotation); - updateObject(newAnnotationSnapshot.sourceObject); - updateObject(newAnnotationSnapshot.targetObject); + updateAnnotationObject(newAnnotationSnapshot.sourceObject); + updateAnnotationObject(newAnnotationSnapshot.targetObject); updateArrowAnnotationTileIds(newAnnotationSnapshot, tileIdMap); newAnnotationSnapshot.id = uniqueId(); self.addArrow(ArrowAnnotation.create(newAnnotationSnapshot)); @@ -361,7 +363,7 @@ export const DocumentContentModel = DocumentContentModelWithTileDragging.named(" const { tiles, sharedModels, annotations } = dragTiles; // Convert IDragSharedModelItems to partial SharedModelEnries - const sharedModelEntries: PartialSharedModelEntry[] = []; + const sharedModelEntries: SharedModelEntrySnapshotType[] = []; sharedModels.forEach(dragSharedModel => { try { const content = JSON.parse(dragSharedModel.content); @@ -370,7 +372,7 @@ export const DocumentContentModel = DocumentContentModelWithTileDragging.named(" if (sharedModel) { sharedModelEntries.push({ sharedModel, - tiles: dragSharedModel.tileIds.map(tileId => ({ id: tileId })) + tiles: dragSharedModel.tileIds }); } } catch (e) { @@ -389,9 +391,14 @@ export const DocumentContentModel = DocumentContentModelWithTileDragging.named(" const sharedModelEntries = Object.values(self.getSharedModelsUsedByTiles(tileIds)); const annotations = Object.values(self.getAnnotationsUsedByTiles(tileIds, true)); + // Make sure we are only passing snapshots to copyTiles, since they need to be cloned & modified. + const snapshots = sharedModelEntries.map(sme => { + return getSnapshot(sme); + }); + self.copyTiles( tiles, - sharedModelEntries, + snapshots, annotations, { rowInsertIndex: rowIndex } ); diff --git a/src/models/history/tree-monitor.ts b/src/models/history/tree-monitor.ts index 9500fed235..939fcd6bad 100644 --- a/src/models/history/tree-monitor.ts +++ b/src/models/history/tree-monitor.ts @@ -1,6 +1,6 @@ import { addDisposer, addMiddleware, getSnapshot, IJsonPatch, Instance, IPatchRecorder, - isActionContextThisOrChildOf, isAlive, recordPatches + isActionContextThisOrChildOf, isAlive, recordPatches, tryResolve } from "mobx-state-tree"; import { nanoid } from "nanoid"; import { TreeManagerAPI } from "./tree-manager-api"; @@ -222,6 +222,14 @@ export class TreeMonitor { // inform the tiles of all changes at the same time. for (const [sharedModelPath, numModifications] of Object.entries(sharedModelModifications)) { if (numModifications > 0) { + // If a shared model has been deleted, we can't run these callbacks without errors, + // so we bail out now. May need improvement if tiles need to be notified about + // deleted shared models. + try { + tryResolve(this.tree, `${sharedModelPath}/sharedModel`); + } catch { + continue; + } // Run the callbacks tracking changes to the shared model. // We need to wait for these to complete because the manager // needs to know when this history entry is complete. If it diff --git a/src/models/tiles/table/table-utils.ts b/src/models/tiles/table/table-utils.ts index bea9af3c17..0d5bf4b807 100644 --- a/src/models/tiles/table/table-utils.ts +++ b/src/models/tiles/table/table-utils.ts @@ -1,7 +1,7 @@ import { TableContentSnapshotType } from "./table-content"; import { IClueObjectSnapshot } from "../../annotations/clue-object"; -import { PartialSharedModelEntry } from "../../document/document-content-types"; import { UpdatedSharedDataSetIds } from "../../shared/shared-data-set"; +import { SharedModelEntrySnapshotType } from "../../document/shared-model-entry"; const cellIdRegEx = /^cell:{(.+)}:{(.+)}$/; @@ -21,7 +21,7 @@ export function decipherCellId(cellId: string) { export function updateTableContentWithNewSharedModelIds( content: TableContentSnapshotType, - sharedDataSetEntries: PartialSharedModelEntry[], + sharedDataSetEntries: SharedModelEntrySnapshotType[], updatedSharedModelMap: Record ) { // Column widths uses attribute ids, so we have to update them when updating shared dataset ids @@ -44,7 +44,7 @@ export function updateTableContentWithNewSharedModelIds( export function updateTableObjectWithNewSharedModelIds( object: IClueObjectSnapshot, - sharedDataSetEntries: PartialSharedModelEntry[], + sharedDataSetEntries: SharedModelEntrySnapshotType[], updatedSharedModelMap: Record ) { if (object.objectType === "cell") { diff --git a/src/models/tiles/tile-content-info.ts b/src/models/tiles/tile-content-info.ts index 08f4b786cf..35f872286a 100644 --- a/src/models/tiles/tile-content-info.ts +++ b/src/models/tiles/tile-content-info.ts @@ -2,8 +2,8 @@ import { ITileMetadataModel, TileMetadataModel } from "./tile-metadata"; import { TileContentModel, ITileContentModel } from "./tile-content"; import { IClueObjectSnapshot } from "../annotations/clue-object"; import { AppConfigModelType } from "../stores/app-config-model"; -import { PartialSharedModelEntry } from "../document/document-content-types"; import { UpdatedSharedDataSetIds } from "../shared/shared-data-set"; +import { SharedModelEntrySnapshotType } from "../document/shared-model-entry"; export interface IDefaultContentOptions { // title is only currently used by the Geometry and Table tiles @@ -22,13 +22,13 @@ type TileContentSnapshotPostProcessor = type TileContentNewSharedModelIdUpdater = ( content: any, - sharedModelEntries: PartialSharedModelEntry[], + sharedModelEntries: SharedModelEntrySnapshotType[], updatedSharedModelMap: Record ) => any; type ClueObjectNewSharedModelIdUpdater = ( clueObject: IClueObjectSnapshot, - sharedModelEntries: PartialSharedModelEntry[], + sharedModelEntries: SharedModelEntrySnapshotType[], updatedSharedModelMap: Record ) => any; diff --git a/src/plugins/data-card/data-card-content.ts b/src/plugins/data-card/data-card-content.ts index 0958091c4b..58af9ad881 100644 --- a/src/plugins/data-card/data-card-content.ts +++ b/src/plugins/data-card/data-card-content.ts @@ -17,8 +17,8 @@ import { import { updateSharedDataSetColors } from "../../models/shared/shared-data-set-colors"; import { SharedModelType } from "../../models/shared/shared-model"; import { uniqueId, uniqueTitle } from "../../utilities/js-utils"; -import { PartialSharedModelEntry } from "../../models/document/document-content-types"; import { getTileModel } from "../../models/tiles/tile-model"; +import { SharedModelEntrySnapshotType } from "../../models/document/shared-model-entry"; export function defaultDataSet(name?: string) { const dataSet = DataSet.create({name}); @@ -278,7 +278,7 @@ export type DataCardContentSnapshotType = SnapshotIn ) { const updatedContent = cloneDeep(content); diff --git a/src/plugins/graph/assets/add-points-icon.svg b/src/plugins/graph/assets/add-points-icon.svg new file mode 100644 index 0000000000..94b419c5f7 --- /dev/null +++ b/src/plugins/graph/assets/add-points-icon.svg @@ -0,0 +1,3 @@ + + + diff --git a/src/plugins/graph/assets/select-tool-icon.svg b/src/plugins/graph/assets/select-tool-icon.svg new file mode 100644 index 0000000000..5de5dc6003 --- /dev/null +++ b/src/plugins/graph/assets/select-tool-icon.svg @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/plugins/graph/components/background.tsx b/src/plugins/graph/components/background.tsx index aa5c2cbec7..e7e0259d8f 100644 --- a/src/plugins/graph/components/background.tsx +++ b/src/plugins/graph/components/background.tsx @@ -1,56 +1,121 @@ import {autorun} from "mobx"; -import React, {forwardRef, MutableRefObject, useCallback, useEffect, useMemo, useRef} from "react"; +import React, {forwardRef, MutableRefObject, useCallback, useEffect, useMemo, useRef, useState} from "react"; import {drag, select, color, range} from "d3"; import RTreeLib from 'rtree'; type RTree = ReturnType; -import {CaseData} from "../d3-types"; -import {InternalizedData, rTreeRect} from "../graph-types"; +import {CaseData, graphDotSelector} from "../d3-types"; +import { Point, rTreeRect} from "../graph-types"; import {useGraphLayoutContext} from "../models/graph-layout"; import {rectangleSubtract, rectNormalize} from "../utilities/graph-utils"; import {MarqueeState} from "../models/marquee-state"; import {IGraphModel} from "../models/graph-model"; import {useInstanceIdContext} from "../imports/hooks/use-instance-id-context"; import { useGraphModelContext } from "../hooks/use-graph-model-context"; +import { ICell } from "../../../models/data/data-types"; interface IProps { marqueeState: MarqueeState } +/** + * Gets the transformation matrix that transforms from the user coordinate + * system on the source element to the user coordinate system on the + * target element. + * Replaces deprecated SVG method of the same name. + * @param source + * @param elem + * @returns + */ +function getTransformToElement(source: SVGGraphicsElement, elem: SVGGraphicsElement) { + const elemCTM = elem.getScreenCTM(); + const sourceCTM = source.getScreenCTM(); + if (elemCTM && sourceCTM) { + return elemCTM.inverse().multiply(sourceCTM); + } else { + console.warn("Can't get CTM on element"); + } +} + +// Get bounding box INCLUDING any transform="..." on the element itself. +// Adapted from https://stackoverflow.com/a/64909822 +function boundingBoxRelativeToElement(fromSpace: SVGGraphicsElement, toSpace: SVGGraphicsElement) { + const bbox = fromSpace.getBBox(); + const m = getTransformToElement(fromSpace, toSpace); + let bbC = new DOMPoint(); bbC.x = bbox.x; bbC.y = bbox.y; + let bbD = new DOMPoint(); bbD.x = bbox.x + bbox.width; bbD.y = bbox.y + bbox.height; + bbC = bbC.matrixTransform(m); + bbD = bbD.matrixTransform(m); + return { x: bbC.x, y: bbC.y, width: Math.abs(bbD.x - bbC.x), height: Math.abs(bbD.y - bbC.y)}; +} + const prepareTree = (areaSelector: string, circleSelector: string): RTree => { const selectionTree = RTreeLib(10); - select(areaSelector).selectAll(circleSelector) - .each((datum: InternalizedData, index, groups) => { - const element: any = groups[index], + select(areaSelector).selectAll(circleSelector) + .each((datum: CaseData, index, groups) => { + const + element: any = groups[index], + bbox = boundingBoxRelativeToElement(element, element.parentElement), rect = { - x: Number(element.cx.baseVal.value), - y: Number(element.cy.baseVal.value), - w: 1, h: 1 + x: bbox.x, + y: bbox.y, + w: bbox.width, + h: bbox.height }; - selectionTree.insert(rect, (element.__data__ as CaseData).caseID); + selectionTree.insert(rect, (element.__data__ as CaseData)); }); return selectionTree; }, + /** + * Searches the new area, and returns cases found in it. + */ getCasesForDelta = (tree: any, newRect: rTreeRect, prevRect: rTreeRect) => { const diffRects = rectangleSubtract(newRect, prevRect); - let caseIDs: string[] = []; + let allCases: CaseData[] = []; diffRects.forEach(aRect => { - const newlyFoundIDs = tree.search(aRect); - caseIDs = caseIDs.concat(newlyFoundIDs); + const newlyFoundCases = tree.search(aRect); + allCases = allCases.concat(newlyFoundCases); }); - return caseIDs; + return allCases; + }, + + /** + * Take list of CaseData objects, return map from data configuration id to cell info. + */ + sortByDataConfiguration = (graphModel: IGraphModel, cases: CaseData[]) => { + const caseDatas = new Map(); + for (const c of cases) { + if (!caseDatas.has(c.dataConfigID)) { + caseDatas.set(c.dataConfigID, []); + } + const dataConfiguration = graphModel.layerForDataConfigurationId(c.dataConfigID)?.config; + if (dataConfiguration) { + const attributeId = dataConfiguration.attributeIdforPlotNumber(c.plotNum); + caseDatas.get(c.dataConfigID)?.push({caseId: c.caseID, attributeId}); + } + } + return caseDatas; }, updateSelections = (graphModel: IGraphModel, tree: any, newRect: rTreeRect, prevRect: rTreeRect) => { const newSelection = getCasesForDelta(tree, newRect, prevRect); const newDeselection = getCasesForDelta(tree, prevRect, newRect); if (newSelection.length) { - graphModel.layers[0].config.dataset?.selectCases(newSelection, true); // FIXME multi dataset + for (const [dataConfId, cells] of sortByDataConfiguration(graphModel, newSelection).entries()) { + const dataConfiguration = graphModel.layerForDataConfigurationId(dataConfId); + if (dataConfiguration) { + dataConfiguration.config.dataset?.selectCells(cells, true); + } + } } if (newDeselection.length) { - graphModel.layers[0].config.dataset?.selectCases(newDeselection, false); + for (const [dataConfId, cells] of sortByDataConfiguration(graphModel, newDeselection).entries()) { + const dataConfiguration = graphModel.layerForDataConfigurationId(dataConfId); + if (dataConfiguration) { + dataConfiguration.config.dataset?.selectCells(cells, false); + } + } } - }; export const Background = forwardRef((props, ref) => { @@ -64,12 +129,36 @@ export const Background = forwardRef((props, ref) => { width = useRef(0), height = useRef(0), selectionTree = useRef(null), - previousMarqueeRect = useRef(); + previousMarqueeRect = useRef(), + [potentialPoint, setPotentialPoint] = useState(undefined); + + const pointCoordinates = useCallback((offsetX: number, offsetY: number) => { + const plotBounds = layout.computedBounds.plot; + const relX = offsetX - plotBounds.left; + const relY = offsetY - plotBounds.top; + const { data: x } = layout.getAxisMultiScale("bottom").getDataCoordinate(relX); + const { data: y } = layout.getAxisMultiScale("left").getDataCoordinate(relY); + return { x, y }; + }, [layout]); - const onDragStart = useCallback((event: { x: number; y: number; sourceEvent: { shiftKey: boolean } }) => { + const onClick = useCallback((event: { offsetX: number, offsetY: number, shiftKey: boolean }) => { + if (graphModel.editingMode==="add") { + const {x, y} = pointCoordinates(event.offsetX, event.offsetY); + graphModel.editingLayer?.addPoint(x, y); + } else { + // If not in add mode or shifted, clicking on background deselects everything + if (!event.shiftKey) { + graphModel.clearAllSelectedCases(); + } + } + }, [graphModel, pointCoordinates]); + + // Define the dragging behaviors for "edit" mode and for "add" mode, then assemble into one "drag" object. + const + dragStartEditMode = useCallback((event: { x: number; y: number; sourceEvent: { shiftKey: boolean } }) => { const {computedBounds} = layout, plotBounds = computedBounds.plot; - selectionTree.current = prepareTree(`.${instanceId}`, 'circle'); + selectionTree.current = prepareTree(`.${instanceId}`, graphDotSelector); startX.current = event.x - plotBounds.left; startY.current = event.y - plotBounds.top; width.current = 0; @@ -80,7 +169,7 @@ export const Background = forwardRef((props, ref) => { marqueeState.setMarqueeRect({x: startX.current, y: startY.current, width: 0, height: 0}); }, [graphModel, instanceId, layout, marqueeState]), - onDrag = useCallback((event: { dx: number; dy: number }) => { + dragMoveEditMode = useCallback((event: { dx: number; dy: number }) => { if (event.dx !== 0 || event.dy !== 0) { previousMarqueeRect.current = rectNormalize( {x: startX.current, y: startY.current, w: width.current, h: height.current}); @@ -101,14 +190,55 @@ export const Background = forwardRef((props, ref) => { } }, [graphModel, marqueeState]), - onDragEnd = useCallback(() => { + dragEndEditMode = useCallback((event: { x: number; y: number; }) => { marqueeState.setMarqueeRect({x: 0, y: 0, width: 0, height: 0}); selectionTree.current = null; }, [marqueeState]), - dragBehavior = useMemo(() => drag() - .on("start", onDragStart) - .on("drag", onDrag) - .on("end", onDragEnd), [onDrag, onDragEnd, onDragStart]); + + dragStartAddMode = useCallback((event: { x: number; y: number; sourceEvent: { shiftKey: boolean } }) => { + setPotentialPoint(event); + }, []), + + dragMoveAddMode = useCallback((event: { x: number; y: number; dx: number; dy: number }) => { + setPotentialPoint(event); + }, []), + + dragEndAddMode = useCallback((event: { x: number; y: number; }) => { + const point = pointCoordinates(event.x, event.y); + graphModel.editingLayer?.addPoint(point.x, point.y); + setPotentialPoint(undefined); + }, [graphModel.editingLayer, pointCoordinates]), + + dragStart = useCallback((event: { x: number; y: number; sourceEvent: { shiftKey: boolean } }) => { + if (graphModel.editingMode === "add") { + dragStartAddMode(event); + } else { + dragStartEditMode(event); + } + }, [dragStartAddMode, graphModel.editingMode, dragStartEditMode]), + + dragMove = useCallback((event: { x: number; y: number; dx: number; dy: number }) => { + if (graphModel.editingMode === "add") { + dragMoveAddMode(event); + } else { + dragMoveEditMode(event); + } + }, [dragMoveAddMode, graphModel.editingMode, dragMoveEditMode]), + + dragEnd = useCallback((event: { x: number; y: number; }) => { + if (graphModel.editingMode === "add") { + dragEndAddMode(event); + } else { + dragEndEditMode(event); + } + }, [dragEndAddMode, graphModel.editingMode, dragEndEditMode]); + + const dragBehavior = useMemo(() => { + return drag() + .on("start", dragStart) + .on("drag", dragMove) + .on("end", dragEnd); + }, [dragMove, dragEnd, dragStart]); useEffect(() => { return autorun(() => { @@ -125,12 +255,7 @@ export const Background = forwardRef((props, ref) => { col = (index: number) => index % numCols, groupElement = bgRef.current; select(groupElement) - // clicking on the background deselects all cases - .on('click', (event) => { - if (!event.shiftKey) { - graphModel.clearAllSelectedCases(); - } - }) + .on('click', onClick) .selectAll('rect') .data(range(numRows * numCols)) .join('rect') @@ -144,10 +269,15 @@ export const Background = forwardRef((props, ref) => { .style('fill-opacity', isTransparent ? 0 : 1) .call(dragBehavior); }); - }, [bgRef, dragBehavior, graphModel, layout]); + }, [bgRef, dragBehavior, graphModel, layout, onClick]); return ( - + + + {potentialPoint && + } + ); }); Background.displayName = "Background"; diff --git a/src/plugins/graph/components/graph-axis.tsx b/src/plugins/graph/components/graph-axis.tsx index 3e0875306b..6d83921121 100644 --- a/src/plugins/graph/components/graph-axis.tsx +++ b/src/plugins/graph/components/graph-axis.tsx @@ -18,24 +18,24 @@ 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 { computeNiceNumericBounds } from "../utilities/graph-utils"; -import { isNumericAxisModel } from "../imports/components/axis/models/axis-model"; 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 - autoAdjust?: React.MutableRefObject - onDropAttribute?: (place: GraphPlace, dataSet: IDataSet, attrId: string) => void - onRemoveAttribute?: (place: GraphPlace, attrId: string) => void - onTreatAttributeAs?: (place: GraphPlace, attrId: string, treatAs: AttributeType) => void + place: AxisPlace; + enableAnimation: MutableRefObject; + autoAdjust?: React.MutableRefObject; + 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, onDropAttribute, onRemoveAttribute, onTreatAttributeAs + place, enableAnimation, autoAdjust, controller, onDropAttribute, onRemoveAttribute, onTreatAttributeAs }: IProps) { - const dataConfig = useDataConfigurationContext(), + const dataConfig = useDataConfigurationContext(), // FIXME mult-dataset. isDropAllowed = dataConfig?.graphPlaceCanAcceptAttributeIDDrop ?? (() => true), graphModel = useGraphModelContext(), instanceId = useInstanceIdContext(), @@ -89,37 +89,21 @@ export const GraphAxis = observer(function GraphAxis({ useEffect(() => { if (autoAdjust?.current) { - // TODO multi dataset - this should consider all layers - dataConfig?.onAction(action => { - if ( - isAlive(graphModel) && + // 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 && - (isAddCasesAction(action) || isSetCaseValuesAction(action)) - ) - { - const _axisModel = graphModel?.getAxis(place); - const xValues = dataConfig.numericValuesForAttrRole("x"); - const yValues = dataConfig.numericValuesForAttrRole("y"); - - if (_axisModel && isNumericAxisModel(_axisModel)) { - if (xValues.length > 0 && place === "bottom") { - const minX = Math.min(...xValues); - const maxX = Math.max(...xValues); - const newXBounds = computeNiceNumericBounds(minX, maxX); - _axisModel.setDomain(newXBounds.min, newXBounds.max); - } - - if (yValues.length > 0 && place === "left") { - const minY = Math.min(...yValues); - const maxY = Math.max(...yValues); - const newYBounds = computeNiceNumericBounds(minY, maxY); - _axisModel.setDomain(newYBounds.min, newYBounds.max); - } + !graphModel.interactionInProgress && + (isAddCasesAction(action) || isSetCaseValuesAction(action))) { + controller.autoscaleAllAxes(); } - } - }); + }); + } } - }, [autoAdjust, dataConfig, graphModel, layout, place]); + // 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 () => { diff --git a/src/plugins/graph/components/graph-toolbar-registration.tsx b/src/plugins/graph/components/graph-toolbar-registration.tsx index bac3a7901a..881a4f2da9 100644 --- a/src/plugins/graph/components/graph-toolbar-registration.tsx +++ b/src/plugins/graph/components/graph-toolbar-registration.tsx @@ -14,6 +14,8 @@ 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"; +import SelectToolIcon from "../assets/select-tool-icon.svg"; +import AddPointsIcon from "../assets/add-points-icon.svg"; function LinkTileButton(name: string, title: string, allowMultiple: boolean) { @@ -91,7 +93,6 @@ 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. @@ -102,6 +103,7 @@ const AddPointsByHandButton = observer(function AddPointsByHandButton({name}: IT function handleClick() { graph.createEditableLayer(); + graph.setEditingMode("add"); } return ( @@ -117,6 +119,56 @@ const AddPointsByHandButton = observer(function AddPointsByHandButton({name}: IT }); +const SelectPointsButton = observer(function({name}: IToolbarButtonComponentProps) { + const graph = useGraphModelContext(); + const currentMode = graph.editingMode; + + const linked = graph.isLinkedToDataSet; + + function handleClick() { + graph.setEditingMode(currentMode==="edit" ? "none" : "edit"); + } + + return ( + + + + ); +}); + +const AddPointsButton = observer(function({name}: IToolbarButtonComponentProps) { + const graph = useGraphModelContext(); + const currentMode = graph.editingMode; + + const editableLayers = graph.getEditableLayers(); + const hasEditableLayers = editableLayers.length > 0; + + const iconStyle = { fill: graph.getEditablePointsColor() }; + + function handleClick() { + graph.setEditingMode(currentMode==="add" ? "none" : "add"); + } + + return ( + + + + ); + +}); + registerTileToolbarButtons("graph", [ { @@ -138,5 +190,14 @@ registerTileToolbarButtons("graph", { name: 'add-points-by-hand', component: AddPointsByHandButton + }, + { + name: 'add-points', + component: AddPointsButton + }, + { + name: 'move-points', + component: SelectPointsButton } + ]); diff --git a/src/plugins/graph/components/graph-wrapper-component.tsx b/src/plugins/graph/components/graph-wrapper-component.tsx index dc31ba8785..5c0b80842c 100644 --- a/src/plugins/graph/components/graph-wrapper-component.tsx +++ b/src/plugins/graph/components/graph-wrapper-component.tsx @@ -1,6 +1,7 @@ import React, { useCallback, useEffect } from "react"; import classNames from "classnames"; import { observer } from "mobx-react-lite"; +import { ScaleLinear } from "d3"; import { kSmallAnnotationNodeRadius } from "../../../components/annotations/annotation-utilities"; import { BasicEditableTileTitle } from "../../../components/tiles/basic-editable-tile-title"; @@ -18,7 +19,6 @@ import { decipherDotId } from "../utilities/graph-utils"; import { GraphComponent } from "./graph-component"; import { isNumericAxisModel } from "../imports/components/axis/models/axis-model"; import { Point } from "../graph-types"; -import { ScaleLinear } from "d3"; import "./graph-toolbar-registration"; diff --git a/src/plugins/graph/components/graph.tsx b/src/plugins/graph/components/graph.tsx index 593b5d6e95..560180e5fc 100644 --- a/src/plugins/graph/components/graph.tsx +++ b/src/plugins/graph/components/graph.tsx @@ -183,6 +183,7 @@ export const Graph = observer( return axes.map((place: AxisPlace) => { return { + onRequestRowHeight?.(instanceId, kGraphDefaultHeight + totalHeight); + }, 1); }, [instanceId, layout, onRequestRowHeight, totalHeight]); return ( diff --git a/src/plugins/graph/components/scatterdots.tsx b/src/plugins/graph/components/scatterdots.tsx index 475bf82a59..2c7cd4ca5f 100644 --- a/src/plugins/graph/components/scatterdots.tsx +++ b/src/plugins/graph/components/scatterdots.tsx @@ -1,7 +1,7 @@ import {ScaleBand, ScaleLinear, select} from "d3"; import React, {useCallback, useRef, useState} from "react"; import {ScaleNumericBaseType} from "../imports/components/axis/axis-types"; -import {CaseData} from "../d3-types"; +import {CaseData, inGraphDot} from "../d3-types"; import {PlotProps} from "../graph-types"; import {useDragHandlers, usePlotResponders} from "../hooks/use-plot"; import {useDataConfigurationContext} from "../hooks/use-data-configuration-context"; @@ -12,8 +12,7 @@ import { // getScreenCoord, handleClickOnDot, setPointCoordinates, - setPointSelection, - startAnimation + setPointSelection } from "../utilities/graph-utils"; import { useGraphLayerContext } from "../hooks/use-graph-layer-context"; @@ -32,9 +31,7 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { yScaleRef = useRef(), [dragID, setDragID] = useState(''), currPos = useRef({x: 0, y: 0}), - didDrag = useRef(false), target = useRef(), - selectedDataObjects = useRef>({}), plotNumRef = useRef(0); secondaryAttrIDsRef.current = dataConfiguration?.yAttributeIDs || []; @@ -44,70 +41,73 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { yScaleRef.current = layout.getAxisScale("left") as ScaleNumericBaseType; const onDragStart = useCallback((event: MouseEvent) => { - target.current = select(event.target as SVGSVGElement); + if (graphModel.editingMode !== "none") { + const targetDot = event.target && inGraphDot(event.target as SVGSVGElement); + if (!targetDot) return; + target.current = select(targetDot as SVGSVGElement); const aCaseData: CaseData = target.current.node().__data__; - if (!aCaseData) return; + if (!aCaseData || !dataConfiguration || aCaseData.dataConfigID !== dataConfiguration.id) return; + event.stopPropagation(); + graphModel.setInteractionInProgress(true); dataset?.beginCaching(); - secondaryAttrIDsRef.current = dataConfiguration?.yAttributeIDs || []; + secondaryAttrIDsRef.current = dataConfiguration.yAttributeIDs || []; enableAnimation.current = false; // We don't want to animate points until end of drag - didDrag.current = false; const tItsID = aCaseData.caseID; plotNumRef.current = target.current.datum()?.plotNum ?? 0; - if (target.current.node()?.nodeName === 'circle') { - target.current - .property('isDragging', true) - .transition() - .attr('r', dragPointRadiusRef.current); - setDragID(tItsID); - currPos.current = {x: event.clientX, y: event.clientY}; - - handleClickOnDot(event, aCaseData, dataConfiguration); - // Record the current values, so we can change them during the drag and restore them when done - const { caseSelection } = dataConfiguration || {}, - xAttrID = dataConfiguration?.attributeID('x') ?? ''; - caseSelection?.forEach(anID => { - selectedDataObjects.current[anID] = { - x: dataset?.getNumeric(anID, xAttrID) ?? 0, - y: dataset?.getNumeric(anID, secondaryAttrIDsRef.current[plotNumRef.current]) ?? 0 - }; - }); - } - }, [dataConfiguration, dataset, enableAnimation]), - - onDrag = useCallback((event: MouseEvent) => { - const xAxisScale = layout.getAxisScale('bottom') as ScaleLinear, - xAttrID = dataConfiguration?.attributeID('x') ?? ''; + // 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, + 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 (dx !== 0 || dy !== 0) { - didDrag.current = true; + 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[] = [], - { caseSelection } = dataConfiguration || {}; - caseSelection?.forEach(anID => { - const currX = Number(dataset?.getNumeric(anID, xAttrID)), - currY = Number(dataset?.getNumeric(anID, secondaryAttrIDsRef.current[plotNumRef.current])); + 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__: anID, + __id__: cell.caseId, [xAttrID]: currX + deltaX, - [secondaryAttrIDsRef.current[plotNumRef.current]]: currY + deltaY + [cell.attributeId]: currY + deltaY }); } }); caseValues.length && - dataset?.setCaseValues(caseValues, - [xAttrID, secondaryAttrIDsRef.current[plotNumRef.current]]); + dataset?.setCanonicalCaseValues(caseValues); } } }, [layout, dataConfiguration, dataset, dragID]), - onDragEnd = useCallback(() => { + onDrag = useCallback((event: MouseEvent) => { + updatePositions(event, false); + }, [updatePositions]), + + onDragEnd = useCallback((event: MouseEvent) => { if (dragID !== '') { + 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) @@ -115,25 +115,8 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { .attr('r', selectedPointRadiusRef.current); setDragID(() => ''); target.current = null; - - if (didDrag.current) { - const caseValues: ICase[] = [], - { caseSelection } = dataConfiguration || {}, - xAttrID = dataConfiguration?.attributeID('x') ?? ''; - caseSelection?.forEach(anID => { - caseValues.push({ - __id__: anID, - [xAttrID]: selectedDataObjects.current[anID].x, - [secondaryAttrIDsRef.current[plotNumRef.current]]: selectedDataObjects.current[anID].y - }); - }); - startAnimation(enableAnimation); // So points will animate back to original positions - caseValues.length && dataset?.setCaseValues(caseValues, - [xAttrID, secondaryAttrIDsRef.current[plotNumRef.current]]); - didDrag.current = false; - } } - }, [dataConfiguration, dataset, dragID, enableAnimation,]); + }, [dragID, graphModel, updatePositions]); useDragHandlers(window, {start: onDragStart, drag: onDrag, end: onDragEnd}); diff --git a/src/plugins/graph/d3-types.ts b/src/plugins/graph/d3-types.ts index c2a873bdfb..4f6b0c931d 100644 --- a/src/plugins/graph/d3-types.ts +++ b/src/plugins/graph/d3-types.ts @@ -14,10 +14,12 @@ export type DotsElt = SVGSVGElement | null; // unknown: type of data attached to parent element (none in this case) export type DotSelection = Selection; +export const graphDotSelector = "g.graph-dot"; + // selects all all g elements, which contain inner and outer circles export function selectGraphDots(svg: DotsElt): DotSelection | null { return svg - ? select(svg).selectAll("g.graph-dot") + ? select(svg).selectAll(graphDotSelector) : null; } @@ -38,3 +40,29 @@ export function selectInnerCircles(svg: DotsElt): DotSelection | null { ? select(svg).selectAll("g.graph-dot .inner-circle") : null; } + +/** + * Returns true if node is a graph dot. + * @param node + * @returns boolean + */ +export function isGraphDot(node: Node) { + return (node.nodeName === "g" + && node instanceof SVGElement + && node.classList.contains("graph-dot")); +} + +/** + * Returns true if node is a graph dot, or part of a graph dot. + * @param node + * @returns the graph dot element, or undefined + */ +export function inGraphDot(node: Node): SVGElement | undefined { + if (isGraphDot(node)) return node as SVGElement; + const parent = node.parentNode; + if (parent && parent instanceof SVGElement) { + return inGraphDot(parent); + } else { + return undefined; + } +} diff --git a/src/plugins/graph/graph-types.ts b/src/plugins/graph/graph-types.ts index 9d9b055e5e..44054b819e 100644 --- a/src/plugins/graph/graph-types.ts +++ b/src/plugins/graph/graph-types.ts @@ -108,3 +108,6 @@ export const kGraphAdornmentsClassSelector = `.${kGraphAdornmentsClass}`; export const kDefaultNumericAxisBounds = [0, 10] as const; export const kGraphPortalClass = ".canvas-area"; + +export const GraphEditModes = ["none", "edit", "add"]; +export type GraphEditMode = typeof GraphEditModes[number]; diff --git a/src/plugins/graph/hooks/use-graph-model.ts b/src/plugins/graph/hooks/use-graph-model.ts index 9dd2e9126f..29979f4f14 100644 --- a/src/plugins/graph/hooks/use-graph-model.ts +++ b/src/plugins/graph/hooks/use-graph-model.ts @@ -1,7 +1,6 @@ import {MutableRefObject, useCallback, useEffect} from "react"; -import {isAddCasesAction, isRemoveCasesAction} from "../../../models/data/data-set-actions"; import {IGraphModel, isGraphVisualPropsAction} from "../models/graph-model"; -import {matchAllCirclesToData, matchCirclesToData, setNiceDomain, startAnimation} from "../utilities/graph-utils"; +import {matchAllCirclesToData, setNiceDomain, startAnimation} from "../utilities/graph-utils"; import {onAnyAction} from "../../../utilities/mst-utils"; interface IProps { @@ -13,16 +12,16 @@ 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 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({ @@ -31,17 +30,20 @@ export function useGraphModel(props: IProps) { }, [graphModel, enableAnimation, instanceId]); // respond to added/removed cases - useEffect(function installAddRemoveCaseHandler() { - const disposers: (()=>void)[] = []; - for (const layer of graphModel.layers) { - disposers.push(layer.config.onAction(action => { - if (isAddCasesAction(action) || isRemoveCasesAction(action)) { - callMatchCirclesToData(layer); - } - })); - } - return () => { disposers.forEach((d) => { d(); }); }; - }, [graphModel.layers, callMatchCirclesToData]); + // 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() { diff --git a/src/plugins/graph/hooks/use-plot.ts b/src/plugins/graph/hooks/use-plot.ts index fa9369bca9..b02dc5a745 100644 --- a/src/plugins/graph/hooks/use-plot.ts +++ b/src/plugins/graph/hooks/use-plot.ts @@ -1,6 +1,7 @@ import React, {useCallback, useEffect, useRef} from "react"; import {autorun, reaction} from "mobx"; -import { isSetCaseValuesAction } from "../../../models/data/data-set-actions"; +import { isAddCasesAction, isRemoveAttributeAction, isRemoveCasesAction, isSetCaseValuesAction } + from "../../../models/data/data-set-actions"; import {IDotsRef, GraphAttrRoles} from "../graph-types"; import {INumericAxisModel} from "../imports/components/axis/models/axis-model"; import {useGraphLayoutContext} from "../models/graph-layout"; @@ -10,6 +11,8 @@ import {useCurrent} from "../../../hooks/use-current"; import {useInstanceIdContext} from "../imports/hooks/use-instance-id-context"; import {onAnyAction} from "../../../utilities/mst-utils"; import { IGraphLayerModel } from "../models/graph-layer-model"; +import { mstReaction } from "../../../utilities/mst-reaction"; +import { useReadOnlyContext } from "../../../components/document/read-only-context"; interface IDragHandlers { start: (event: MouseEvent) => void @@ -18,8 +21,9 @@ interface IDragHandlers { } export const useDragHandlers = (target: any, {start, drag, end}: IDragHandlers) => { + const readOnly = useReadOnlyContext(); useEffect(() => { - if (target) { + if (target && !readOnly) { target.addEventListener('mousedown', start); target.addEventListener('mousemove', drag); target.addEventListener('mouseup', end); @@ -30,7 +34,7 @@ export const useDragHandlers = (target: any, {start, drag, end}: IDragHandlers) target.removeEventListener('mouseup', end); }; } - }, [target, start, drag, end]); + }, [target, start, drag, end, readOnly]); }; export interface IPlotResponderProps { @@ -129,12 +133,14 @@ export const usePlotResponders = (props: IPlotResponderProps) => { // respond to attribute assignment changes useEffect(() => { - const disposer = reaction( + const disposer = mstReaction( () => GraphAttrRoles.map((aRole) => dataConfiguration?.attributeID(aRole)), () => { startAnimation(enableAnimation); callRefreshPointPositions(false); - } + }, + { name: "usePlot.attribute assignment reaction" }, + dataConfiguration ); return () => disposer(); }, [callRefreshPointPositions, dataConfiguration, enableAnimation]); @@ -191,7 +197,11 @@ export const usePlotResponders = (props: IPlotResponderProps) => { // respond to added or removed cases and change in attribute type useEffect(function handleAddRemoveCases() { const disposer = dataConfiguration?.onAction(action => { - if (['addCases', 'removeCases', 'setAttributeType'].includes(action.name)) { + if (isAddCasesAction(action) + || isRemoveCasesAction(action) + || isRemoveAttributeAction(action) + || ['addCases', 'removeCases', 'setAttributeType'].includes(action.name)) { + matchCirclesToData({ dataConfiguration, pointRadius: graphModel.getPointRadius(), diff --git a/src/plugins/graph/models/data-configuration-model.ts b/src/plugins/graph/models/data-configuration-model.ts index 2dc546f684..f017054f3a 100644 --- a/src/plugins/graph/models/data-configuration-model.ts +++ b/src/plugins/graph/models/data-configuration-model.ts @@ -2,8 +2,8 @@ import { observable } from "mobx"; import {scaleQuantile, ScaleQuantile, schemeBlues} from "d3"; import { getSnapshot, Instance, ISerializedActionCall, SnapshotIn, types} from "mobx-state-tree"; import {AttributeType, attributeTypes} from "../../../models/data/attribute"; -import {ICase} from "../../../models/data/data-set-types"; -import {DataSet, IDataSet } from "../../../models/data/data-set"; +import { ICase } from "../../../models/data/data-set-types"; +import { DataSet, IDataSet } from "../../../models/data/data-set"; import {getCategorySet, ISharedCaseMetadata, SharedCaseMetadata} from "../../../models/shared/shared-case-metadata"; import {isRemoveAttributeAction, isSetCaseValuesAction} from "../../../models/data/data-set-actions"; import {FilteredCases, IFilteredChangedCases} from "../../../models/data/filtered-cases"; @@ -90,6 +90,9 @@ export const DataConfigurationModel = types get yAttributeIDs() { return this.yAttributeDescriptions.map((d: IAttributeDescriptionSnapshot) => d.attributeID); }, + attributeIdforPlotNumber(plotNum: number) { + return this.yAttributeIDs[plotNum]; + }, /** * Returns the sequential number of the given Y attribute ID. * This includes the rightNumeric attribute if any. @@ -713,7 +716,14 @@ export const DataConfigurationModel = types removeAttributeFromRole(role: GraphAttrRole) { self._setAttributeDescription(role); }, - setAttributeForRole(role: GraphAttrRole, desc?: IAttributeDescriptionSnapshot) { + /** + * Assign the Attribute to the given graph role. + * By default will also select the attribute. + * @param role graph role. + * @param desc attribute description, including the attribute ID and optionally a type. + * @param select boolean default true to select the attribute. + */ + setAttributeForRole(role: GraphAttrRole, desc?: IAttributeDescriptionSnapshot, select: boolean=true) { if (role === 'y') { // Setting "Y" role implies that user only wants one, or no Y attributes. while (self._yAttributeDescriptions.length) { @@ -721,20 +731,16 @@ export const DataConfigurationModel = types } if (desc && desc.attributeID !== '') { self._yAttributeDescriptions.push(desc); - self.dataset?.setSelectedAttributes([desc.attributeID]); } } else if (role === 'yPlus' && desc && desc.attributeID !== '') { self._yAttributeDescriptions.push(desc); } else if (role === 'rightNumeric') { this.setY2Attribute(desc); - if (desc) { - self.dataset?.setSelectedAttributes([desc.attributeID]); - } } else { self._setAttributeDescription(role, desc); - if (desc) { - self.dataset?.setSelectedAttributes([desc.attributeID]); - } + } + if (desc && select) { + self.dataset?.setSelectedAttributes([desc.attributeID]); } this.syncFilteredCasesCount(true); if (role === 'legend') { diff --git a/src/plugins/graph/models/graph-layer-model.ts b/src/plugins/graph/models/graph-layer-model.ts index 37128bb9c3..68f4eaa08a 100644 --- a/src/plugins/graph/models/graph-layer-model.ts +++ b/src/plugins/graph/models/graph-layer-model.ts @@ -1,4 +1,4 @@ -import { getParentOfType, Instance, SnapshotIn, types } from "@concord-consortium/mobx-state-tree"; +import { getParentOfType, Instance, SnapshotIn, types } from "mobx-state-tree"; import { typedId } from "../../../utilities/js-utils"; import { onAnyAction } from "../../../utilities/mst-utils"; import { DataConfigurationModel, IDataConfigurationModel } from "./data-configuration-model"; @@ -7,9 +7,10 @@ import { GraphPlace } from "../imports/components/axis-graph-shared"; import { GraphAttrRole } from "../graph-types"; import { IUpdateCategoriesOptions } from "../adornments/adornment-models"; import { GraphModel } from "./graph-model"; -import { IDataSet } from "../../../models/data/data-set"; +import { IDataSet, addCanonicalCasesToDataSet } from "../../../models/data/data-set"; import { ISharedCaseMetadata } from "../../../models/shared/shared-case-metadata"; import { DotsElt } from "../d3-types"; +import { ICaseCreation } from "../../../models/data/data-set-types"; export const GraphLayerModel = types .model('GraphLayerModel') @@ -63,6 +64,28 @@ export const GraphLayerModel = types clearAutoAssignedAttributes() { self.autoAssignedAttributes = []; }, + /** + * Add a point to this layer with the given x and y values. + * A plot number can be provided; it defaults to 0 since currently + * only a single trace of manually created points can be created in the graph. + * @param x + * @param y + * @param plotNum optional, default 0 + */ + addPoint(x: number, y: number, plotNum: number=0) { + const dataset = self.config.dataset; + const xAttr = self.config.attributeID("x"); + const yAttr = self.config.yAttributeIDs[plotNum]; + if (dataset && xAttr && yAttr) { + const newCase: ICaseCreation = {}; + newCase[xAttr] = x; + newCase[yAttr] = y; + const caseAdded = addCanonicalCasesToDataSet(dataset, [newCase]); + // The values are already correct, but various reactions in the graph code + // expect there to be a value setting action after case creation. + dataset.setCanonicalCaseValues(caseAdded); + } + }, configureLinkedLayer() { if (!self.config) { console.warn("GraphLayerModel.configureLinkedLayer requires a dataset"); diff --git a/src/plugins/graph/models/graph-model.ts b/src/plugins/graph/models/graph-model.ts index 9d0dc5f4e0..4c0a281af7 100644 --- a/src/plugins/graph/models/graph-model.ts +++ b/src/plugins/graph/models/graph-model.ts @@ -10,7 +10,7 @@ import { } from "../imports/components/axis/models/axis-model"; import { GraphPlace } from "../imports/components/axis-graph-shared"; import { - GraphAttrRole, hoverRadiusFactor, kDefaultAxisLabel, kDefaultNumericAxisBounds, kGraphTileType, + GraphAttrRole, GraphEditMode, hoverRadiusFactor, kDefaultAxisLabel, kDefaultNumericAxisBounds, kGraphTileType, PlotType, PlotTypes, pointRadiusMax, pointRadiusSelectionAddend } from "../graph-types"; import { withoutUndo } from "../../../models/history/without-undo"; @@ -81,7 +81,10 @@ export const GraphModel = TileContentModel yAttributeLabel: types.optional(types.string, kDefaultAxisLabel) }) .volatile(self => ({ - // prevDataSetId: "", + // True if a dragging operation is ongoing - automatic rescaling is deferred until drag is done. + interactionInProgress: false, + editingMode: "none" as GraphEditMode, + editingLayerId: undefined as string|undefined })) .preProcessSnapshot((snapshot: any) => { const hasLayerAlready:boolean = (snapshot?.layers?.length || 0) > 0; @@ -204,6 +207,10 @@ export const GraphModel = TileContentModel attributeType(role: GraphAttrRole) { return self.layers[0].config.attributeType(role); }, + getLayerById(layerId: string): IGraphLayerModel|undefined { + if (!layerId) return undefined; + return self.layers.find(layer => layer.id === layerId); + }, layerForDataConfigurationId(dataConfID: string) { return self.layers.find(layer => layer.config.id === dataConfID); }, @@ -229,6 +236,13 @@ export const GraphModel = TileContentModel getEditableLayers() { return self.layers.filter(l => l.editable); }, + /** + * Return the layer currently being edited, or undefined if none. + */ + get editingLayer(): IGraphLayerModel|undefined { + if (!self.editingLayerId) return undefined; + return this.getLayerById(self.editingLayerId); + }, /** * Find all tooltip-related attributes from all layers. * Returned as a list of { role, attribute } pairs. @@ -361,8 +375,8 @@ export const GraphModel = TileContentModel const dataConfiguration = DataConfigurationModel.create(); layer.setDataConfiguration(dataConfiguration); dataConfiguration.setDataset(dataset, metadata); - dataConfiguration.setAttributeForRole("x", { attributeID: xAttr.id, type: "numeric" }); - dataConfiguration.setAttributeForRole("y", { attributeID: yAttr.id, type: "numeric" }); + dataConfiguration.setAttributeForRole("x", { attributeID: xAttr.id, type: "numeric" }, false); + dataConfiguration.setAttributeForRole("y", { attributeID: yAttr.id, type: "numeric" }, false); } }, setXAttributeLabel(label: string) { @@ -370,6 +384,22 @@ export const GraphModel = TileContentModel }, setYAttributeLabel(label: string) { self.yAttributeLabel = label; + }, + setEditingMode(mode: GraphEditMode, layer?: IGraphLayerModel) { + self.editingMode = mode; + if (mode === "none") { + self.editingLayerId = undefined; + } else { + if (layer) { + self.editingLayerId = layer && layer.id; + } else { + const editables = self.getEditableLayers(); + self.editingLayerId = editables.length>0 ? editables[0].id : undefined; + } + } + }, + setInteractionInProgress(value: boolean) { + self.interactionInProgress = value; } })) .actions(self => ({ @@ -514,6 +544,21 @@ export const GraphModel = TileContentModel const colorIndex = self._idColors.get(id); if (colorIndex === undefined) return "black"; return clueGraphColors[colorIndex % clueGraphColors.length].name; + }, + getEditablePointsColor() { + let color = "#000000"; + let layer = self.editingLayer; + if (!layer) { + // Even if no layer is currently being edited, show the color of the one that would be. + layer = self.getEditableLayers()?.[0]; + } + if (layer) { + const yAttributes = layer.config.yAttributeIDs; + if (yAttributes.length > 0) { + color = this.getColorForId(yAttributes[0]); + } + } + return color; } })) .actions(self => ({ diff --git a/src/plugins/graph/utilities/graph-utils.ts b/src/plugins/graph/utilities/graph-utils.ts index e6d8900503..8cd5885241 100644 --- a/src/plugins/graph/utilities/graph-utils.ts +++ b/src/plugins/graph/utilities/graph-utils.ts @@ -1,10 +1,9 @@ import {extent, format, select, timeout} from "d3"; import React from "react"; import { isInteger} from "lodash"; -import { SnapshotOut } from "mobx-state-tree"; +import { SnapshotOut, getParentOfType } from "mobx-state-tree"; import { IClueObjectSnapshot } from "../../../models/annotations/clue-object"; -import { PartialSharedModelEntry } from "../../../models/document/document-content-types"; import { UpdatedSharedDataSetIds, replaceJsonStringsWithUpdatedIds } from "../../../models/shared/shared-data-set"; import { CaseData, DotSelection, DotsElt, selectGraphDots, selectInnerCircles, selectOuterCircles @@ -24,6 +23,7 @@ import {IDataConfigurationModel} from "../models/data-configuration-model"; import {measureText} from "../../../components/tiles/hooks/use-measure-text"; import { GraphModel, IGraphModel } from "../models/graph-model"; import { isFiniteNumber } from "../../../utilities/math-utils"; +import { SharedModelEntrySnapshotType } from "../../../models/document/shared-model-entry"; /** * Utility routines having to do with graph entities @@ -130,19 +130,38 @@ export function getPointTipText(caseID: string, attributeIDs: (string|undefined) export function handleClickOnDot(event: MouseEvent, caseData: CaseData, dataConfiguration?: IDataConfigurationModel) { if (!dataConfiguration) return; + event.stopPropagation(); + const graphModel = getParentOfType(dataConfiguration, GraphModel); const dataset = dataConfiguration.dataset; const yAttributeId = dataConfiguration.yAttributeID(caseData.plotNum); const yCell = { attributeId: yAttributeId, caseId: caseData.caseID }; - const extendSelection = event.shiftKey, - cellIsSelected = dataset?.isCellSelected(yCell); - if (!cellIsSelected) { - if (extendSelection) { // y cell is not selected and Shift key is down => add y cell to selection - dataset?.selectCells([yCell]); - } else { // y cell is not selected and Shift key is up => only this y cell should be selected - dataset?.setSelectedCells([yCell]); + + if (graphModel.editingMode==="add" + && graphModel.editingLayer && graphModel.editingLayer.config !== dataConfiguration) { + // We're in "Add points" mode, and clicked on a case that is not in the editable dataset: + // add a case to the editable dataset at the same values as the existing case clicked on. + const existingCase = dataset?.getCanonicalCase(caseData.caseID); + if (existingCase) { + const xAttributeId = dataConfiguration.xAttributeID; + const x = dataset?.getNumeric(caseData.caseID, xAttributeId); + const y = dataset?.getNumeric(caseData.caseID, yAttributeId); + if (x !== undefined && y !== undefined) { + graphModel.editingLayer.addPoint(x, y); + } + } + } else { + // Otherwise, clicking on a dot means updating the selection. + const extendSelection = event.shiftKey, + cellIsSelected = dataset?.isCellSelected(yCell); + if (!cellIsSelected) { + if (extendSelection) { // Dot is not selected and Shift key is down => add to selection + dataset?.selectCells([yCell]); + } else { // Dot is not selected and Shift key is up => only this dot should be selected + dataset?.setSelectedCells([yCell]); + } + } else if (extendSelection) { // Dot is selected and Shift key is down => deselect + dataset?.selectCells([yCell], false); } - } else if (extendSelection) { // y cell is selected and Shift key is down => deselect cell - dataset?.selectCells([yCell], false); } } @@ -180,8 +199,7 @@ export interface IMatchCirclesProps { export function matchCirclesToData(props: IMatchCirclesProps) { const { dataConfiguration, enableAnimation, instanceId, dotsElement } = props; const allCaseData = dataConfiguration.joinedCaseDataArrays; - const caseDataKeyFunc = (d: CaseData) => `${d.dataConfigID}-${d.plotNum}-${d.caseID}`; - + const caseDataKeyFunc = (d: CaseData) => `${d.dataConfigID}_${instanceId}_${d.plotNum}_${d.caseID}`; // Create the circles const allCircles = selectGraphDots(dotsElement); if (!allCircles) return; @@ -193,7 +211,7 @@ export function matchCirclesToData(props: IMatchCirclesProps) { enter => { const g = enter.append('g') .attr('class', `graph-dot`) - .property('id', (d: CaseData) => `${d.dataConfigID}_${instanceId}_${d.caseID}`); + .property('id', (d: CaseData) => `${d.dataConfigID}_${instanceId}_${d.plotNum}_${d.caseID}`); g.append('circle') .attr('class', 'outer-circle'); g.append('circle') @@ -203,7 +221,6 @@ export function matchCirclesToData(props: IMatchCirclesProps) { ); dotsElement && select(dotsElement).on('click', (event: MouseEvent) => { - event.stopPropagation(); const target = select(event.target as SVGSVGElement); if (target.node()?.nodeName === 'circle') { handleClickOnDot(event, target.datum() as CaseData, dataConfiguration); @@ -487,9 +504,14 @@ export function setPointCoordinates(props: ISetPointCoordinates) { const setPositions = (dots: DotSelection | null) => { if (dots !== null) { + // This utilizes a transition() to move the dots smoothly to new positions. + // However, any dots that don't have a position already should just move + // immediately; otherwise they enter from the top left for no reason. dots .transition() - .duration(duration) + .duration((d, i, nodes) => { + return nodes[i].getAttribute('transform') ? duration : 1; + }) .attr('transform', transformForCase) // The rest of this should not be necessary, but works around an apparent Chrome bug. // At least in Chrome v120 on MacOS, if the points are animated from a position far off-screen, @@ -570,7 +592,7 @@ export function decipherDotId(dotId: string) { export function updateGraphContentWithNewSharedModelIds( content: SnapshotOut, - sharedDataSetEntries: PartialSharedModelEntry[], + sharedDataSetEntries: SharedModelEntrySnapshotType[], updatedSharedModelMap: Record ) { return replaceJsonStringsWithUpdatedIds(content, ...Object.values(updatedSharedModelMap)); @@ -578,7 +600,7 @@ export function updateGraphContentWithNewSharedModelIds( export function updateGraphObjectWithNewSharedModelIds( object: IClueObjectSnapshot, - sharedDataSetEntries: PartialSharedModelEntry[], + sharedDataSetEntries: SharedModelEntrySnapshotType[], updatedSharedModelMap: Record ) { if (object.objectType === "dot") { diff --git a/src/public/demo/units/qa-config-subtabs/content.json b/src/public/demo/units/qa-config-subtabs/content.json index 5694d5ad91..1b84cf4c83 100644 --- a/src/public/demo/units/qa-config-subtabs/content.json +++ b/src/public/demo/units/qa-config-subtabs/content.json @@ -39,8 +39,12 @@ "tools": [ "link-tile-multiple", "add-points-by-hand", + "|", "fit-all", - "toggle-lock" + "toggle-lock", + "|", + "move-points", + "add-points" ] }, "diagram": { diff --git a/src/public/demo/units/qa/content.json b/src/public/demo/units/qa/content.json index 975f93ac20..aab3c3d309 100644 --- a/src/public/demo/units/qa/content.json +++ b/src/public/demo/units/qa/content.json @@ -57,7 +57,6 @@ "connectPointsByDefault": true, "tools": [ "link-tile-multiple", - "add-points-by-hand", "fit-all", "toggle-lock" ]