From 9d51ae60419dffef69d09ff7ec2d250fe28e668f Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Tue, 27 Feb 2024 11:39:43 -0800 Subject: [PATCH 01/78] Publish for PR --- src/plugins/numberline/components/numberline-tile.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index edcc9edbb1..4eb84934ff 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -24,9 +24,18 @@ import { EditableNumberlineValue } from './editable-numberline-value'; import "./numberline-tile.scss"; +// As students, we want to express both closed form counting and inequalities which end with an open circle point. +// - add an open circle point to the toolbar +// - revise other toolbar buttons to the latest versions +// - when the open circle point is used, create an open circle point +// - point types are saved and restored and can be used in curriculum. + + export const NumberlineTile: React.FC = observer(function NumberlineTile(props){ const { documentContent, model, readOnly, scale, tileElt, onRegisterTileApi, onUnregisterTileApi } = props; + console.log("numberlineTile for PR change"); + const content = model.content as NumberlineContentModelType; const [hoverPointId, setHoverPointId] = useState(""); const [_selectedPointId, setSelectedPointId] = useState(""); // Just used to rerender when a point is selected From 7d33c3e0c3ad2d8e3555db822d55623f8309693b Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Tue, 27 Feb 2024 17:04:33 -0800 Subject: [PATCH 02/78] Publish for PR --- src/plugins/numberline/components/numberline-tile.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index 4eb84934ff..dd372c7a6f 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -24,6 +24,7 @@ import { EditableNumberlineValue } from './editable-numberline-value'; import "./numberline-tile.scss"; + // As students, we want to express both closed form counting and inequalities which end with an open circle point. // - add an open circle point to the toolbar // - revise other toolbar buttons to the latest versions From 6a94eb85cf50771e8e25b25a8fe31d09a5746c09 Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Wed, 28 Feb 2024 12:42:48 -0800 Subject: [PATCH 03/78] Push for diff - placed all icons --- .../elements/tile/NumberlineToolTile.js | 2 +- .../assets/numberline-toolbar-clear-icon.svg | 5 -- ...mberline-toolbar-delete-selection-icon.svg | 6 +++ .../assets/numberline-toolbar-point-icon.svg | 7 +-- .../numberline-toolbar-point-open-icon.svg | 7 +++ .../assets/numberline-toolbar-reset-icon.svg | 6 +++ .../assets/numberline-toolbar-select-tool.svg | 6 +++ .../numberline/components/numberline-tile.tsx | 7 ++- .../components/numberline-toolbar-buttons.tsx | 51 +++++++++++++++---- .../components/numberline-toolbar.tsx | 24 ++++++--- 10 files changed, 87 insertions(+), 34 deletions(-) delete mode 100644 src/plugins/numberline/assets/numberline-toolbar-clear-icon.svg create mode 100644 src/plugins/numberline/assets/numberline-toolbar-delete-selection-icon.svg create mode 100644 src/plugins/numberline/assets/numberline-toolbar-point-open-icon.svg create mode 100644 src/plugins/numberline/assets/numberline-toolbar-reset-icon.svg create mode 100644 src/plugins/numberline/assets/numberline-toolbar-select-tool.svg diff --git a/cypress/support/elements/tile/NumberlineToolTile.js b/cypress/support/elements/tile/NumberlineToolTile.js index 4e1fbcc240..108d4eff40 100644 --- a/cypress/support/elements/tile/NumberlineToolTile.js +++ b/cypress/support/elements/tile/NumberlineToolTile.js @@ -29,7 +29,7 @@ class NumberlineToolTile { this.getClearButton().click(); } getClearButton(){ - return cy.get(`.clear-points`); + return cy.get(`.clear-points`); //TODO: fix this } setMaxValue(max){ this.getMaxBox().dblclick().clear().type(`${max}{enter}`); diff --git a/src/plugins/numberline/assets/numberline-toolbar-clear-icon.svg b/src/plugins/numberline/assets/numberline-toolbar-clear-icon.svg deleted file mode 100644 index 6b3e04396f..0000000000 --- a/src/plugins/numberline/assets/numberline-toolbar-clear-icon.svg +++ /dev/null @@ -1,5 +0,0 @@ - - - diff --git a/src/plugins/numberline/assets/numberline-toolbar-delete-selection-icon.svg b/src/plugins/numberline/assets/numberline-toolbar-delete-selection-icon.svg new file mode 100644 index 0000000000..de3e9cfe17 --- /dev/null +++ b/src/plugins/numberline/assets/numberline-toolbar-delete-selection-icon.svg @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/plugins/numberline/assets/numberline-toolbar-point-icon.svg b/src/plugins/numberline/assets/numberline-toolbar-point-icon.svg index d011f50851..31db530f7c 100644 --- a/src/plugins/numberline/assets/numberline-toolbar-point-icon.svg +++ b/src/plugins/numberline/assets/numberline-toolbar-point-icon.svg @@ -1,10 +1,7 @@ - - - - - + + diff --git a/src/plugins/numberline/assets/numberline-toolbar-point-open-icon.svg b/src/plugins/numberline/assets/numberline-toolbar-point-open-icon.svg new file mode 100644 index 0000000000..3dc9ebba8e --- /dev/null +++ b/src/plugins/numberline/assets/numberline-toolbar-point-open-icon.svg @@ -0,0 +1,7 @@ + + + + + + + diff --git a/src/plugins/numberline/assets/numberline-toolbar-reset-icon.svg b/src/plugins/numberline/assets/numberline-toolbar-reset-icon.svg new file mode 100644 index 0000000000..7a7ab612a9 --- /dev/null +++ b/src/plugins/numberline/assets/numberline-toolbar-reset-icon.svg @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/plugins/numberline/assets/numberline-toolbar-select-tool.svg b/src/plugins/numberline/assets/numberline-toolbar-select-tool.svg new file mode 100644 index 0000000000..5de5dc6003 --- /dev/null +++ b/src/plugins/numberline/assets/numberline-toolbar-select-tool.svg @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index dd372c7a6f..a8b3394d19 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -24,8 +24,8 @@ import { EditableNumberlineValue } from './editable-numberline-value'; import "./numberline-tile.scss"; - -// As students, we want to express both closed form counting and inequalities which end with an open circle point. +// - Guidelines - +// - As students, we want to express both closed form counting and inequalities which end with an open circle point. // - add an open circle point to the toolbar // - revise other toolbar buttons to the latest versions // - when the open circle point is used, create an open circle point @@ -34,8 +34,7 @@ import "./numberline-tile.scss"; export const NumberlineTile: React.FC = observer(function NumberlineTile(props){ const { documentContent, model, readOnly, scale, tileElt, onRegisterTileApi, onUnregisterTileApi } = props; - - console.log("numberlineTile for PR change"); + console.log("-------< Numberline >-----------"); const content = model.content as NumberlineContentModelType; const [hoverPointId, setHoverPointId] = useState(""); diff --git a/src/plugins/numberline/components/numberline-toolbar-buttons.tsx b/src/plugins/numberline/components/numberline-toolbar-buttons.tsx index 79a42545fe..9803d8bb36 100644 --- a/src/plugins/numberline/components/numberline-toolbar-buttons.tsx +++ b/src/plugins/numberline/components/numberline-toolbar-buttons.tsx @@ -3,8 +3,10 @@ import classNames from "classnames"; import { Tooltip, TooltipProps } from "react-tippy"; import { useTooltipOptions } from "../../../hooks/use-tooltip-options"; -import PlacePointIcon from "../assets/numberline-toolbar-point-icon.svg"; -import ClearPointsIcon from "../assets/numberline-toolbar-clear-icon.svg"; //undo icon for now +import SelectIcon from "../assets/numberline-toolbar-select-tool.svg"; +import PointIcon from "../assets/numberline-toolbar-point-icon.svg"; +import PointOpenIcon from "../assets/numberline-toolbar-point-open-icon.svg"; +import ResetIcon from "../assets/numberline-toolbar-reset-icon.svg"; import DeletePointsIcon from "../assets/numberline-toolbar-delete-icon.svg"; import "./numberline-toolbar.scss"; @@ -32,27 +34,54 @@ interface ISetNumberlineHandler { onClick?: () => void; } -export const PlacePointButton = ({ onClick }: ISetNumberlineHandler) => ( +// export const + +//Select +//Point +//Point-Open +//Reset +//Delete + +export const SelectButton = ({ onClick }: ISetNumberlineHandler) => ( + } + onClick={onClick} + tooltipOptions={{ title: "Select Point"}} + /> +); + +export const PointButton = ({ onClick }: ISetNumberlineHandler) => ( } + className="point-numberline-toolbar" + icon={} onClick={onClick} tooltipOptions={{ title: "Place Point"}} /> ); -export const ClearPointsButton = ({ onClick }: ISetNumberlineHandler) => ( +export const PointOpenButton = ({ onClick }: ISetNumberlineHandler) => ( } + className="point-open-numberline-toolbar" + icon={} onClick={onClick} - tooltipOptions={{ title: "Clear Points"}} + tooltipOptions={{ title: "Place Open Point"}} /> ); -export const DeletePointButton = ({ onClick }: ISetNumberlineHandler) => ( +export const ResetButton = ({ onClick }: ISetNumberlineHandler) => ( + } + onClick={onClick} + tooltipOptions={{ title: "Reset"}} + /> +); + + +export const DeleteButton = ({ onClick }: ISetNumberlineHandler) => ( } onClick={onClick} tooltipOptions={{ title: "Delete Point(s)"}} diff --git a/src/plugins/numberline/components/numberline-toolbar.tsx b/src/plugins/numberline/components/numberline-toolbar.tsx index e6522da517..cd5ea3a5bb 100644 --- a/src/plugins/numberline/components/numberline-toolbar.tsx +++ b/src/plugins/numberline/components/numberline-toolbar.tsx @@ -6,11 +6,11 @@ import { IFloatingToolbarProps, useFloatingToolbarLocation } from "../../../components/tiles/hooks/use-floating-toolbar-location"; import { useSettingFromStores } from "../../../hooks/use-stores"; -import { PlacePointButton, ClearPointsButton, DeletePointButton } from "./numberline-toolbar-buttons"; +import { SelectButton, PointButton, PointOpenButton, ResetButton, DeleteButton } from "./numberline-toolbar-buttons"; import "./numberline-toolbar.scss"; -const defaultButtons = ["place-point", "clear-points", "delete-points"]; +const defaultButtons = ["select", "point", "point-open", "reset", "delete"]; interface INumberlineToolbarProps extends IFloatingToolbarProps { handleClearPoints: () => void; @@ -20,6 +20,8 @@ interface INumberlineToolbarProps extends IFloatingToolbarProps { export const NumberlineToolbar: React.FC = observer((props) => { const { documentContent, tileElt, onIsEnabled, handleClearPoints, handleDeletePoint, ...others } = props; + + console.log("-------< NumberlineToolbar >-----------"); const enabled = onIsEnabled(); const location = useFloatingToolbarLocation({ documentContent, @@ -31,16 +33,22 @@ export const NumberlineToolbar: React.FC = observer((pr }); const buttonSettings = useSettingFromStores("tools", "numberline") as unknown as string[] | undefined; + console.log("buttonSettings:", buttonSettings); const buttons = buttonSettings || defaultButtons; + console.log("buttons:", buttons); const getToolbarButton = (toolName: string) => { switch (toolName) { - case "place-point": - return ; - case "clear-points": - return ; - case "delete-points": - return ; + case "select": + return ; + case "point": + return ; + case "point-open": + return ; + case "reset": + return ; + case "delete": + return ; } }; From 4207784321a82c795a9afe542316bf64179c59cb Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Wed, 28 Feb 2024 15:15:10 -0800 Subject: [PATCH 04/78] 5 buttons are added, UI changes and state is captured. --- .../numberline/components/numberline-tile.tsx | 29 ++++---- .../components/numberline-toolbar-buttons.tsx | 69 ++++++++++--------- .../components/numberline-toolbar.scss | 7 ++ .../components/numberline-toolbar.tsx | 28 ++++++-- .../numberline/models/numberline-content.ts | 2 +- 5 files changed, 83 insertions(+), 52 deletions(-) diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index a8b3394d19..20037bcce2 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -24,17 +24,21 @@ import { EditableNumberlineValue } from './editable-numberline-value'; import "./numberline-tile.scss"; -// - Guidelines - +//**********************✔️•↳******************* GUIDELINES ************************************************ // - As students, we want to express both closed form counting and inequalities which end with an open circle point. -// - add an open circle point to the toolbar -// - revise other toolbar buttons to the latest versions -// - when the open circle point is used, create an open circle point -// - point types are saved and restored and can be used in curriculum. + +// ✔️ add an open circle point to the toolbar +// ✔️ add a select point arrow to the toolbar +// ✔️ revise solid point button to the latest versions +// ✔️ when the open circle point is used, create an open circle point +// when the solid circle point is used, create a solid point like before +// when the selection button is used, points can be moved or selected for deletion +// point types are saved and restored and can be used in curriculum. + export const NumberlineTile: React.FC = observer(function NumberlineTile(props){ const { documentContent, model, readOnly, scale, tileElt, onRegisterTileApi, onUnregisterTileApi } = props; - console.log("-------< Numberline >-----------"); const content = model.content as NumberlineContentModelType; const [hoverPointId, setHoverPointId] = useState(""); @@ -43,9 +47,6 @@ export const NumberlineTile: React.FC = observer(function Numberline const isTileSelected = ui.isSelectedTile(model); //---------------- Model Manipulation Functions ------------------------------------------------- - const deleteSelectedPoints = useCallback(() => { - content.deleteSelectedPoints(); - }, [content]); const createPoint = (xValue: number) => { if (!readOnly) { @@ -62,6 +63,10 @@ export const NumberlineTile: React.FC = observer(function Numberline } }; + const deleteSelectedPoints = useCallback(() => { + content.deleteSelectedPoints(); + }, [content]); + // Set up key handling const hotKeys = useRef(new HotKeys()); useEffect(()=>{ @@ -73,6 +78,7 @@ export const NumberlineTile: React.FC = observer(function Numberline } }, [deleteSelectedPoints, readOnly]); + //---------------- Calculate Width Of Tile / Scale ---------------------------------------------- const documentScrollerRef = useRef(null); const [tileWidth, setTileWidth] = useState(0); @@ -108,9 +114,6 @@ export const NumberlineTile: React.FC = observer(function Numberline return () => obs?.disconnect(); }, []); - - - //----------------- Register Tile API functions ------------------------------------------------- const annotationPointCenter = useCallback((pointId: string) => { const point = content.getPoint(pointId); @@ -382,6 +385,8 @@ export const NumberlineTile: React.FC = observer(function Numberline scale={scale} handleClearPoints={() => content.deleteAllPoints()} handleDeletePoint={deleteSelectedPoints} + + />
void; tooltipOptions?: TooltipProps + selected?: boolean; + } -const NumberlineButton = ({ className, icon, onClick, tooltipOptions}: INumberlineButtonProps) => { +const NumberlineButton = ({ className, icon, onClick, tooltipOptions, selected}: INumberlineButtonProps) => { const to = useTooltipOptions(tooltipOptions); const classes = classNames("toolbar-button", className); return ( @@ -32,46 +36,47 @@ const NumberlineButton = ({ className, icon, onClick, tooltipOptions}: INumberli interface ISetNumberlineHandler { onClick?: () => void; + selected?: boolean; } -// export const - -//Select -//Point -//Point-Open -//Reset -//Delete - export const SelectButton = ({ onClick }: ISetNumberlineHandler) => ( } onClick={onClick} tooltipOptions={{ title: "Select Point"}} - /> -); -export const PointButton = ({ onClick }: ISetNumberlineHandler) => ( - } - onClick={onClick} - tooltipOptions={{ title: "Place Point"}} /> ); -export const PointOpenButton = ({ onClick }: ISetNumberlineHandler) => ( - } - onClick={onClick} - tooltipOptions={{ title: "Place Open Point"}} - /> -); +export const PointButton = ({ onClick }: ISetNumberlineHandler) => { + return ( + } + onClick={onClick} + tooltipOptions={{ title: "Place Point"}} + /> + ); +}; + + + +export const PointOpenButton = ({ onClick, selected }: ISetNumberlineHandler) => { + console.log("point open button with selected:", selected); + return ( + } + onClick={onClick} + tooltipOptions={{ title: "Place Open Point"}} + /> + ); +}; export const ResetButton = ({ onClick }: ISetNumberlineHandler) => ( } onClick={onClick} tooltipOptions={{ title: "Reset"}} @@ -81,8 +86,8 @@ export const ResetButton = ({ onClick }: ISetNumberlineHandler) => ( export const DeleteButton = ({ onClick }: ISetNumberlineHandler) => ( } + className="delete-button" + icon={} onClick={onClick} tooltipOptions={{ title: "Delete Point(s)"}} /> diff --git a/src/plugins/numberline/components/numberline-toolbar.scss b/src/plugins/numberline/components/numberline-toolbar.scss index 097c20afba..ef90ed028a 100644 --- a/src/plugins/numberline/components/numberline-toolbar.scss +++ b/src/plugins/numberline/components/numberline-toolbar.scss @@ -39,5 +39,12 @@ opacity: 35%; pointer-events: none; } + + &.point-open-button { + &.selected { + background-color: $workspace-teal-light-4; + } + } + } } diff --git a/src/plugins/numberline/components/numberline-toolbar.tsx b/src/plugins/numberline/components/numberline-toolbar.tsx index cd5ea3a5bb..4c701212ce 100644 --- a/src/plugins/numberline/components/numberline-toolbar.tsx +++ b/src/plugins/numberline/components/numberline-toolbar.tsx @@ -1,7 +1,6 @@ import { observer } from "mobx-react"; -import React from "react"; +import React, { useState } from "react"; import ReactDOM from "react-dom"; - import { IFloatingToolbarProps, useFloatingToolbarLocation } from "../../../components/tiles/hooks/use-floating-toolbar-location"; @@ -21,6 +20,13 @@ export const NumberlineToolbar: React.FC = observer((pr const { documentContent, tileElt, onIsEnabled, handleClearPoints, handleDeletePoint, ...others } = props; + //---------------- Determine Selected Point Type ------------------------ + const [selPointType, setSelPointType] = useState('filled'); // 'filled' or 'open' + const handleSelectPointType = (type: 'filled' | 'open') => { + setSelPointType(type); + + }; + console.log("-------< NumberlineToolbar >-----------"); const enabled = onIsEnabled(); const location = useFloatingToolbarLocation({ @@ -32,21 +38,29 @@ export const NumberlineToolbar: React.FC = observer((pr ...others }); + const buttonSettings = useSettingFromStores("tools", "numberline") as unknown as string[] | undefined; - console.log("buttonSettings:", buttonSettings); const buttons = buttonSettings || defaultButtons; - console.log("buttons:", buttons); + const getToolbarButton = (toolName: string) => { switch (toolName) { case "select": return ; case "point": - return ; + return handleSelectPointType('filled')} + selected={selPointType === 'filled'} + />; case "point-open": - return ; + return handleSelectPointType('open')} + selected={selPointType === 'open'} + />; case "reset": - return ; + return ; case "delete": return ; } diff --git a/src/plugins/numberline/models/numberline-content.ts b/src/plugins/numberline/models/numberline-content.ts index 9184e4292c..98422505fe 100644 --- a/src/plugins/numberline/models/numberline-content.ts +++ b/src/plugins/numberline/models/numberline-content.ts @@ -118,7 +118,7 @@ export const NumberlineContentModel = TileContentModel } self.clearSelectedPoints(); }, - deleteAllPoints() { + deleteAllPoints() { //TODO: This is not being used we may need to get rid of it self.points.clear(); }, })) From a387fb56fe725a1d89c702cd73b662237b33ff06 Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Thu, 29 Feb 2024 12:25:28 -0800 Subject: [PATCH 05/78] Push to pair with Boris --- .../numberline/components/numberline-tile.tsx | 39 ++++++++++++------- .../components/numberline-toolbar-buttons.tsx | 15 +++---- .../components/numberline-toolbar.scss | 6 +++ .../components/numberline-toolbar.tsx | 27 +++++-------- .../numberline/models/numberline-content.ts | 13 ++++--- 5 files changed, 55 insertions(+), 45 deletions(-) diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index 20037bcce2..cf21928a5d 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -35,20 +35,33 @@ import "./numberline-tile.scss"; // when the selection button is used, points can be moved or selected for deletion // point types are saved and restored and can be used in curriculum. - - export const NumberlineTile: React.FC = observer(function NumberlineTile(props){ const { documentContent, model, readOnly, scale, tileElt, onRegisterTileApi, onUnregisterTileApi } = props; - + // console.log("-------------------"); const content = model.content as NumberlineContentModelType; + // console.log("content:", content); const [hoverPointId, setHoverPointId] = useState(""); const [_selectedPointId, setSelectedPointId] = useState(""); // Just used to rerender when a point is selected const ui = useUIStore(); const isTileSelected = ui.isSelectedTile(model); - //---------------- Model Manipulation Functions ------------------------------------------------- + /* ========================== [ Determine Point is Open or Filled ] ========================= */ + const [pointTypeIsOpen, setPointTypeIsOpen] = useState(false); //default start with filled in point + + const handleCreatePointType = (_isOpen: boolean) => { + console.log("--------------------------------------"); + console.log("initial STATE:", pointTypeIsOpen); + setPointTypeIsOpen(_isOpen); + }; - const createPoint = (xValue: number) => { + useEffect(() => { + console.log("Updated STATE:", pointTypeIsOpen); + }, [pointTypeIsOpen]); + + /* ============================ [ Model Manipulation Functions ] ============================ */ + const createPoint = (xValue: number, _pointTypeIsOpen: boolean) => { + const pointType = (_pointTypeIsOpen) ? "OPEN" : "FILLED"; + console.log("createPoint with xValue", xValue, "pointTypeisOpen:", pointType); if (!readOnly) { const point = content.createAndSelectPoint(xValue); setHoverPointId(point.id); @@ -78,8 +91,7 @@ export const NumberlineTile: React.FC = observer(function Numberline } }, [deleteSelectedPoints, readOnly]); - - //---------------- Calculate Width Of Tile / Scale ---------------------------------------------- + /* ============================ [ Calculate Width of Tile / Scale ] ========================= */ const documentScrollerRef = useRef(null); const [tileWidth, setTileWidth] = useState(0); const containerWidth = tileWidth * kContainerWidth; @@ -114,7 +126,7 @@ export const NumberlineTile: React.FC = observer(function Numberline return () => obs?.disconnect(); }, []); - //----------------- Register Tile API functions ------------------------------------------------- + /* ============================ [ Register Tile API Functions ] ============================= */ const annotationPointCenter = useCallback((pointId: string) => { const point = content.getPoint(pointId); if (!point) return undefined; @@ -177,7 +189,7 @@ export const NumberlineTile: React.FC = observer(function Numberline }); }, [annotationPointCenter, content, getObjectBoundingBox, onRegisterTileApi]); - //------------------- SVG Ref to Numberline & SVG ---------------------------------------------- + /* ============================ [ SVG Ref to Numberline & SVG ] ============================= */ const svgRef = useRef(null); const svg = select(svgRef.current); const svgNode = svg.node(); @@ -195,7 +207,7 @@ export const NumberlineTile: React.FC = observer(function Numberline return isBetweenYBounds && isBetweenXBounds; }; - const handleMouseClick = (e: Event) => { + const handleMouseClick = (e: Event, _pointTypeIsOpen: boolean) => { if (!readOnly){ if (hoverPointId) { const hoverPoint = content.getPoint(hoverPointId); @@ -207,7 +219,7 @@ export const NumberlineTile: React.FC = observer(function Numberline // only create point if we are not hovering over a point and within bounding box const [mouseX, mouseY] = mousePos(e); if (mouseInBoundingBox(mouseX, mouseY)) { - createPoint(xScale.invert(mouseX)); + createPoint(xScale.invert(mouseX), _pointTypeIsOpen); } } } @@ -251,7 +263,7 @@ export const NumberlineTile: React.FC = observer(function Numberline } }; - svg.on("click", (e) => handleMouseClick(e)); + svg.on("click", (e) => handleMouseClick(e, pointTypeIsOpen)); svg.on("mousemove", (e) => handleMouseMove(e)); // * ================================ [ Construct Numberline ] =============================== */ @@ -385,7 +397,8 @@ export const NumberlineTile: React.FC = observer(function Numberline scale={scale} handleClearPoints={() => content.deleteAllPoints()} handleDeletePoint={deleteSelectedPoints} - + handleCreatePointType={handleCreatePointType} + pointTypeIsOpen={pointTypeIsOpen} />
void; - selected?: boolean; + pointTypeIsOpen?: boolean; } export const SelectButton = ({ onClick }: ISetNumberlineHandler) => ( @@ -45,14 +45,14 @@ export const SelectButton = ({ onClick }: ISetNumberlineHandler) => ( icon={} onClick={onClick} tooltipOptions={{ title: "Select Point"}} - /> ); -export const PointButton = ({ onClick }: ISetNumberlineHandler) => { +export const PointButton = ({ onClick, pointTypeIsOpen }: ISetNumberlineHandler) => { + const pointTypeIsFilled = !pointTypeIsOpen; return ( } onClick={onClick} tooltipOptions={{ title: "Place Point"}} @@ -60,13 +60,10 @@ export const PointButton = ({ onClick }: ISetNumberlineHandler) => { ); }; - - -export const PointOpenButton = ({ onClick, selected }: ISetNumberlineHandler) => { - console.log("point open button with selected:", selected); +export const PointOpenButton = ({ onClick, pointTypeIsOpen }: ISetNumberlineHandler) => { return ( } onClick={onClick} tooltipOptions={{ title: "Place Open Point"}} diff --git a/src/plugins/numberline/components/numberline-toolbar.scss b/src/plugins/numberline/components/numberline-toolbar.scss index ef90ed028a..0a0dabd9c7 100644 --- a/src/plugins/numberline/components/numberline-toolbar.scss +++ b/src/plugins/numberline/components/numberline-toolbar.scss @@ -40,6 +40,12 @@ pointer-events: none; } + &.point-button{ + &.selected { + background-color: $workspace-teal-light-4; + } + } + &.point-open-button { &.selected { background-color: $workspace-teal-light-4; diff --git a/src/plugins/numberline/components/numberline-toolbar.tsx b/src/plugins/numberline/components/numberline-toolbar.tsx index 4c701212ce..f18cf51c38 100644 --- a/src/plugins/numberline/components/numberline-toolbar.tsx +++ b/src/plugins/numberline/components/numberline-toolbar.tsx @@ -1,5 +1,5 @@ import { observer } from "mobx-react"; -import React, { useState } from "react"; +import React from "react"; import ReactDOM from "react-dom"; import { IFloatingToolbarProps, useFloatingToolbarLocation @@ -14,18 +14,13 @@ const defaultButtons = ["select", "point", "point-open", "reset", "delete"]; interface INumberlineToolbarProps extends IFloatingToolbarProps { handleClearPoints: () => void; handleDeletePoint: () => void; + handleCreatePointType: (isOpen: boolean) => void; + pointTypeIsOpen: boolean } export const NumberlineToolbar: React.FC = observer((props) => { const { documentContent, tileElt, onIsEnabled, - handleClearPoints, handleDeletePoint, ...others } = props; - - //---------------- Determine Selected Point Type ------------------------ - const [selPointType, setSelPointType] = useState('filled'); // 'filled' or 'open' - const handleSelectPointType = (type: 'filled' | 'open') => { - setSelPointType(type); - - }; + handleDeletePoint, handleCreatePointType, pointTypeIsOpen, ...others } = props; console.log("-------< NumberlineToolbar >-----------"); const enabled = onIsEnabled(); @@ -38,26 +33,24 @@ export const NumberlineToolbar: React.FC = observer((pr ...others }); - const buttonSettings = useSettingFromStores("tools", "numberline") as unknown as string[] | undefined; const buttons = buttonSettings || defaultButtons; - const getToolbarButton = (toolName: string) => { switch (toolName) { case "select": return ; case "point": - return handleSelectPointType('filled')} - selected={selPointType === 'filled'} + onClick={() => handleCreatePointType(false)} + pointTypeIsOpen={pointTypeIsOpen} />; case "point-open": - return handleSelectPointType('open')} - selected={selPointType === 'open'} + onClick={() => handleCreatePointType(true)} + pointTypeIsOpen={pointTypeIsOpen} />; case "reset": return ; diff --git a/src/plugins/numberline/models/numberline-content.ts b/src/plugins/numberline/models/numberline-content.ts index 98422505fe..77422acf0b 100644 --- a/src/plugins/numberline/models/numberline-content.ts +++ b/src/plugins/numberline/models/numberline-content.ts @@ -14,6 +14,7 @@ export const PointObjectModel = types .model("PointObject", { id: types.identifier, xValue: 0, + isOpen: false }) .volatile(self => ({ dragXValue: undefined as undefined | number, @@ -32,7 +33,7 @@ export const PointObjectModel = types self.xValue = self.dragXValue; self.dragXValue = undefined; } - } + }, })); export interface PointObjectModelType extends Instance {} @@ -99,9 +100,9 @@ export const NumberlineContentModel = TileContentModel } })) .actions(self => ({ - createNewPoint(xValue: number) { + createNewPoint(xValue: number, isOpen = false) { const id = uniqueId(); - const pointModel = PointObjectModel.create({ id, xValue }); + const pointModel = PointObjectModel.create({ id, xValue, isOpen }); self.points.set(id, pointModel); return pointModel; }, @@ -118,13 +119,13 @@ export const NumberlineContentModel = TileContentModel } self.clearSelectedPoints(); }, - deleteAllPoints() { //TODO: This is not being used we may need to get rid of it + deleteAllPoints() { self.points.clear(); }, })) .actions(self => ({ - createAndSelectPoint(xValue: number) { - const newPoint = self.createNewPoint(xValue); + createAndSelectPoint(xValue: number, isOpen = false) { //TODO: we need to pass the "filled" or "open" + const newPoint = self.createNewPoint(xValue, isOpen); self.setSelectedPoint(newPoint); return newPoint; } From 021fa3c3ed0bddffa4f38bfb60e2da92555dd4af Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Thu, 29 Feb 2024 13:56:26 -0800 Subject: [PATCH 06/78] Push to pair with Boris, refactored numberline-tile toolbar Co-authored-by: Boris Goldowsky --- docs/unit-configuration.md | 4 +- src/clue/app-config.json | 9 + .../components/numberline-tile.scss | 8 +- .../numberline/components/numberline-tile.tsx | 114 ++++++----- .../components/numberline-toolbar-context.ts | 11 ++ .../numberline-toolbar-registration.tsx | 180 ++++++++++++++++++ .../components/numberline-toolbar.tsx | 5 +- 7 files changed, 273 insertions(+), 58 deletions(-) create mode 100644 src/plugins/numberline/components/numberline-toolbar-context.ts create mode 100644 src/plugins/numberline/components/numberline-toolbar-registration.tsx diff --git a/docs/unit-configuration.md b/docs/unit-configuration.md index f8c0107e22..3804122cf4 100644 --- a/docs/unit-configuration.md +++ b/docs/unit-configuration.md @@ -181,7 +181,9 @@ Not updated to common toolbar framework and does not support toolbar configurati #### Numberline -Not updated to common toolbar framework. However, supports toolbar configuration in a similar manner. Default buttons: `["place-point", "clear-points", "delete-points"]` +Uses common toolbar framework. +Default buttons: + - "place-point", "clear-points", "delete-points" #### Simulation diff --git a/src/clue/app-config.json b/src/clue/app-config.json index 95e3211ba3..ae57c89469 100644 --- a/src/clue/app-config.json +++ b/src/clue/app-config.json @@ -274,6 +274,15 @@ "toggle-lock" ] }, + "numberline": { + "tools": [ + "select", + "point", + "point-open", + "reset", + "delete" + ] + }, "simulator": { "defaultSimulation": "EMG_and_claw" }, diff --git a/src/plugins/numberline/components/numberline-tile.scss b/src/plugins/numberline/components/numberline-tile.scss index 6d00db4047..75ba16d258 100644 --- a/src/plugins/numberline/components/numberline-tile.scss +++ b/src/plugins/numberline/components/numberline-tile.scss @@ -61,15 +61,17 @@ } .point-inner-circle { - fill: #ccc; + fill: #0069ff; + // fill: red; opacity: 1; - stroke: #888; + stroke: black; stroke-width: 1.5px; &.selected { fill: $highlight-blue; - stroke: $highlight-blue; + // stroke: $ } + } } //end svg container diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index cf21928a5d..3a08bd82e9 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -5,7 +5,6 @@ import { observer } from 'mobx-react'; import { useUIStore } from '../../../hooks/use-stores'; import { kSmallAnnotationNodeRadius } from '../../../components/annotations/annotation-utilities'; import { BasicEditableTileTitle } from "../../../components/tiles/basic-editable-tile-title"; -import { useToolbarTileApi } from "../../../components/tiles/hooks/use-toolbar-tile-api"; import { ITileProps } from "../../../components/tiles/tile-component"; import { OffsetModel } from '../../../models/annotations/clue-object'; import { ITileExportOptions } from "../../../models/tiles/tile-content-info"; @@ -17,12 +16,16 @@ import { innerPointRadius, outerPointRadius, numberlineYBound, yMidPoint, kTitleHeight, kArrowheadTop, kArrowheadOffset, kPointButtonRadius, tickTextTopOffsetDefault, tickTextTopOffsetMinAndMax } from '../numberline-tile-constants'; -import { NumberlineToolbar } from "./numberline-toolbar"; import NumberlineArrowLeft from "../../../assets/numberline-arrow-left.svg"; import NumberlineArrowRight from "../../../assets/numberline-arrow-right.svg"; import { EditableNumberlineValue } from './editable-numberline-value'; +// import { TileToolbar } from 'src/components/toolbar/tile-toolbar'; +import { TileToolbar } from "../../../components/toolbar/tile-toolbar"; + +import "./numberline-toolbar-registration"; import "./numberline-tile.scss"; +import { INumberlineToolbarContext, NumberlineToolbarContext } from './numberline-toolbar-context'; //**********************✔️•↳******************* GUIDELINES ************************************************ // - As students, we want to express both closed form counting and inequalities which end with an open circle point. @@ -365,13 +368,18 @@ export const NumberlineTile: React.FC = observer(function Numberline .append("circle") .attr("class", "inner-point") .attr('cx', (p) => xScale(p.currentXValue)) //mapped to axis width - .attr('cy', yMidPoint).attr('r', innerPointRadius).attr('id', p => p.id) + .attr('cy', yMidPoint) + .attr('r', innerPointRadius) + .attr('id', p => p.id) .classed("point-inner-circle", true) .call((e) => handleDrag(e)); // Attach drag behavior to newly created circles // --- Update functions inner circles innerPoints .attr('cx', (p, idx) => xScale(p.currentXValue)) + // .attr('fill', pointTypeIsOpen ? '#ffffff' : "#0069ff") // added + // .attr('stroke', pointTypeIsOpen ? "#0069ff" : 'none') // added + // .attr('stroke-width', pointTypeIsOpen ? 2 : 0) // added .classed("selected", (p)=> p.id in content.selectedPoints) .call((e) => handleDrag(e)); // pass again in case axisWidth changes @@ -379,8 +387,16 @@ export const NumberlineTile: React.FC = observer(function Numberline }; updateCircles(); } - // Set up toolbar props - const toolbarProps = useToolbarTileApi({ id: model.id, enabled: !readOnly, onRegisterTileApi, onUnregisterTileApi }); + + // * ================================= [ Register Toolbar ] ================================== */ + + const toolbarFunctions: INumberlineToolbarContext = { + handleClearPoints: () => content.deleteAllPoints(), + handleDeletePoint: deleteSelectedPoints, + handleCreatePointType, + pointTypeIsOpen + }; + return (
= observer(function Numberline
- content.deleteAllPoints()} - handleDeletePoint={deleteSelectedPoints} - handleCreatePointType={handleCreatePointType} - pointTypeIsOpen={pointTypeIsOpen} - - /> -
-
- - - - - - handleMinMaxChange("min", newValue)} - /> - handleMinMaxChange("max", newValue)} - /> + + +
+
+ + + + + + handleMinMaxChange("min", newValue)} + /> + handleMinMaxChange("max", newValue)} + /> +
-
+
); }); diff --git a/src/plugins/numberline/components/numberline-toolbar-context.ts b/src/plugins/numberline/components/numberline-toolbar-context.ts new file mode 100644 index 0000000000..2f2fab8fb0 --- /dev/null +++ b/src/plugins/numberline/components/numberline-toolbar-context.ts @@ -0,0 +1,11 @@ +import { createContext } from "react"; + +export interface INumberlineToolbarContext { + handleClearPoints: () => void; + handleDeletePoint: () => void; + handleCreatePointType: (isOpen: boolean) => void; + pointTypeIsOpen: boolean; +} + +export const NumberlineToolbarContext = createContext(null); + diff --git a/src/plugins/numberline/components/numberline-toolbar-registration.tsx b/src/plugins/numberline/components/numberline-toolbar-registration.tsx new file mode 100644 index 0000000000..e51eb0dc26 --- /dev/null +++ b/src/plugins/numberline/components/numberline-toolbar-registration.tsx @@ -0,0 +1,180 @@ +import React, { useContext } from "react"; +import { IToolbarButtonComponentProps, registerTileToolbarButtons } + from "../../../components/toolbar/toolbar-button-manager"; +import { TileToolbarButton } from "../../../components/toolbar/tile-toolbar-button"; +import { NumberlineToolbarContext } from "./numberline-toolbar-context"; +import SelectIcon from "../assets/numberline-toolbar-select-tool.svg"; +import PointIcon from "../assets/numberline-toolbar-point-icon.svg"; +import PointOpenIcon from "../assets/numberline-toolbar-point-open-icon.svg"; +import ResetIcon from "../assets/numberline-toolbar-reset-icon.svg"; +import DeleteIcon from "../assets/numberline-toolbar-delete-icon.svg"; + +//TODO: delete numberline-toolbar, and numberline-toolbar-buttons.tsx +//TODO: update line 184 of unit-configuration.md + +const SelectButton = ({name}: IToolbarButtonComponentProps) => { + + function handleClick() { + console.log("select clicked"); + } + + return ( + + + + ); +}; + +const PointButton = ({name}: IToolbarButtonComponentProps) => { + + const context = useContext(NumberlineToolbarContext); + const selected = !context?.pointTypeIsOpen; + + function handleClick() { + if (context) { + context.handleCreatePointType(false); + } + } + + return ( + + + + + ); +}; + +const PointOpenButton = ({name}: IToolbarButtonComponentProps) => { + + const context = useContext(NumberlineToolbarContext); + const selected = context?.pointTypeIsOpen; + + function handleClick() { + if (context) { + context.handleCreatePointType(true); + } + } + + return ( + + + + ); +}; + + +// export interface INumberlineToolbarContext { +// handleClearPoints: () => void; +// handleDeletePoint: () => void; +// handleCreatePointType: (isOpen: boolean) => void; +// pointTypeIsOpen: boolean; +// } + +const ResetButton = ({name}: IToolbarButtonComponentProps) => { + + const context = useContext(NumberlineToolbarContext); + const handleClearPoints = context?.handleClearPoints; + + function handleClick() { + if (handleClearPoints) { + handleClearPoints(); + } + } + + return ( + + + + ); +}; + +const DeleteButton = ({name}: IToolbarButtonComponentProps) => { + + const context = useContext(NumberlineToolbarContext); + const handleDeletePoint = context?.handleDeletePoint; + + function handleClick() { + if (handleDeletePoint) { + handleDeletePoint(); + } + } + + return ( + + + + ); +}; + + + + +// export const ResetButton = ({ onClick }: ISetNumberlineHandler) => ( +// } +// onClick={onClick} +// tooltipOptions={{ title: "Reset"}} +// /> +// ); + + +// export const DeleteButton = ({ onClick }: ISetNumberlineHandler) => ( +// } +// onClick={onClick} +// tooltipOptions={{ title: "Delete Point(s)"}} +// /> +// ); + + + + +registerTileToolbarButtons("numberline", +[ + { + name: "select", + component: SelectButton + }, + { + name: "point", + component: PointButton + }, + { + name: "point-open", + component: PointOpenButton + }, + { + name: "reset", + component: ResetButton + }, + { + name: "delete", + component: DeleteButton + } +]); diff --git a/src/plugins/numberline/components/numberline-toolbar.tsx b/src/plugins/numberline/components/numberline-toolbar.tsx index f18cf51c38..934f890386 100644 --- a/src/plugins/numberline/components/numberline-toolbar.tsx +++ b/src/plugins/numberline/components/numberline-toolbar.tsx @@ -12,8 +12,8 @@ import "./numberline-toolbar.scss"; const defaultButtons = ["select", "point", "point-open", "reset", "delete"]; interface INumberlineToolbarProps extends IFloatingToolbarProps { - handleClearPoints: () => void; - handleDeletePoint: () => void; + handleClearPoints: () => void; //model - content.deleteAllPoints() + handleDeletePoint: () => void; //deleteSelectedPoints handleCreatePointType: (isOpen: boolean) => void; pointTypeIsOpen: boolean } @@ -22,7 +22,6 @@ export const NumberlineToolbar: React.FC = observer((pr const { documentContent, tileElt, onIsEnabled, handleDeletePoint, handleCreatePointType, pointTypeIsOpen, ...others } = props; - console.log("-------< NumberlineToolbar >-----------"); const enabled = onIsEnabled(); const location = useFloatingToolbarLocation({ documentContent, From ead46110078239fdb4287f23bddc65051ad746da Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Thu, 29 Feb 2024 14:10:49 -0800 Subject: [PATCH 07/78] Push up for deployment --- .../components/numberline-tile.scss | 4 +-- .../numberline/components/numberline-tile.tsx | 27 ++++++++++++++++--- .../numberline-toolbar-registration.tsx | 2 -- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/plugins/numberline/components/numberline-tile.scss b/src/plugins/numberline/components/numberline-tile.scss index 75ba16d258..5332b37869 100644 --- a/src/plugins/numberline/components/numberline-tile.scss +++ b/src/plugins/numberline/components/numberline-tile.scss @@ -68,8 +68,8 @@ stroke-width: 1.5px; &.selected { - fill: $highlight-blue; - // stroke: $ + fill: $highlight-blue; //0081ff + // fill: yellow; } } diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index 3a08bd82e9..7d33bd6bd6 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -33,11 +33,24 @@ import { INumberlineToolbarContext, NumberlineToolbarContext } from './numberlin // ✔️ add an open circle point to the toolbar // ✔️ add a select point arrow to the toolbar // ✔️ revise solid point button to the latest versions +// ↳ this is also on by default // ✔️ when the open circle point is used, create an open circle point // when the solid circle point is used, create a solid point like before // when the selection button is used, points can be moved or selected for deletion // point types are saved and restored and can be used in curriculum. + +//TODO: hover circle would be at drawMouseFollowPoint + +//TODO: delete numberline-toolbar, and numberline-toolbar-buttons.tsx +//TODO: update line 184 of unit-configuration.md + + +//**********************✔️•↳******************* END ************************************************ + + + + export const NumberlineTile: React.FC = observer(function NumberlineTile(props){ const { documentContent, model, readOnly, scale, tileElt, onRegisterTileApi, onUnregisterTileApi } = props; // console.log("-------------------"); @@ -228,6 +241,8 @@ export const NumberlineTile: React.FC = observer(function Numberline } }; + //PIN 1: HOVER AREA + function findHoverPoint(e: MouseEvent) { const [mouseX, mouseY] = mousePos(e); let hoverPoint: PointObjectModelType | undefined; @@ -363,6 +378,9 @@ export const NumberlineTile: React.FC = observer(function Numberline const innerPoints = svg.selectAll('.circle,.inner-point') .data(content.pointsArr); + const blue = "#0069ff"; + const white = "#ffffff"; + // Initialize Attributes innerPoints.enter() .append("circle") @@ -377,10 +395,11 @@ export const NumberlineTile: React.FC = observer(function Numberline // --- Update functions inner circles innerPoints .attr('cx', (p, idx) => xScale(p.currentXValue)) - // .attr('fill', pointTypeIsOpen ? '#ffffff' : "#0069ff") // added - // .attr('stroke', pointTypeIsOpen ? "#0069ff" : 'none') // added - // .attr('stroke-width', pointTypeIsOpen ? 2 : 0) // added - .classed("selected", (p)=> p.id in content.selectedPoints) + + .attr('fill', pointTypeIsOpen ? white : blue) // added + .attr('stroke', pointTypeIsOpen ? blue : 'none') // added + .attr('stroke-width', pointTypeIsOpen ? 2 : 0) // added + // .classed("selected", (p)=> p.id in content.selectedPoints) //TODO:may need to get rid of since not used .call((e) => handleDrag(e)); // pass again in case axisWidth changes innerPoints.exit().remove(); //cleanup diff --git a/src/plugins/numberline/components/numberline-toolbar-registration.tsx b/src/plugins/numberline/components/numberline-toolbar-registration.tsx index e51eb0dc26..74a4bae0e4 100644 --- a/src/plugins/numberline/components/numberline-toolbar-registration.tsx +++ b/src/plugins/numberline/components/numberline-toolbar-registration.tsx @@ -9,8 +9,6 @@ import PointOpenIcon from "../assets/numberline-toolbar-point-open-icon.svg"; import ResetIcon from "../assets/numberline-toolbar-reset-icon.svg"; import DeleteIcon from "../assets/numberline-toolbar-delete-icon.svg"; -//TODO: delete numberline-toolbar, and numberline-toolbar-buttons.tsx -//TODO: update line 184 of unit-configuration.md const SelectButton = ({name}: IToolbarButtonComponentProps) => { From 6c309155a247b0211ac417624c427b2af2be806e Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Fri, 1 Mar 2024 16:37:58 -0800 Subject: [PATCH 08/78] Working demo of blue/white circles along with blue filled circles --- .../components/numberline-tile.scss | 12 +-- .../numberline/components/numberline-tile.tsx | 78 +++++++++++++------ .../numberline/models/numberline-content.ts | 4 +- 3 files changed, 65 insertions(+), 29 deletions(-) diff --git a/src/plugins/numberline/components/numberline-tile.scss b/src/plugins/numberline/components/numberline-tile.scss index 5332b37869..3a17da38c9 100644 --- a/src/plugins/numberline/components/numberline-tile.scss +++ b/src/plugins/numberline/components/numberline-tile.scss @@ -62,17 +62,19 @@ .point-inner-circle { fill: #0069ff; - // fill: red; opacity: 1; stroke: black; stroke-width: 1.5px; + } - &.selected { - fill: $highlight-blue; //0081ff - // fill: yellow; - } + .point-inner-circle-open { + fill: black; + opacity: 1; + stroke: black; + stroke-width: 1.5px; } + } //end svg container .arrow { diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index 7d33bd6bd6..9071d584b8 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -65,21 +65,22 @@ export const NumberlineTile: React.FC = observer(function Numberline const [pointTypeIsOpen, setPointTypeIsOpen] = useState(false); //default start with filled in point const handleCreatePointType = (_isOpen: boolean) => { - console.log("--------------------------------------"); - console.log("initial STATE:", pointTypeIsOpen); + // console.log("--------------------------------------"); + // console.log("initial STATE:", pointTypeIsOpen); setPointTypeIsOpen(_isOpen); }; useEffect(() => { - console.log("Updated STATE:", pointTypeIsOpen); + // console.log("Updated STATE:", pointTypeIsOpen); }, [pointTypeIsOpen]); /* ============================ [ Model Manipulation Functions ] ============================ */ const createPoint = (xValue: number, _pointTypeIsOpen: boolean) => { const pointType = (_pointTypeIsOpen) ? "OPEN" : "FILLED"; - console.log("createPoint with xValue", xValue, "pointTypeisOpen:", pointType); + // console.log("createPoint with xValue", xValue, "pointTypeisOpen:", pointType); if (!readOnly) { - const point = content.createAndSelectPoint(xValue); + const point = content.createAndSelectPoint(xValue, _pointTypeIsOpen); + console.log("pointCreated:", point.id, "-----", pointType); setHoverPointId(point.id); } }; @@ -241,8 +242,6 @@ export const NumberlineTile: React.FC = observer(function Numberline } }; - //PIN 1: HOVER AREA - function findHoverPoint(e: MouseEvent) { const [mouseX, mouseY] = mousePos(e); let hoverPoint: PointObjectModelType | undefined; @@ -259,12 +258,31 @@ export const NumberlineTile: React.FC = observer(function Numberline } const drawMouseFollowPoint = (mouseX: number) => { - svg.append("circle") //create a circle that follows the mouse + //when user's mouse hovers over numberline, create a circle that follows the mouse + const followPoint = svg.append("circle") .attr("cx", mouseX) .attr("cy", yMidPoint) .attr("r", innerPointRadius) .classed("mouse-follow-point", true) - .classed("point-inner-circle", true); + .classed("point-inner-circle", !pointTypeIsOpen); //filled + + /* =========================== [ Blue / White Circles ] ============================= */ + if(pointTypeIsOpen) { + followPoint //recreate outer blue circle + .attr("fill", "#0069ff") + .attr("stroke", "black") + .attr("opacity", 1) + .attr("stroke-width", 1.5); + //create + svg.append("circle") + .attr("fill", "white") + .attr("cx", mouseX) + .attr("cy", yMidPoint) + .attr("r", innerPointRadius * 0.5) + .attr("opacity", 1) + .classed("mouse-follow-point", true); + + } }; const clearMouseFollowPoint = () => svg.selectAll(".mouse-follow-point").remove(); @@ -354,13 +372,17 @@ export const NumberlineTile: React.FC = observer(function Numberline const updateCircles = () => { /* =========================== [ Outer Hover Circles ] ======================= */ + //---- Initialize outer hover circles const outerPoints = svg.selectAll('.circle,.outer-point') .data(content.pointsArr); outerPoints.enter() .append("circle").attr("class", "outer-point") - .attr('cx', (p) => xScale(p.currentXValue)) //mapped to axis width + .attr('cx', (p) => { + // console.log("line 364:", p); + return xScale(p.currentXValue); + }) //mapped to axis width .attr('cy', yMidPoint).attr('r', outerPointRadius).attr('id', p => p.id) .classed("point-outer-circle", true) .call((e) => handleDrag(e)); // Attach drag behavior to newly created circles @@ -373,14 +395,13 @@ export const NumberlineTile: React.FC = observer(function Numberline outerPoints.exit().remove(); //cleanup + //TODO: Revise inner circles back to original + // create an innerPointsOpen that is white and append it to svg + /* =========================== [ Inner Circles ] ============================= */ - //---- Initialize inner hover circles const innerPoints = svg.selectAll('.circle,.inner-point') .data(content.pointsArr); - const blue = "#0069ff"; - const white = "#ffffff"; - // Initialize Attributes innerPoints.enter() .append("circle") @@ -389,21 +410,34 @@ export const NumberlineTile: React.FC = observer(function Numberline .attr('cy', yMidPoint) .attr('r', innerPointRadius) .attr('id', p => p.id) - .classed("point-inner-circle", true) + .classed("point-inner-circle", true) //may change .call((e) => handleDrag(e)); // Attach drag behavior to newly created circles // --- Update functions inner circles innerPoints - .attr('cx', (p, idx) => xScale(p.currentXValue)) + .attr('cx', (p, idx) => xScale(p.currentXValue)) + .classed("selected", (p)=> p.id in content.selectedPoints) + .call((e) => handleDrag(e)); // pass again in case axisWidth changes + + innerPoints.exit().remove(); + + + /* =========================== [ Blue White Circles] ============================= */ + content.pointsArr.forEach(p => { + if(p.isOpen) { + svg.append("circle") + .attr("class", "inner-white-point") + .attr("cx", xScale(p.currentXValue)) + .attr("cy", yMidPoint) + .attr("r", innerPointRadius * 0.5) + .attr("fill", "white") + .attr('id', `inner-white-${p.id}`); + } + }); - .attr('fill', pointTypeIsOpen ? white : blue) // added - .attr('stroke', pointTypeIsOpen ? blue : 'none') // added - .attr('stroke-width', pointTypeIsOpen ? 2 : 0) // added - // .classed("selected", (p)=> p.id in content.selectedPoints) //TODO:may need to get rid of since not used - .call((e) => handleDrag(e)); // pass again in case axisWidth changes - innerPoints.exit().remove(); //cleanup }; + updateCircles(); } diff --git a/src/plugins/numberline/models/numberline-content.ts b/src/plugins/numberline/models/numberline-content.ts index 77422acf0b..2e635e05d7 100644 --- a/src/plugins/numberline/models/numberline-content.ts +++ b/src/plugins/numberline/models/numberline-content.ts @@ -100,7 +100,7 @@ export const NumberlineContentModel = TileContentModel } })) .actions(self => ({ - createNewPoint(xValue: number, isOpen = false) { + createNewPoint(xValue: number, isOpen: boolean) { const id = uniqueId(); const pointModel = PointObjectModel.create({ id, xValue, isOpen }); self.points.set(id, pointModel); @@ -124,7 +124,7 @@ export const NumberlineContentModel = TileContentModel }, })) .actions(self => ({ - createAndSelectPoint(xValue: number, isOpen = false) { //TODO: we need to pass the "filled" or "open" + createAndSelectPoint(xValue: number, isOpen: boolean) { //TODO: we need to pass the "filled" or "open" const newPoint = self.createNewPoint(xValue, isOpen); self.setSelectedPoint(newPoint); return newPoint; From ce6f6894e18fc92f4a6493159625aa9365801feb Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Sat, 2 Mar 2024 16:28:15 -0500 Subject: [PATCH 09/78] setting up to create exemplar docs - surfacing exemplars from problem snapshot - stubbing out loading into document store - question about potential problem model volatile property `exemplars` --- src/models/curriculum/problem.ts | 18 ++++++++++++++---- src/models/stores/create-exemplar-docs.ts | 15 +++++++++++++++ src/models/stores/stores.ts | 2 ++ 3 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 src/models/stores/create-exemplar-docs.ts diff --git a/src/models/curriculum/problem.ts b/src/models/curriculum/problem.ts index d7bfa9f7db..65b432b31c 100644 --- a/src/models/curriculum/problem.ts +++ b/src/models/curriculum/problem.ts @@ -30,10 +30,12 @@ const ModernProblemModel = types * with all of the other sections in this problem, or this problem's unit */ sectionsFromSnapshot: types.frozen(), + exemplarsFromSnapshot: types.frozen(), config: types.maybe(types.frozen>()) }) .volatile(self => ({ - sections: observable.array() as SectionModelType[] + sections: observable.array() as SectionModelType[], + // exemplars: observable.array() as any[] // HMM: I may need this later? })) .views(self => ({ get fullTitle() { @@ -121,22 +123,30 @@ const isAmbiguousSnapshot = (sn: ModernProblemSnapshot | LegacyProblemSnapshot) export const ProblemModel = types.snapshotProcessor(ModernProblemModel, { preProcessor(sn: ModernProblemSnapshot | LegacyProblemSnapshot) { - const { sections, ...nonSectionProps } = sn as any; + const { sections, exemplars, ...nonSectionProps } = sn as any; // Move sections to sectionsFromSnapshot so we load them into a volatile property `sections` + // creating a exemplarsFromSnapshot that will be passed in similar way, + // but no volatile property yet const sectionsFromSnapshot = sections || []; + const exemplarsFromSnapshot = exemplars || []; if (isLegacySnapshot(sn)) { const { disabled: disabledFeatures, settings, ...others } = sn; - return { ...others, sectionsFromSnapshot, config: { disabledFeatures, settings } } as ModernProblemSnapshot; + return { ...others, + sectionsFromSnapshot, + exemplarsFromSnapshot, // HMM: I don't think this ever will exist + config: { disabledFeatures, settings } + } as ModernProblemSnapshot; } if (isAmbiguousSnapshot(sn)) { const { disabled: disabledFeatures, settings, config, ...others } = sn as any; return { ...others, sectionsFromSnapshot, + exemplarsFromSnapshot, config: { disabledFeatures, settings, ...config } } as ModernProblemSnapshot; } - return { ...nonSectionProps, sectionsFromSnapshot }; + return { ...nonSectionProps, sectionsFromSnapshot, exemplarsFromSnapshot }; } }); export interface ProblemModelType extends Instance {} diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts new file mode 100644 index 0000000000..4a0cc9f3c1 --- /dev/null +++ b/src/models/stores/create-exemplar-docs.ts @@ -0,0 +1,15 @@ +/* + Parameters for development: + + &demoName=Joe2 + &fakeClass=1 + &fakeUser=student:3 + &problem=1.1 + &unit=mothed + &curriculumBranch=exemplar-3 +*/ + +export function createExemplarDocs(problem: any, documentStore: any){ + const { exemplarsFromSnapshot: exemplarUrls } = problem; + console.log("| exemplar urls to load and turn into documents in store | ", exemplarUrls); +} diff --git a/src/models/stores/stores.ts b/src/models/stores/stores.ts index c4cf13c8fd..a5e0dc0627 100644 --- a/src/models/stores/stores.ts +++ b/src/models/stores/stores.ts @@ -32,6 +32,7 @@ import { CurriculumConfig, ICurriculumConfig } from "./curriculum-config"; import { urlParams } from "../../utilities/url-params"; import curriculumConfigJson from "../../clue/curriculum-config.json"; import { gImageMap } from "../image-map"; +import { createExemplarDocs } from "./create-exemplar-docs"; export interface IStores extends IBaseStores { problemPath: string; @@ -254,6 +255,7 @@ class Stores implements IStores{ if (problem && unitUrls) { problem.loadSections(unitUrls.content); + createExemplarDocs(problem, this.documents); } // We are changing our observable state here so we need to be in an action. From 0b54d4e836729efbb91ecc294ea69a062401c1a9 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Sat, 2 Mar 2024 18:02:45 -0500 Subject: [PATCH 10/78] fetch exemplar data, simplify exemplar path type --- src/models/curriculum/problem.ts | 10 +++--- src/models/stores/create-exemplar-docs.ts | 38 ++++++++++++++--------- src/models/stores/stores.ts | 6 +++- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/models/curriculum/problem.ts b/src/models/curriculum/problem.ts index 65b432b31c..4c96f75908 100644 --- a/src/models/curriculum/problem.ts +++ b/src/models/curriculum/problem.ts @@ -30,7 +30,7 @@ const ModernProblemModel = types * with all of the other sections in this problem, or this problem's unit */ sectionsFromSnapshot: types.frozen(), - exemplarsFromSnapshot: types.frozen(), + exemplarPaths: types.array(types.string), // HMM: is above ever not string[]? config: types.maybe(types.frozen>()) }) .volatile(self => ({ @@ -128,12 +128,12 @@ export const ProblemModel = types.snapshotProcessor(ModernProblemModel, { // creating a exemplarsFromSnapshot that will be passed in similar way, // but no volatile property yet const sectionsFromSnapshot = sections || []; - const exemplarsFromSnapshot = exemplars || []; + const exemplarPaths = exemplars || []; if (isLegacySnapshot(sn)) { const { disabled: disabledFeatures, settings, ...others } = sn; return { ...others, sectionsFromSnapshot, - exemplarsFromSnapshot, // HMM: I don't think this ever will exist + exemplarPaths, config: { disabledFeatures, settings } } as ModernProblemSnapshot; } @@ -142,11 +142,11 @@ export const ProblemModel = types.snapshotProcessor(ModernProblemModel, { return { ...others, sectionsFromSnapshot, - exemplarsFromSnapshot, + exemplarPaths, config: { disabledFeatures, settings, ...config } } as ModernProblemSnapshot; } - return { ...nonSectionProps, sectionsFromSnapshot, exemplarsFromSnapshot }; + return { ...nonSectionProps, sectionsFromSnapshot, exemplarPaths }; } }); export interface ProblemModelType extends Instance {} diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index 4a0cc9f3c1..490867a64c 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -1,15 +1,25 @@ -/* - Parameters for development: - - &demoName=Joe2 - &fakeClass=1 - &fakeUser=student:3 - &problem=1.1 - &unit=mothed - &curriculumBranch=exemplar-3 -*/ - -export function createExemplarDocs(problem: any, documentStore: any){ - const { exemplarsFromSnapshot: exemplarUrls } = problem; - console.log("| exemplar urls to load and turn into documents in store | ", exemplarUrls); +import { ProblemModelType } from "../curriculum/problem"; +import { DocumentsModelType } from "./documents"; + +interface ICreateExemplarDocsParams { + unitContentUrl: string; + problem: ProblemModelType; + docsStore: DocumentsModelType; +} + +export async function createExemplarDocs({ unitContentUrl, problem, docsStore }: ICreateExemplarDocsParams) { + const { exemplarPaths } = problem; + const exemplarsData = await fetchExemplars(unitContentUrl, exemplarPaths); + console.log("| create and load in exemplar docs from this data: ", exemplarsData); } + +export async function fetchExemplars(unitContentUrl: string, exemplarUrls: string[]){ + return await Promise.all( + exemplarUrls.map(async (url: string) => { + const fetchUrl = new URL(url, unitContentUrl).href; + const response = await fetch(fetchUrl); + return await response.json(); + }) + ); +} + diff --git a/src/models/stores/stores.ts b/src/models/stores/stores.ts index a5e0dc0627..d321d071fa 100644 --- a/src/models/stores/stores.ts +++ b/src/models/stores/stores.ts @@ -255,7 +255,11 @@ class Stores implements IStores{ if (problem && unitUrls) { problem.loadSections(unitUrls.content); - createExemplarDocs(problem, this.documents); + createExemplarDocs({ + unitContentUrl: unitUrls.content, + problem, + docsStore: this.documents + }); } // We are changing our observable state here so we need to be in an action. From ee08fda4e61c77bed095238ef38ae199579136ee Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Sun, 3 Mar 2024 22:34:34 -0500 Subject: [PATCH 11/78] wip --- .../db-listeners/db-other-docs-listener.ts | 1 + src/models/stores/create-exemplar-docs.ts | 53 +++++++++++++++++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/lib/db-listeners/db-other-docs-listener.ts b/src/lib/db-listeners/db-other-docs-listener.ts index 3039cc41df..bdbdba5bb2 100644 --- a/src/lib/db-listeners/db-other-docs-listener.ts +++ b/src/lib/db-listeners/db-other-docs-listener.ts @@ -69,6 +69,7 @@ export class DBOtherDocumentsListener extends BaseListener { const dbDoc: DBOtherDocument|null = snapshot.val(); this.debugLogSnapshot("#handleDocumentAdded", snapshot); if (dbDoc) { + console.log("| createDocumentModelFromOtherDocument", dbDoc, this.documentType); this.db.createDocumentModelFromOtherDocument(dbDoc, this.documentType) .then(doc => { if (doc.uid === user.id) { diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index 490867a64c..f166733def 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -1,3 +1,4 @@ +import { useStores } from "../../hooks/use-stores"; import { ProblemModelType } from "../curriculum/problem"; import { DocumentsModelType } from "./documents"; @@ -9,17 +10,61 @@ interface ICreateExemplarDocsParams { export async function createExemplarDocs({ unitContentUrl, problem, docsStore }: ICreateExemplarDocsParams) { const { exemplarPaths } = problem; - const exemplarsData = await fetchExemplars(unitContentUrl, exemplarPaths); - console.log("| create and load in exemplar docs from this data: ", exemplarsData); + const exemplarsData = await getExemplarsData(unitContentUrl, exemplarPaths); + addExemplarDocsToStore(docsStore, exemplarsData); } -export async function fetchExemplars(unitContentUrl: string, exemplarUrls: string[]){ +export async function getExemplarsData(unitContentUrl: string, exemplarUrls: string[]){ return await Promise.all( exemplarUrls.map(async (url: string) => { const fetchUrl = new URL(url, unitContentUrl).href; const response = await fetch(fetchUrl); - return await response.json(); + const data = await response.json(); + // HMM: I know I am supposed to retain this info for id generation later? + const urlSegments = fetchUrl.split("/"); + return { + url: fetchUrl, + curriculumBranch: urlSegments[5], + unit: urlSegments[6], + dirName: urlSegments[7], + investigationSlug: urlSegments[8], + problemSlug: urlSegments[9], + exemplarSlug: urlSegments[10], + ...data + }; //HMM it was fun to get these segments, but maybe I should just swap/escape stuff }) ); } +export function useExemplar(docsStore: DocumentsModelType, exemplarsData: any) { + const { problem, documents, unit, user, groups } = useStores(); + exemplarsData.forEach((exemplarData: any) => { + // create a document + const exemplarDocId = createExemplarDocId(exemplarData); + const group = groups.groupForUser(uid); + const groupId = group && group.id; + const newDocParams = { + groupId: 1, // HEY: we will need get the current user's group id + title: exemplarData.exemplarSlug, // HEY: we are going to need to have gotten the title + uid: 999, // HEY: we will need IvanIdeas user id? + type: "personal", // HEY this is for now, will soon be "exemplar" + visibility: "public", + properties: { key: exemplarDocId} // HEY: this is not right, but lets see if it works + }; + console.log("| make a doc with these params", newDocParams); + // THEN: ? ...make sure it is exposed in sort view + }); +} + +function createExemplarDocId(exemplarData: any) { + // HMM: I have yet to understand how this is going to be used so this is dummy function + const { + curriculumBranch, + unit, + dirName, + investigationSlug, + problemSlug, + exemplarSlug + } = exemplarData; + return `${curriculumBranch}-${unit}-${dirName}-${investigationSlug}-${problemSlug}-${exemplarSlug}`; +} From e332a5aa131f0b6b063c1ab9c37f91ad5940d48d Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Sun, 3 Mar 2024 23:34:55 -0500 Subject: [PATCH 12/78] setting up --- src/models/stores/create-exemplar-docs.ts | 44 +++++++++++------------ src/models/stores/stores.ts | 9 ++--- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index f166733def..e6cc603de5 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -1,28 +1,29 @@ -import { useStores } from "../../hooks/use-stores"; import { ProblemModelType } from "../curriculum/problem"; import { DocumentsModelType } from "./documents"; +import { UserModelType } from "./user"; interface ICreateExemplarDocsParams { - unitContentUrl: string; + unitUrl: string; problem: ProblemModelType; - docsStore: DocumentsModelType; + documents: DocumentsModelType; + user: UserModelType; } -export async function createExemplarDocs({ unitContentUrl, problem, docsStore }: ICreateExemplarDocsParams) { +export async function createAndLoadExemplarDocs({ unitUrl, problem, documents, user }: ICreateExemplarDocsParams) { const { exemplarPaths } = problem; - const exemplarsData = await getExemplarsData(unitContentUrl, exemplarPaths); - addExemplarDocsToStore(docsStore, exemplarsData); + const exemplarsData = await getExemplarsData(unitUrl, exemplarPaths); + createExemplarDocs(documents, user, exemplarsData); } -export async function getExemplarsData(unitContentUrl: string, exemplarUrls: string[]){ +export async function getExemplarsData(unitUrl: string, exemplarUrls: string[]){ return await Promise.all( exemplarUrls.map(async (url: string) => { - const fetchUrl = new URL(url, unitContentUrl).href; + const fetchUrl = new URL(url, unitUrl).href; const response = await fetch(fetchUrl); const data = await response.json(); - // HMM: I know I am supposed to retain this info for id generation later? const urlSegments = fetchUrl.split("/"); return { + ...data, url: fetchUrl, curriculumBranch: urlSegments[5], unit: urlSegments[6], @@ -30,34 +31,29 @@ export async function getExemplarsData(unitContentUrl: string, exemplarUrls: str investigationSlug: urlSegments[8], problemSlug: urlSegments[9], exemplarSlug: urlSegments[10], - ...data - }; //HMM it was fun to get these segments, but maybe I should just swap/escape stuff + }; }) ); } -export function useExemplar(docsStore: DocumentsModelType, exemplarsData: any) { - const { problem, documents, unit, user, groups } = useStores(); +function createExemplarDocs(documents: DocumentsModelType, user: UserModelType, exemplarsData: any) { exemplarsData.forEach((exemplarData: any) => { - // create a document const exemplarDocId = createExemplarDocId(exemplarData); - const group = groups.groupForUser(uid); - const groupId = group && group.id; const newDocParams = { - groupId: 1, // HEY: we will need get the current user's group id - title: exemplarData.exemplarSlug, // HEY: we are going to need to have gotten the title - uid: 999, // HEY: we will need IvanIdeas user id? - type: "personal", // HEY this is for now, will soon be "exemplar" + title: exemplarData.title, + uid: 'ivan_idea_1', + type: "personal", visibility: "public", - properties: { key: exemplarDocId} // HEY: this is not right, but lets see if it works + content: exemplarData.content, + docId: exemplarDocId, }; - console.log("| make a doc with these params", newDocParams); - // THEN: ? ...make sure it is exposed in sort view + console.log("| make a doc from the data |", newDocParams); + // NEXT: try one of the many apparent ways to make the doc }); } function createExemplarDocId(exemplarData: any) { - // HMM: I have yet to understand how this is going to be used so this is dummy function + // QUESTION: I have yet to understand how this is going to be used so this is dummy function const { curriculumBranch, unit, diff --git a/src/models/stores/stores.ts b/src/models/stores/stores.ts index d321d071fa..7e0685ee1f 100644 --- a/src/models/stores/stores.ts +++ b/src/models/stores/stores.ts @@ -30,9 +30,9 @@ import { removeLoadingMessage, showLoadingMessage } from "../../utilities/loadin import { problemLoaded } from "../../lib/misc"; import { CurriculumConfig, ICurriculumConfig } from "./curriculum-config"; import { urlParams } from "../../utilities/url-params"; +import { createAndLoadExemplarDocs } from "./create-exemplar-docs"; import curriculumConfigJson from "../../clue/curriculum-config.json"; import { gImageMap } from "../image-map"; -import { createExemplarDocs } from "./create-exemplar-docs"; export interface IStores extends IBaseStores { problemPath: string; @@ -255,10 +255,11 @@ class Stores implements IStores{ if (problem && unitUrls) { problem.loadSections(unitUrls.content); - createExemplarDocs({ - unitContentUrl: unitUrls.content, + createAndLoadExemplarDocs({ + unitUrl: unitUrls.content, problem, - docsStore: this.documents + documents: this.documents, + user: this.user, }); } From 536a1c83e132952934eae73f262ef4d1b114bb3f Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Mon, 4 Mar 2024 09:13:42 -0500 Subject: [PATCH 13/78] proof-of-concept: add the exemplar as a personal doc and see it in sort view --- .../db-listeners/db-other-docs-listener.ts | 1 - src/models/stores/create-exemplar-docs.ts | 37 ++++++++++++------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/lib/db-listeners/db-other-docs-listener.ts b/src/lib/db-listeners/db-other-docs-listener.ts index bdbdba5bb2..3039cc41df 100644 --- a/src/lib/db-listeners/db-other-docs-listener.ts +++ b/src/lib/db-listeners/db-other-docs-listener.ts @@ -69,7 +69,6 @@ export class DBOtherDocumentsListener extends BaseListener { const dbDoc: DBOtherDocument|null = snapshot.val(); this.debugLogSnapshot("#handleDocumentAdded", snapshot); if (dbDoc) { - console.log("| createDocumentModelFromOtherDocument", dbDoc, this.documentType); this.db.createDocumentModelFromOtherDocument(dbDoc, this.documentType) .then(doc => { if (doc.uid === user.id) { diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index e6cc603de5..89a114290a 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -1,5 +1,6 @@ import { ProblemModelType } from "../curriculum/problem"; import { DocumentsModelType } from "./documents"; +import { DocumentModel } from "../document/document"; import { UserModelType } from "./user"; interface ICreateExemplarDocsParams { @@ -36,31 +37,39 @@ export async function getExemplarsData(unitUrl: string, exemplarUrls: string[]){ ); } +function createExemplarDocId(exemplarData: any) { + // QUESTION: I have yet to understand how this is going to be used so this is dummy function + const { + curriculumBranch, + unit, + dirName, + investigationSlug, + problemSlug, + exemplarSlug + } = exemplarData; + return `${curriculumBranch}-${unit}-${dirName}-${investigationSlug}-${problemSlug}-${exemplarSlug}`; +} + function createExemplarDocs(documents: DocumentsModelType, user: UserModelType, exemplarsData: any) { exemplarsData.forEach((exemplarData: any) => { const exemplarDocId = createExemplarDocId(exemplarData); + // QUESTION: I see "title" some places? And properties.caption others? Using both for now. const newDocParams = { title: exemplarData.title, uid: 'ivan_idea_1', type: "personal", visibility: "public", content: exemplarData.content, - docId: exemplarDocId, + key: exemplarDocId, + properties: { + caption: exemplarData.title + } }; - console.log("| make a doc from the data |", newDocParams); - // NEXT: try one of the many apparent ways to make the doc + makeDocFromData(newDocParams, documents); }); } -function createExemplarDocId(exemplarData: any) { - // QUESTION: I have yet to understand how this is going to be used so this is dummy function - const { - curriculumBranch, - unit, - dirName, - investigationSlug, - problemSlug, - exemplarSlug - } = exemplarData; - return `${curriculumBranch}-${unit}-${dirName}-${investigationSlug}-${problemSlug}-${exemplarSlug}`; +function makeDocFromData(newDocParams: any, documents: DocumentsModelType) { + const newDoc = DocumentModel.create(newDocParams); + documents.add(newDoc); } From 4cf610285ed900b8a15a379b7861158d6077bfa2 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Mon, 4 Mar 2024 10:53:29 -0500 Subject: [PATCH 14/78] patch in "Idea, Ivan", but he should probably be a real user --- src/models/stores/sorted-documents.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index a00dba3166..9bfb052ae0 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -106,7 +106,7 @@ export class SortedDocuments { const documentMap = new Map(); this.filteredDocsByType.forEach((doc) => { const user = this.class.getUserById(doc.uid); - const sectionLabel = user && `${user.lastName}, ${user.firstName}`; + const sectionLabel = user ? `${user.lastName}, ${user.firstName}` : "Idea, Ivan"; if (!documentMap.has(sectionLabel)) { documentMap.set(sectionLabel, { sectionLabel, From 0b7703d63530075d435fc3036dd525b4d20f6bec Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Mon, 4 Mar 2024 14:27:07 -0500 Subject: [PATCH 15/78] add exemplar user - add `addSingleUser` method to class.ts -call it before loading exemplars -pass class store to exemplar loading - define a constant for exemplar user "Ivan Idea" --- src/models/stores/class.ts | 3 +++ src/models/stores/create-exemplar-docs.ts | 12 +++++++++++- src/models/stores/stores.ts | 1 + src/models/stores/user-types.ts | 9 +++++++++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/models/stores/class.ts b/src/models/stores/class.ts index b26078bf13..3c125967ed 100644 --- a/src/models/stores/class.ts +++ b/src/models/stores/class.ts @@ -43,6 +43,9 @@ export const ClassModel = types initials: user.initials, })); }); + }, + addSingleUser(user: any) { + self.users.put(user); } }; }) diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index 89a114290a..cda90b908e 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -2,17 +2,27 @@ import { ProblemModelType } from "../curriculum/problem"; import { DocumentsModelType } from "./documents"; import { DocumentModel } from "../document/document"; import { UserModelType } from "./user"; +import { ClassModelType, ClassUserModel } from "./class"; +import { kExemplarUserParams } from "./user-types"; interface ICreateExemplarDocsParams { unitUrl: string; problem: ProblemModelType; documents: DocumentsModelType; + classStore: ClassModelType; user: UserModelType; } -export async function createAndLoadExemplarDocs({ unitUrl, problem, documents, user }: ICreateExemplarDocsParams) { +export async function createAndLoadExemplarDocs({ + unitUrl, + problem, + documents, + classStore, + user +}: ICreateExemplarDocsParams) { const { exemplarPaths } = problem; const exemplarsData = await getExemplarsData(unitUrl, exemplarPaths); + classStore.addSingleUser(ClassUserModel.create(kExemplarUserParams)); createExemplarDocs(documents, user, exemplarsData); } diff --git a/src/models/stores/stores.ts b/src/models/stores/stores.ts index 62204357b5..1e69c192f4 100644 --- a/src/models/stores/stores.ts +++ b/src/models/stores/stores.ts @@ -261,6 +261,7 @@ class Stores implements IStores{ problem, documents: this.documents, user: this.user, + classStore: this.class, }); } diff --git a/src/models/stores/user-types.ts b/src/models/stores/user-types.ts index fd3a43fd8b..136c0b50e4 100644 --- a/src/models/stores/user-types.ts +++ b/src/models/stores/user-types.ts @@ -5,3 +5,12 @@ export type UserType = Instance; export const DisplayUserTypeEnum = types.maybe(UserTypeEnum); export type DisplayUserType = Instance; + +export const kExemplarUserParams = { + type: "student", + id: "ivan_idea_1", + firstName: "Ivan", + lastName: "Idea", + fullName: "Ivan Idea", + initials: "II", +}; From 420db9ea2e649d76ae574de87e0a01dc28733d1b Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Mon, 4 Mar 2024 17:06:07 -0500 Subject: [PATCH 16/78] update exemplar loading - rename curriculumBaseUrl to curriculumSiteUrl - define curriculumBaseUrl property - define a test for generating exemplar id - define ExemplarDocument type - change name of addSingleUser to addUser - improve encoding of exemplar id --- README.md | 4 +- src/clue/curriculum-config.json | 2 +- src/lib/auth.test.ts | 2 +- src/lib/portal-api.test.ts | 2 +- src/models/document/document-types.ts | 5 +- src/models/stores/class.ts | 2 +- .../stores/create-exemplar-docs.test.ts | 17 +++++ src/models/stores/create-exemplar-docs.ts | 73 ++++++++++--------- src/models/stores/curriculum-config.test.ts | 2 +- src/models/stores/curriculum-config.ts | 13 +++- src/models/stores/stores.test.ts | 2 +- src/models/stores/stores.ts | 1 + 12 files changed, 76 insertions(+), 49 deletions(-) create mode 100644 src/models/stores/create-exemplar-docs.test.ts diff --git a/README.md b/README.md index ca253ba007..df03f3ab4c 100644 --- a/README.md +++ b/README.md @@ -223,8 +223,8 @@ There are a number of URL parameters that can aid in testing: The `unit` parameter can be in 3 forms: - a valid URL starting with `https:` or `http:` will be treated as an absolute URL. - a string starting with `./` will be treated as a URL relative to the current page in the browser. -- Everything else is treated as a unit code, these codes are first looked up in a map to remap legacy codes. Then the URL of the unit is created by `${curriculumBaseUrl}/branch/${branchName}/${unitCode}/content.json`. - - `curriculumBaseUrl` defaults to `https://models-resources.concord.org/clue-curriculum`. +- Everything else is treated as a unit code, these codes are first looked up in a map to remap legacy codes. Then the URL of the unit is created by `${curriculumSiteUrl}/branch/${branchName}/${unitCode}/content.json`. + - `curriculumSiteUrl` defaults to `https://models-resources.concord.org/clue-curriculum`. - `branchName` defaults to `main`. - To find out more about customizing these values look at `app-config-model.ts`. diff --git a/src/clue/curriculum-config.json b/src/clue/curriculum-config.json index bdd773aa2c..ed32a76967 100644 --- a/src/clue/curriculum-config.json +++ b/src/clue/curriculum-config.json @@ -1,5 +1,5 @@ { - "curriculumBaseUrl": "https://models-resources.concord.org/clue-curriculum", + "curriculumSiteUrl": "https://models-resources.concord.org/clue-curriculum", "unitCodeMap": { "bio4community": "bio", "completely-rational": "cr", diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index 33b615ebad..70e9a0e278 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -105,7 +105,7 @@ const PARTIAL_RAW_OFFERING_INFO = { activity_url: "https://foo.bar/?problem=3.2" }; -const curriculumConfig = CurriculumConfig.create({curriculumBaseUrl: ""}); +const curriculumConfig = CurriculumConfig.create({curriculumSiteUrl: ""}); describe("dev mode", () => { const originalUrlParams = UrlParams.urlParams; diff --git a/src/lib/portal-api.test.ts b/src/lib/portal-api.test.ts index e9b5435a33..d673e79bfa 100644 --- a/src/lib/portal-api.test.ts +++ b/src/lib/portal-api.test.ts @@ -39,7 +39,7 @@ describe("Portal Offerings", () => { }); describe("getPortalClassOfferings", () => { - const curriculumConfig = CurriculumConfig.create({curriculumBaseUrl: ""}); + const curriculumConfig = CurriculumConfig.create({curriculumSiteUrl: ""}); const mockAppConfig = { config: { defaultProblemOrdinal: "1.1" } } as AppConfigModelType; diff --git a/src/models/document/document-types.ts b/src/models/document/document-types.ts index ae17b75964..af7bb7553e 100644 --- a/src/models/document/document-types.ts +++ b/src/models/document/document-types.ts @@ -7,6 +7,7 @@ export const ProblemDocument = "problem"; export const PersonalDocument = "personal"; export const PlanningDocument = "planning"; export const LearningLogDocument = "learningLog"; +export const ExemplarDocument = "exemplar"; export const ProblemPublication = "publication"; export const PersonalPublication = "personalPublication"; export const LearningLogPublication = "learningLogPublication"; @@ -38,7 +39,7 @@ export function isPublishedType(type: string) { .indexOf(type) >= 0; } export function isSortableType(type: string){ - return [ProblemDocument, PersonalDocument, LearningLogDocument].indexOf(type) >= 0; + return [ProblemDocument, PersonalDocument, LearningLogDocument, ExemplarDocument].indexOf(type) >= 0; } // This function uses a bit of a hack to determine if a document is curriculum or not: // curriculum documents have no ids. @@ -50,7 +51,7 @@ export function isCurriculumDocument(documentId?: string) { export const DocumentTypeEnum = types.enumeration("type", [SectionDocumentDEPRECATED, - ProblemDocument, PersonalDocument, PlanningDocument, LearningLogDocument, + ProblemDocument, PersonalDocument, PlanningDocument, LearningLogDocument, ExemplarDocument, ProblemPublication, PersonalPublication, LearningLogPublication, SupportPublication]); export type DocumentType = Instance; export type ProblemOrPlanningDocumentType = typeof ProblemDocument | typeof PlanningDocument; diff --git a/src/models/stores/class.ts b/src/models/stores/class.ts index 3c125967ed..714d837cf6 100644 --- a/src/models/stores/class.ts +++ b/src/models/stores/class.ts @@ -44,7 +44,7 @@ export const ClassModel = types })); }); }, - addSingleUser(user: any) { + addUser(user: any) { self.users.put(user); } }; diff --git a/src/models/stores/create-exemplar-docs.test.ts b/src/models/stores/create-exemplar-docs.test.ts new file mode 100644 index 0000000000..af15f690ca --- /dev/null +++ b/src/models/stores/create-exemplar-docs.test.ts @@ -0,0 +1,17 @@ +import { createExemplarDocId } from "./create-exemplar-docs"; + +describe("Create Exemplar Docs", () => { + describe("createExemplarDocId", () => { + test("should encode disallowed characters", async () => { + // We need to ensure that we don't create exemplar doc Ids with + // characters that firebase disallows in keys [, ], #, $, ., / + const result = createExemplarDocId("https://www.example.com/?thing=[1,2,3]$FOO#", ""); + expect(result).toBe("curriculum:https%3A%2F%2Fwww%2Eexample%2Ecom%2F%3Fthing%3D%5B1%2C2%2C3%5D%24FOO%23"); + }); + + test("should strip curriculum base url", () => { + const result = createExemplarDocId("https://www.example.com/curriculum/thing", "https://www.example.com/curriculum"); + expect(result).toBe("curriculum:%2Fthing"); + }); + }); +}); diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index cda90b908e..2d9071f486 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -1,9 +1,11 @@ import { ProblemModelType } from "../curriculum/problem"; import { DocumentsModelType } from "./documents"; -import { DocumentModel } from "../document/document"; +import { DocumentModel, DocumentModelSnapshotType } from "../document/document"; import { UserModelType } from "./user"; import { ClassModelType, ClassUserModel } from "./class"; import { kExemplarUserParams } from "./user-types"; +import { ICurriculumConfig } from "./curriculum-config"; +import { ExemplarDocument } from "../document/document-types"; interface ICreateExemplarDocsParams { unitUrl: string; @@ -11,75 +13,76 @@ interface ICreateExemplarDocsParams { documents: DocumentsModelType; classStore: ClassModelType; user: UserModelType; + curriculumConfig: ICurriculumConfig; } +interface IExemplarData { + content: DocumentModelSnapshotType; + tag: string; + title: string; + url: string; +} + +// TODO: pass the stores as a parameter, but use a simplified interface +// plus a second paramter for the unitUrl +// function would only require the properties it needs export async function createAndLoadExemplarDocs({ unitUrl, problem, documents, classStore, - user + curriculumConfig }: ICreateExemplarDocsParams) { const { exemplarPaths } = problem; const exemplarsData = await getExemplarsData(unitUrl, exemplarPaths); - classStore.addSingleUser(ClassUserModel.create(kExemplarUserParams)); - createExemplarDocs(documents, user, exemplarsData); + classStore.addUser(ClassUserModel.create(kExemplarUserParams)); + createExemplarDocs(documents, exemplarsData, curriculumConfig); } export async function getExemplarsData(unitUrl: string, exemplarUrls: string[]){ - return await Promise.all( + return Promise.all( exemplarUrls.map(async (url: string) => { const fetchUrl = new URL(url, unitUrl).href; const response = await fetch(fetchUrl); const data = await response.json(); - const urlSegments = fetchUrl.split("/"); - return { + // TODO: validate shape of `data`? + const result: IExemplarData = { ...data, - url: fetchUrl, - curriculumBranch: urlSegments[5], - unit: urlSegments[6], - dirName: urlSegments[7], - investigationSlug: urlSegments[8], - problemSlug: urlSegments[9], - exemplarSlug: urlSegments[10], + url: fetchUrl }; + return result; }) ); } -function createExemplarDocId(exemplarData: any) { - // QUESTION: I have yet to understand how this is going to be used so this is dummy function - const { - curriculumBranch, - unit, - dirName, - investigationSlug, - problemSlug, - exemplarSlug - } = exemplarData; - return `${curriculumBranch}-${unit}-${dirName}-${investigationSlug}-${problemSlug}-${exemplarSlug}`; +export function createExemplarDocId(exemplarDataUrl: string, curriculumBaseUrl: string) { + let identifier = exemplarDataUrl; + if (exemplarDataUrl.startsWith(curriculumBaseUrl)) { + identifier = exemplarDataUrl.slice(curriculumBaseUrl.length); + } + return "curriculum:" + encodeURIComponent(identifier).replace(/\./g, "%2E"); } -function createExemplarDocs(documents: DocumentsModelType, user: UserModelType, exemplarsData: any) { +function createExemplarDocs( + documents: DocumentsModelType, + exemplarsData: IExemplarData[], + curriculumConfig: ICurriculumConfig +) { exemplarsData.forEach((exemplarData: any) => { - const exemplarDocId = createExemplarDocId(exemplarData); - // QUESTION: I see "title" some places? And properties.caption others? Using both for now. - const newDocParams = { + const exemplarDocId = createExemplarDocId(exemplarData.url, curriculumConfig.curriculumBaseUrl); + const newDocParams: DocumentModelSnapshotType = { title: exemplarData.title, uid: 'ivan_idea_1', - type: "personal", + type: ExemplarDocument, visibility: "public", content: exemplarData.content, - key: exemplarDocId, - properties: { - caption: exemplarData.title - } + key: exemplarDocId }; makeDocFromData(newDocParams, documents); }); } -function makeDocFromData(newDocParams: any, documents: DocumentsModelType) { +function makeDocFromData(newDocParams: DocumentModelSnapshotType, documents: DocumentsModelType) { const newDoc = DocumentModel.create(newDocParams); documents.add(newDoc); } diff --git a/src/models/stores/curriculum-config.test.ts b/src/models/stores/curriculum-config.test.ts index 687750614d..85b5516926 100644 --- a/src/models/stores/curriculum-config.test.ts +++ b/src/models/stores/curriculum-config.test.ts @@ -2,7 +2,7 @@ import { CurriculumConfig, ICurriculumConfigSnapshot, getProblemOrdinal } from " function getConfig(overrides?: Partial) { return CurriculumConfig.create({ - curriculumBaseUrl: "https://curriculum.example.com", + curriculumSiteUrl: "https://curriculum.example.com", ...overrides }); } diff --git a/src/models/stores/curriculum-config.ts b/src/models/stores/curriculum-config.ts index 7b1976545b..62eb215600 100644 --- a/src/models/stores/curriculum-config.ts +++ b/src/models/stores/curriculum-config.ts @@ -10,7 +10,7 @@ interface ICurriculumConfigEnv { export const CurriculumConfig = types .model("CurriculumConfig", { // base URL of external curriculum unit repository - curriculumBaseUrl: types.string, + curriculumSiteUrl: types.string, // unit code overrides (legacy unit code support) unitCodeMap: types.map(types.string), // default problem to load if none specified @@ -21,6 +21,13 @@ export const CurriculumConfig = types return getEnv(self) as ICurriculumConfigEnv | undefined; } })) + .views(self => ({ + get curriculumBaseUrl() { + const { curriculumBranch } = self.env?.urlParams || {}; + const branchName = stripPTNumberFromBranch(curriculumBranch ?? "main"); + return `${self.curriculumSiteUrl}/branch/${branchName}`; + } + })) .views(self => ({ getUnitUrl(unitParam: string) { const unitParamUrl = getUrlFromRelativeOrFullString(unitParam); @@ -28,9 +35,7 @@ export const CurriculumConfig = types return unitParamUrl.href; } const unitCode = self.unitCodeMap.get(unitParam) || unitParam; - const { curriculumBranch } = self.env?.urlParams || {}; - const branchName = stripPTNumberFromBranch(curriculumBranch ?? "main"); - return `${self.curriculumBaseUrl}/branch/${branchName}/${unitCode}/content.json`; + return `${self.curriculumBaseUrl}/${unitCode}/content.json`; } })) .views(self => ({ diff --git a/src/models/stores/stores.test.ts b/src/models/stores/stores.test.ts index 88f1605d58..486ed441cd 100644 --- a/src/models/stores/stores.test.ts +++ b/src/models/stores/stores.test.ts @@ -22,7 +22,7 @@ describe("stores object", () => { it("supports passing in stores for testing", () => { const curriculumConfig = CurriculumConfig.create({ - curriculumBaseUrl: "", + curriculumSiteUrl: "", defaultUnit: "foo", }); const appMode: AppMode = "dev"; diff --git a/src/models/stores/stores.ts b/src/models/stores/stores.ts index 1e69c192f4..2c012a7da2 100644 --- a/src/models/stores/stores.ts +++ b/src/models/stores/stores.ts @@ -262,6 +262,7 @@ class Stores implements IStores{ documents: this.documents, user: this.user, classStore: this.class, + curriculumConfig }); } From f429d4b7ff047e691b2c6e353ecc07d3684e32f4 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Mon, 4 Mar 2024 17:34:13 -0500 Subject: [PATCH 17/78] include appConfig when creating exemplar document --- src/models/stores/create-exemplar-docs.ts | 22 +++++++++++++++------- src/models/stores/stores.ts | 3 ++- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index 2d9071f486..c31815b1cf 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -1,11 +1,12 @@ import { ProblemModelType } from "../curriculum/problem"; import { DocumentsModelType } from "./documents"; -import { DocumentModel, DocumentModelSnapshotType } from "../document/document"; +import { DocumentModelSnapshotType, createDocumentModelWithEnv } from "../document/document"; import { UserModelType } from "./user"; import { ClassModelType, ClassUserModel } from "./class"; import { kExemplarUserParams } from "./user-types"; import { ICurriculumConfig } from "./curriculum-config"; import { ExemplarDocument } from "../document/document-types"; +import { AppConfigModelType } from "./app-config-model"; interface ICreateExemplarDocsParams { unitUrl: string; @@ -14,6 +15,7 @@ interface ICreateExemplarDocsParams { classStore: ClassModelType; user: UserModelType; curriculumConfig: ICurriculumConfig; + appConfig: AppConfigModelType; } interface IExemplarData { @@ -31,12 +33,13 @@ export async function createAndLoadExemplarDocs({ problem, documents, classStore, - curriculumConfig + curriculumConfig, + appConfig }: ICreateExemplarDocsParams) { const { exemplarPaths } = problem; const exemplarsData = await getExemplarsData(unitUrl, exemplarPaths); classStore.addUser(ClassUserModel.create(kExemplarUserParams)); - createExemplarDocs(documents, exemplarsData, curriculumConfig); + createExemplarDocs(documents, exemplarsData, curriculumConfig, appConfig); } export async function getExemplarsData(unitUrl: string, exemplarUrls: string[]){ @@ -66,7 +69,8 @@ export function createExemplarDocId(exemplarDataUrl: string, curriculumBaseUrl: function createExemplarDocs( documents: DocumentsModelType, exemplarsData: IExemplarData[], - curriculumConfig: ICurriculumConfig + curriculumConfig: ICurriculumConfig, + appConfig: AppConfigModelType ) { exemplarsData.forEach((exemplarData: any) => { const exemplarDocId = createExemplarDocId(exemplarData.url, curriculumConfig.curriculumBaseUrl); @@ -78,11 +82,15 @@ function createExemplarDocs( content: exemplarData.content, key: exemplarDocId }; - makeDocFromData(newDocParams, documents); + makeDocFromData(newDocParams, documents, appConfig); }); } -function makeDocFromData(newDocParams: DocumentModelSnapshotType, documents: DocumentsModelType) { - const newDoc = DocumentModel.create(newDocParams); +function makeDocFromData( + newDocParams: DocumentModelSnapshotType, + documents: DocumentsModelType, + appConfig: AppConfigModelType +) { + const newDoc = createDocumentModelWithEnv(appConfig, newDocParams); documents.add(newDoc); } diff --git a/src/models/stores/stores.ts b/src/models/stores/stores.ts index 2c012a7da2..881b436df2 100644 --- a/src/models/stores/stores.ts +++ b/src/models/stores/stores.ts @@ -262,7 +262,8 @@ class Stores implements IStores{ documents: this.documents, user: this.user, classStore: this.class, - curriculumConfig + curriculumConfig, + appConfig }); } From c4fe53d4f47ddadde44740cd2510c18284a20dc6 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Tue, 5 Mar 2024 09:44:24 -0500 Subject: [PATCH 18/78] change qa config subtabs folder structure to match cms - set up exemplar folder and sections folder in qa-config-subtabs - change paths to investigations - add exemplar content --- .../demo/units/qa-config-subtabs/content.json | 15 +++++++++------ .../problem-1/exemplar-1/content.json | 18 ++++++++++++++++++ .../problem-1/first/content.json | 0 .../problem-1/second/content.json | 0 .../problem-1/third/content.json | 0 .../problem-2/first/content.json | 0 .../problem-2/second/content.json | 0 .../problem-2/third/content.json | 0 8 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 src/public/demo/units/qa-config-subtabs/exemplars/investigation-1/problem-1/exemplar-1/content.json rename src/public/demo/units/qa-config-subtabs/{ => sections}/investigation-1/problem-1/first/content.json (100%) rename src/public/demo/units/qa-config-subtabs/{ => sections}/investigation-1/problem-1/second/content.json (100%) rename src/public/demo/units/qa-config-subtabs/{ => sections}/investigation-1/problem-1/third/content.json (100%) rename src/public/demo/units/qa-config-subtabs/{ => sections}/investigation-1/problem-2/first/content.json (100%) rename src/public/demo/units/qa-config-subtabs/{ => sections}/investigation-1/problem-2/second/content.json (100%) rename src/public/demo/units/qa-config-subtabs/{ => sections}/investigation-1/problem-2/third/content.json (100%) diff --git a/src/public/demo/units/qa-config-subtabs/content.json b/src/public/demo/units/qa-config-subtabs/content.json index bc1c48bc2e..c61d4c38f4 100644 --- a/src/public/demo/units/qa-config-subtabs/content.json +++ b/src/public/demo/units/qa-config-subtabs/content.json @@ -310,9 +310,12 @@ "title": "1.1 Unit Toolbar Configuration", "subtitle": "Text, Table, Drawing", "sections": [ - "investigation-1/problem-1/first/content.json", - "investigation-1/problem-1/second/content.json", - "investigation-1/problem-1/third/content.json" + "sections/investigation-1/problem-1/first/content.json", + "sections/investigation-1/problem-1/second/content.json", + "sections/investigation-1/problem-1/third/content.json" + ], + "exemplars": [ + "exemplars/investigation-1/problem-1/exemplar-1/content.json" ] }, { @@ -336,9 +339,9 @@ ] }, "sections": [ - "investigation-1/problem-2/first/content.json", - "investigation-1/problem-2/second/content.json", - "investigation-1/problem-2/third/content.json" + "sections/investigation-1/problem-2/first/content.json", + "sections/investigation-1/problem-2/second/content.json", + "sections/investigation-1/problem-2/third/content.json" ] } ] diff --git a/src/public/demo/units/qa-config-subtabs/exemplars/investigation-1/problem-1/exemplar-1/content.json b/src/public/demo/units/qa-config-subtabs/exemplars/investigation-1/problem-1/exemplar-1/content.json new file mode 100644 index 0000000000..b490a50de6 --- /dev/null +++ b/src/public/demo/units/qa-config-subtabs/exemplars/investigation-1/problem-1/exemplar-1/content.json @@ -0,0 +1,18 @@ +{ + "title": "First Exemplar", + "tag": "building-up", + "content": { + "tiles": [ + { + "id": "Exemplar2", + "content": { + "type": "Text", + "format": "html", + "text": [ + "

I am exemplary.

" + ] + } + } + ] + } +} diff --git a/src/public/demo/units/qa-config-subtabs/investigation-1/problem-1/first/content.json b/src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-1/first/content.json similarity index 100% rename from src/public/demo/units/qa-config-subtabs/investigation-1/problem-1/first/content.json rename to src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-1/first/content.json diff --git a/src/public/demo/units/qa-config-subtabs/investigation-1/problem-1/second/content.json b/src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-1/second/content.json similarity index 100% rename from src/public/demo/units/qa-config-subtabs/investigation-1/problem-1/second/content.json rename to src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-1/second/content.json diff --git a/src/public/demo/units/qa-config-subtabs/investigation-1/problem-1/third/content.json b/src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-1/third/content.json similarity index 100% rename from src/public/demo/units/qa-config-subtabs/investigation-1/problem-1/third/content.json rename to src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-1/third/content.json diff --git a/src/public/demo/units/qa-config-subtabs/investigation-1/problem-2/first/content.json b/src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-2/first/content.json similarity index 100% rename from src/public/demo/units/qa-config-subtabs/investigation-1/problem-2/first/content.json rename to src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-2/first/content.json diff --git a/src/public/demo/units/qa-config-subtabs/investigation-1/problem-2/second/content.json b/src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-2/second/content.json similarity index 100% rename from src/public/demo/units/qa-config-subtabs/investigation-1/problem-2/second/content.json rename to src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-2/second/content.json diff --git a/src/public/demo/units/qa-config-subtabs/investigation-1/problem-2/third/content.json b/src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-2/third/content.json similarity index 100% rename from src/public/demo/units/qa-config-subtabs/investigation-1/problem-2/third/content.json rename to src/public/demo/units/qa-config-subtabs/sections/investigation-1/problem-2/third/content.json From 350ed0c454c9fa50e021a4d010b92dc8e9086a18 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Tue, 5 Mar 2024 10:26:41 -0500 Subject: [PATCH 19/78] sortable types should show name prefix in document caption --- src/hooks/use-document-caption.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/hooks/use-document-caption.tsx b/src/hooks/use-document-caption.tsx index e85af7e70f..5fe1e9608f 100644 --- a/src/hooks/use-document-caption.tsx +++ b/src/hooks/use-document-caption.tsx @@ -1,7 +1,7 @@ import { useFirestoreTeacher } from "./firestore-hooks"; import { useAppConfig, useClassStore, useProblemStore, useUserStore } from "./use-stores"; import { DocumentModelType } from "../models/document/document"; -import { isPublishedType, isUnpublishedType } from "../models/document/document-types"; +import { isPublishedType, isSortableType, isUnpublishedType } from "../models/document/document-types"; import { getDocumentDisplayTitle } from "../models/document/document-utils"; export function useDocumentCaption(document: DocumentModelType, isStudentWorkspaceDoc?: boolean) { @@ -17,7 +17,11 @@ export function useDocumentCaption(document: DocumentModelType, isStudentWorkspa || (document.isRemote ? teacher?.name : "") || "Unknown User"; - const hasNamePrefix = document.isRemote || isPublishedType(type) || isUnpublishedType(type) || isStudentWorkspaceDoc; + const hasNamePrefix = document.isRemote + || isPublishedType(type) + || isUnpublishedType(type) + || isSortableType(type) + || isStudentWorkspaceDoc; const namePrefix = hasNamePrefix ? `${userName}: ` : ""; const dateSuffix = document.isRemote && document.createdAt ? ` (${new Date(document.createdAt).toLocaleDateString()})` From 6690ed1f7593bb0a87ad510acdd515ba8dd81aa5 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 5 Mar 2024 11:24:46 -0500 Subject: [PATCH 20/78] fix exemplarPath prop in tests --- src/models/curriculum/problem.test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/models/curriculum/problem.test.ts b/src/models/curriculum/problem.test.ts index db2bb3e09a..dd104fb5ad 100644 --- a/src/models/curriculum/problem.test.ts +++ b/src/models/curriculum/problem.test.ts @@ -16,7 +16,8 @@ describe("problem model", () => { ordinal: 1, title: "test", subtitle: "", - sectionsFromSnapshot: [] + sectionsFromSnapshot: [], + exemplarPaths: [], }); expect(problem.fullTitle).toBe("test"); }); @@ -48,7 +49,8 @@ describe("problem model", () => { type: "initialChallenge", } ], - config: {} + config: {}, + exemplarPaths: [] }); expect(problem.fullTitle).toBe("test: sub"); }); @@ -216,7 +218,8 @@ describe("problem model", () => { config: { disabledFeatures: ["foo"], settings: { foo: "bar" } - } + }, + exemplarPaths: [] }); }); @@ -238,7 +241,8 @@ describe("problem model", () => { config: { disabledFeatures: ["foo"], settings: { foo: "bar" } - } + }, + exemplarPaths: [] }); }); @@ -262,7 +266,8 @@ describe("problem model", () => { config: { disabledFeatures: ["foo"], settings: { foo: "bar" } - } + }, + exemplarPaths: [] }); }); }); From f36dca89c071908e14a18b0bcce47ffe00aa6716 Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Tue, 5 Mar 2024 12:15:34 -0800 Subject: [PATCH 21/78] Clean up - remove numberline toolbar.tsx and numberline-toolbar-buttons.tsx + corresponding scss files --- .../components/numberline-tile.scss | 1 - .../numberline/components/numberline-tile.tsx | 28 +----- .../components/numberline-toolbar-buttons.tsx | 91 ------------------- .../components/numberline-toolbar.scss | 56 ------------ .../components/numberline-toolbar.tsx | 70 -------------- 5 files changed, 2 insertions(+), 244 deletions(-) delete mode 100644 src/plugins/numberline/components/numberline-toolbar-buttons.tsx delete mode 100644 src/plugins/numberline/components/numberline-toolbar.scss delete mode 100644 src/plugins/numberline/components/numberline-toolbar.tsx diff --git a/src/plugins/numberline/components/numberline-tile.scss b/src/plugins/numberline/components/numberline-tile.scss index 3a17da38c9..07db469f47 100644 --- a/src/plugins/numberline/components/numberline-tile.scss +++ b/src/plugins/numberline/components/numberline-tile.scss @@ -1,4 +1,3 @@ - @import "../../../components/vars.sass"; .numberline-wrapper { diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index 9071d584b8..7873e8634f 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -19,38 +19,15 @@ import { import NumberlineArrowLeft from "../../../assets/numberline-arrow-left.svg"; import NumberlineArrowRight from "../../../assets/numberline-arrow-right.svg"; import { EditableNumberlineValue } from './editable-numberline-value'; -// import { TileToolbar } from 'src/components/toolbar/tile-toolbar'; import { TileToolbar } from "../../../components/toolbar/tile-toolbar"; - - -import "./numberline-toolbar-registration"; -import "./numberline-tile.scss"; import { INumberlineToolbarContext, NumberlineToolbarContext } from './numberline-toolbar-context'; +import "./numberline-toolbar-registration"; -//**********************✔️•↳******************* GUIDELINES ************************************************ -// - As students, we want to express both closed form counting and inequalities which end with an open circle point. - -// ✔️ add an open circle point to the toolbar -// ✔️ add a select point arrow to the toolbar -// ✔️ revise solid point button to the latest versions -// ↳ this is also on by default -// ✔️ when the open circle point is used, create an open circle point -// when the solid circle point is used, create a solid point like before -// when the selection button is used, points can be moved or selected for deletion -// point types are saved and restored and can be used in curriculum. - - -//TODO: hover circle would be at drawMouseFollowPoint +import "./numberline-tile.scss"; //TODO: delete numberline-toolbar, and numberline-toolbar-buttons.tsx //TODO: update line 184 of unit-configuration.md - -//**********************✔️•↳******************* END ************************************************ - - - - export const NumberlineTile: React.FC = observer(function NumberlineTile(props){ const { documentContent, model, readOnly, scale, tileElt, onRegisterTileApi, onUnregisterTileApi } = props; // console.log("-------------------"); @@ -281,7 +258,6 @@ export const NumberlineTile: React.FC = observer(function Numberline .attr("r", innerPointRadius * 0.5) .attr("opacity", 1) .classed("mouse-follow-point", true); - } }; diff --git a/src/plugins/numberline/components/numberline-toolbar-buttons.tsx b/src/plugins/numberline/components/numberline-toolbar-buttons.tsx deleted file mode 100644 index 6f29fa8933..0000000000 --- a/src/plugins/numberline/components/numberline-toolbar-buttons.tsx +++ /dev/null @@ -1,91 +0,0 @@ -import React from "react"; -import classNames from "classnames"; -import { Tooltip, TooltipProps } from "react-tippy"; -import { useTooltipOptions } from "../../../hooks/use-tooltip-options"; -import SelectIcon from "../assets/numberline-toolbar-select-tool.svg"; -import PointIcon from "../assets/numberline-toolbar-point-icon.svg"; -import PointOpenIcon from "../assets/numberline-toolbar-point-open-icon.svg"; -import ResetIcon from "../assets/numberline-toolbar-reset-icon.svg"; -import DeleteIcon from "../assets/numberline-toolbar-delete-icon.svg"; -//TODO: Eventually we want to swap the DeleteIcon (X) with the DeleteSelectionIcon -// keep DeleteIcon until we are already ready to change the main Toolbar Delete Tile Icon -// import DeleteSelectionIcon from "../assets/numberline-toolbar-delete-selection-icon.svg"; - -import "./numberline-toolbar.scss"; - -interface INumberlineButtonProps{ - className?: string; - icon?: JSX.Element; - onClick?: (e:React.MouseEvent) => void; - tooltipOptions?: TooltipProps - selected?: boolean; - -} - -const NumberlineButton = ({ className, icon, onClick, tooltipOptions, selected}: INumberlineButtonProps) => { - const to = useTooltipOptions(tooltipOptions); - const classes = classNames("toolbar-button", className); - return ( - - - - ); -}; - -interface ISetNumberlineHandler { - onClick?: () => void; - pointTypeIsOpen?: boolean; -} - -export const SelectButton = ({ onClick }: ISetNumberlineHandler) => ( - } - onClick={onClick} - tooltipOptions={{ title: "Select Point"}} - /> -); - -export const PointButton = ({ onClick, pointTypeIsOpen }: ISetNumberlineHandler) => { - const pointTypeIsFilled = !pointTypeIsOpen; - return ( - } - onClick={onClick} - tooltipOptions={{ title: "Place Point"}} - /> - ); -}; - -export const PointOpenButton = ({ onClick, pointTypeIsOpen }: ISetNumberlineHandler) => { - return ( - } - onClick={onClick} - tooltipOptions={{ title: "Place Open Point"}} - /> - ); -}; - -export const ResetButton = ({ onClick }: ISetNumberlineHandler) => ( - } - onClick={onClick} - tooltipOptions={{ title: "Reset"}} - /> -); - - -export const DeleteButton = ({ onClick }: ISetNumberlineHandler) => ( - } - onClick={onClick} - tooltipOptions={{ title: "Delete Point(s)"}} - /> -); diff --git a/src/plugins/numberline/components/numberline-toolbar.scss b/src/plugins/numberline/components/numberline-toolbar.scss deleted file mode 100644 index 0a0dabd9c7..0000000000 --- a/src/plugins/numberline/components/numberline-toolbar.scss +++ /dev/null @@ -1,56 +0,0 @@ -@import "../../../components/vars.sass"; - -.numberline-toolbar{ - position: absolute; - background-color: $workspace-teal-light-9; - height: $toolbar-button-height + 4px; - border: $toolbar-border; - border-radius: 0 0 $toolbar-border-radius $toolbar-border-radius; - overflow: hidden; - display: flex; - z-index: $toolbar-z-index; - - &.disabled { - display: none; - } - - .toolbar-button { - position: relative; - // These are button elements so we have to disable default button styles - // of border, padding, and display - border: 0; - padding: 0; - display: block; - user-select: none; - width: $toolbar-button-width; - height: $toolbar-button-height; - cursor: pointer; - background-color: $workspace-teal-light-9; - - &:hover { - background-color: $workspace-teal-light-6; - } - - &:active { - background-color: $workspace-teal-light-4; - } - - &.disabled { - opacity: 35%; - pointer-events: none; - } - - &.point-button{ - &.selected { - background-color: $workspace-teal-light-4; - } - } - - &.point-open-button { - &.selected { - background-color: $workspace-teal-light-4; - } - } - - } - } diff --git a/src/plugins/numberline/components/numberline-toolbar.tsx b/src/plugins/numberline/components/numberline-toolbar.tsx deleted file mode 100644 index 934f890386..0000000000 --- a/src/plugins/numberline/components/numberline-toolbar.tsx +++ /dev/null @@ -1,70 +0,0 @@ -import { observer } from "mobx-react"; -import React from "react"; -import ReactDOM from "react-dom"; -import { - IFloatingToolbarProps, useFloatingToolbarLocation -} from "../../../components/tiles/hooks/use-floating-toolbar-location"; -import { useSettingFromStores } from "../../../hooks/use-stores"; -import { SelectButton, PointButton, PointOpenButton, ResetButton, DeleteButton } from "./numberline-toolbar-buttons"; - -import "./numberline-toolbar.scss"; - -const defaultButtons = ["select", "point", "point-open", "reset", "delete"]; - -interface INumberlineToolbarProps extends IFloatingToolbarProps { - handleClearPoints: () => void; //model - content.deleteAllPoints() - handleDeletePoint: () => void; //deleteSelectedPoints - handleCreatePointType: (isOpen: boolean) => void; - pointTypeIsOpen: boolean -} - -export const NumberlineToolbar: React.FC = observer((props) => { - const { documentContent, tileElt, onIsEnabled, - handleDeletePoint, handleCreatePointType, pointTypeIsOpen, ...others } = props; - - const enabled = onIsEnabled(); - const location = useFloatingToolbarLocation({ - documentContent, - tileElt, - toolbarHeight: 38, - toolbarTopOffset: 2, - enabled, - ...others - }); - - const buttonSettings = useSettingFromStores("tools", "numberline") as unknown as string[] | undefined; - const buttons = buttonSettings || defaultButtons; - - const getToolbarButton = (toolName: string) => { - switch (toolName) { - case "select": - return ; - case "point": - return handleCreatePointType(false)} - pointTypeIsOpen={pointTypeIsOpen} - />; - case "point-open": - return handleCreatePointType(true)} - pointTypeIsOpen={pointTypeIsOpen} - />; - case "reset": - return ; - case "delete": - return ; - } - }; - - return documentContent - ? ReactDOM.createPortal( -
e.stopPropagation()}> - {buttons.map(button => { - return getToolbarButton(button); - })} -
, documentContent) - : null; -}); From 0e0b0530f76e7b950786c75ea2047ebf9e87594a Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Tue, 5 Mar 2024 16:49:03 -0500 Subject: [PATCH 22/78] add exemplar to sort-work tests - confirm exemplar docs load - confirm they appear in name and group sorts - confirm they can be opened and closed, but not edited --- .../teacher_sort_work_view_spec.js | 65 ++++++++++++++----- cypress/support/elements/common/SortedWork.js | 7 +- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js b/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js index 493c755ab2..4ea5450d02 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js @@ -9,8 +9,8 @@ let resourcesPanel = new ResourcesPanel; let dashboard = new TeacherDashboard; let header = new ClueHeader; const canvas = new Canvas; -const title = "1.1 Unit Toolbar Configuration"; -const copyTitle = "Personal Workspace"; +const title = "1.1 Unit Toolbar Configuration"; +const copyTitle = "Personal Workspace"; const queryParams1 = `${Cypress.config("clueTestqaConfigSubtabsUnitTeacher6")}`; const queryParams2 = `${Cypress.config("qaConfigSubtabsUnitTeacher1")}`; @@ -53,7 +53,7 @@ describe('SortWorkView Tests', () => { sortWork.getListItemByGroup().click(); // Select 'Group' sort type cy.wait(1000); - + cy.log('verify opening and closing a document from the sort work view'); sortWork.getSortWorkItem().eq(1).click(); // Open the first document in the list resourcesPanel.getEditableDocumentContent().should('be.visible'); @@ -61,19 +61,33 @@ describe('SortWorkView Tests', () => { sortWork.getSortWorkItem().should('be.visible'); // Verify the document is closed }); + it("should open Sort Work tab and test sorting by group", () => { - const students = ["student:1", "student:2", "student:3", "student:4"] - const studentProblemDocs = [`Student 1: ${title}`, `Student 2: ${title}`, `Student 3: ${title}`,`Student 4: ${title}`]; - const studentPersonalDocs = [`Student 1: ${copyTitle}`, `Student 2: ${copyTitle}`, `Student 3: ${copyTitle}`,`Student 4: ${copyTitle}`]; + const students = ["student:1", "student:2", "student:3", "student:4"]; + const studentProblemDocs = [ + `Student 1: ${title}`, + `Student 2: ${title}`, + `Student 3: ${title}`, + `Student 4: ${title}` + ]; + const studentPersonalDocs = [ + `Student 1: ${copyTitle}`, + `Student 2: ${copyTitle}`, + `Student 3: ${copyTitle}`, + `Student 4: ${copyTitle}` + ]; + const exemplarDocs = [ + `Ivan Idea: First Exemplar` + ]; cy.log("run CLUE for various students creating their problem and personal documents"); students.forEach(student => { runClueAsStudent(student); canvas.copyDocument(copyTitle); canvas.getPersonalDocTitle().find('span').text().should('contain', copyTitle); - }) + }); - cy.log("run CLUE as teacher and check student problem and personal documents show in Sort Work"); + cy.log("run CLUE as teacher and check student problem, personal, and exemplar docs show in Sort Work"); cy.visit(queryParams2); cy.waitForLoad(); cy.openTopTab('sort-work'); @@ -85,29 +99,37 @@ describe('SortWorkView Tests', () => { sortWork.getSortWorkItem().should('contain', doc); }); + cy.log("verify that exemplar document shows in Sort Work"); + sortWork.getSortWorkItem().eq(0).should('contain', exemplarDocs[0]); + cy.log("open problem doc and make sure Edit button doesn't show and Close button shows"); sortWork.getSortWorkItem().contains(studentProblemDocs[0]).click(); resourcesPanel.getDocumentEditButton().should("not.exist"); resourcesPanel.getDocumentCloseButton().should("exist").click(); - + cy.log("open personal doc and make sure Edit button doesn't show and Close button shows"); sortWork.getSortWorkItem().contains(studentPersonalDocs[0]).click(); resourcesPanel.getDocumentEditButton().should("not.exist"); resourcesPanel.getDocumentCloseButton().should("exist").click(); + cy.log("open exemplar doc and make sure Edit button doesn't show and Close button shows"); + sortWork.getSortWorkItem().contains(exemplarDocs[0]).click(); + resourcesPanel.getDocumentEditButton().should("not.exist"); + resourcesPanel.getDocumentCloseButton().should("exist").click(); + cy.log("check all problem and personal docs show in the correct group"); studentProblemDocs.forEach(doc => { sortWork.checkDocumentInGroup("Group 5", doc); - }) + }); studentPersonalDocs.forEach(doc => { sortWork.checkDocumentInGroup("Group 5", doc); - }) - + }); + cy.log("run CLUE as a student:1 and leave the group"); runClueAsStudent(students[0]); header.leaveGroup(); - cy.log("check student:1 problem and personal docs show in No Group"); + cy.log("check student:1 problem, exemplar, and personal docs show in No Group"); cy.visit(queryParams2); cy.waitForLoad(); cy.openTopTab('sort-work'); @@ -120,7 +142,18 @@ describe('SortWorkView Tests', () => { sortWork.checkDocumentInGroup("Group 5", studentPersonalDocs[1]); sortWork.checkDocumentNotInGroup("No Group", studentProblemDocs[1]); sortWork.checkDocumentNotInGroup("No Group", studentPersonalDocs[1]); - + sortWork.checkDocumentInGroup("No Group", exemplarDocs[0]); + + cy.log("check that problem and exemplar documents can be sorted by name"); + sortWork.getSortByMenu().click(); + cy.wait(1000); + sortWork.getListItemByName().click(); + sortWork.checkSectionHeaderLabelsExist([ + "1, Student", "1, Teacher", "2, Student", "3, Student", "4, Student", "Idea, Ivan" + ]); + sortWork.checkDocumentInGroup("Idea, Ivan", exemplarDocs[0]); + sortWork.checkDocumentInGroup("1, Student", studentProblemDocs[0]); + cy.log("run CLUE as a student:1 and join group 6"); runClueAsStudent(students[0], 6); @@ -148,5 +181,5 @@ describe('SortWorkView Tests', () => { sortWork.checkDocumentInGroup("No Group", studentProblemDocs[0]); sortWork.checkDocumentInGroup("No Group", studentPersonalDocs[0]); sortWork.checkGroupDoesNotExist("Group 6"); - }) -}) + }); +}); diff --git a/cypress/support/elements/common/SortedWork.js b/cypress/support/elements/common/SortedWork.js index 03a553a739..03089b63e3 100644 --- a/cypress/support/elements/common/SortedWork.js +++ b/cypress/support/elements/common/SortedWork.js @@ -6,7 +6,7 @@ class SortedWork { return cy.get('[data-test="list-item-name"]'); } getListItemByGroup() { - return cy.get('[data-test="list-item-group"]') + return cy.get('[data-test="list-item-group"]'); } getSortWorkItem() { return cy.get(".sort-work-view .sorted-sections .list-item .footer .info"); @@ -20,6 +20,11 @@ class SortedWork { checkGroupDoesNotExist(group) { cy.get(".sort-work-view .sorted-sections .section-header-label").should("not.contain", group); } + checkSectionHeaderLabelsExist(labels){ + labels.forEach(label => { + cy.get(".sort-work-view .sorted-sections .section-header-label").should("contain", label); + }); + } } export default SortedWork; From a3d2bb99846cb7ad1f3687868253805f28b386b4 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 5 Mar 2024 17:24:57 -0500 Subject: [PATCH 23/78] update the diagram with exemplar loading --- docs/launch-sequence.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/launch-sequence.md b/docs/launch-sequence.md index f0a827a654..df277a55b8 100644 --- a/docs/launch-sequence.md +++ b/docs/launch-sequence.md @@ -39,6 +39,7 @@ flowchart TB %% LE.start: Loading curriculum unit unit(Get unit JSON) unit --> tiles + unit --> exemplars unit --> sections %% LE.end: Loading curriculum uit @@ -54,6 +55,10 @@ flowchart TB sections --> resolveSectionsLoadedPromise([resolve sectionsLoadedPromise]) %% LE.end: Loading curriculum sections + %% LE.start: Loading exemplar documents + exemplars("createAndLoadExemplarDocs") + %% LE.end: Loading exemplar documents + resolveUnitLoadedPromise([resolve unitLoadedPromise]) configStores(Configure some stores) From 8a266826da73ec329c982e5085b890821e903539 Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Wed, 6 Mar 2024 09:04:17 -0800 Subject: [PATCH 24/78] Push for demo/deployment --- .../numberline/components/numberline-tile.tsx | 102 +++++++++++------- .../components/numberline-toolbar-context.ts | 5 +- .../numberline-toolbar-registration.tsx | 16 ++- 3 files changed, 78 insertions(+), 45 deletions(-) diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index 7873e8634f..0bda2764d6 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -25,31 +25,52 @@ import "./numberline-toolbar-registration"; import "./numberline-tile.scss"; -//TODO: delete numberline-toolbar, and numberline-toolbar-buttons.tsx -//TODO: update line 184 of unit-configuration.md +export enum ToolbarOption { + Selection = "selection", + Filled = "filled", + Open = "open" +} + + +//**********************✔️•↳******************* GUIDELINES ************************************************ + +// Numberline ticket + +//✔️ selection tool selected upon initialization +//✔️ first three are mutually exclusive - ie only one can be selected, and it deselects the others +//• HOVER BUG: in "select" & "filled" - the hover circle is "open", +// ↳ in "select" it should be no circle +// ↳ in filled it should be filled circled + +//• increase Active area for point placement +//• bug for scrolling the open point +//• examine any points disappearing when switching between filled/open(?) - make a new tile. + + +//Select tool - should have no hovering circle - but you can move export const NumberlineTile: React.FC = observer(function NumberlineTile(props){ - const { documentContent, model, readOnly, scale, tileElt, onRegisterTileApi, onUnregisterTileApi } = props; - // console.log("-------------------"); + const { model, readOnly, tileElt, onRegisterTileApi } = props; + const content = model.content as NumberlineContentModelType; - // console.log("content:", content); const [hoverPointId, setHoverPointId] = useState(""); const [_selectedPointId, setSelectedPointId] = useState(""); // Just used to rerender when a point is selected const ui = useUIStore(); const isTileSelected = ui.isSelectedTile(model); /* ========================== [ Determine Point is Open or Filled ] ========================= */ - const [pointTypeIsOpen, setPointTypeIsOpen] = useState(false); //default start with filled in point - const handleCreatePointType = (_isOpen: boolean) => { - // console.log("--------------------------------------"); - // console.log("initial STATE:", pointTypeIsOpen); - setPointTypeIsOpen(_isOpen); + const [toolbarOption, setToolbarOption] = useState(ToolbarOption.Selection); //"selection" + + const handleCreatePointType = (_isOpen: ToolbarOption) => { + console.log("--------------------------------------"); + console.log("initial STATE:", toolbarOption); + setToolbarOption(_isOpen); }; useEffect(() => { - // console.log("Updated STATE:", pointTypeIsOpen); - }, [pointTypeIsOpen]); + console.log("Updated STATE:", toolbarOption); + }, [toolbarOption]); /* ============================ [ Model Manipulation Functions ] ============================ */ const createPoint = (xValue: number, _pointTypeIsOpen: boolean) => { @@ -201,7 +222,7 @@ export const NumberlineTile: React.FC = observer(function Numberline return isBetweenYBounds && isBetweenXBounds; }; - const handleMouseClick = (e: Event, _pointTypeIsOpen: boolean) => { + const handleMouseClick = (e: Event, _toolbarOption: ToolbarOption) => { if (!readOnly){ if (hoverPointId) { const hoverPoint = content.getPoint(hoverPointId); @@ -210,10 +231,14 @@ export const NumberlineTile: React.FC = observer(function Numberline setSelectedPointId(hoverPoint.id); } } else { - // only create point if we are not hovering over a point and within bounding box + // Create point if we are not hovering over a point and within bounding box + // and toolbarOption is either filled or open const [mouseX, mouseY] = mousePos(e); if (mouseInBoundingBox(mouseX, mouseY)) { - createPoint(xScale.invert(mouseX), _pointTypeIsOpen); + if(_toolbarOption !== ToolbarOption.Selection){ + const isPointOpen = _toolbarOption === ToolbarOption.Open; + createPoint(xScale.invert(mouseX), isPointOpen); + } } } } @@ -235,32 +260,33 @@ export const NumberlineTile: React.FC = observer(function Numberline } const drawMouseFollowPoint = (mouseX: number) => { - //when user's mouse hovers over numberline, create a circle that follows the mouse - const followPoint = svg.append("circle") + // When in selection mode - do not draw any hover circle + if (toolbarOption === ToolbarOption.Selection) { + clearMouseFollowPoint(); + return; + } + // For either open or filled mode, draw outer circle + svg.append("circle") .attr("cx", mouseX) .attr("cy", yMidPoint) .attr("r", innerPointRadius) .classed("mouse-follow-point", true) - .classed("point-inner-circle", !pointTypeIsOpen); //filled - - /* =========================== [ Blue / White Circles ] ============================= */ - if(pointTypeIsOpen) { - followPoint //recreate outer blue circle - .attr("fill", "#0069ff") - .attr("stroke", "black") - .attr("opacity", 1) - .attr("stroke-width", 1.5); - //create - svg.append("circle") - .attr("fill", "white") - .attr("cx", mouseX) - .attr("cy", yMidPoint) - .attr("r", innerPointRadius * 0.5) - .attr("opacity", 1) - .classed("mouse-follow-point", true); - } + .classed("point-inner-circle", true); + + //For open mode - draw inner white circle + if (toolbarOption === ToolbarOption.Open) { + svg.append("circle") + .attr("fill", "white") + .attr("cx", mouseX) + .attr("cy", yMidPoint) + .attr("r", innerPointRadius * 0.5) + .attr("opacity", 1) + .classed("mouse-follow-point", true); + } }; + + const clearMouseFollowPoint = () => svg.selectAll(".mouse-follow-point").remove(); const handleMouseMove = (e: MouseEvent) => { @@ -275,7 +301,7 @@ export const NumberlineTile: React.FC = observer(function Numberline } }; - svg.on("click", (e) => handleMouseClick(e, pointTypeIsOpen)); + svg.on("click", (e) => handleMouseClick(e, toolbarOption)); svg.on("mousemove", (e) => handleMouseMove(e)); // * ================================ [ Construct Numberline ] =============================== */ @@ -410,8 +436,6 @@ export const NumberlineTile: React.FC = observer(function Numberline .attr('id', `inner-white-${p.id}`); } }); - - }; updateCircles(); @@ -423,7 +447,7 @@ export const NumberlineTile: React.FC = observer(function Numberline handleClearPoints: () => content.deleteAllPoints(), handleDeletePoint: deleteSelectedPoints, handleCreatePointType, - pointTypeIsOpen + toolbarOption }; return ( diff --git a/src/plugins/numberline/components/numberline-toolbar-context.ts b/src/plugins/numberline/components/numberline-toolbar-context.ts index 2f2fab8fb0..bf783d6393 100644 --- a/src/plugins/numberline/components/numberline-toolbar-context.ts +++ b/src/plugins/numberline/components/numberline-toolbar-context.ts @@ -1,10 +1,11 @@ import { createContext } from "react"; +import { ToolbarOption } from "./numberline-tile"; export interface INumberlineToolbarContext { handleClearPoints: () => void; handleDeletePoint: () => void; - handleCreatePointType: (isOpen: boolean) => void; - pointTypeIsOpen: boolean; + handleCreatePointType: (isOpen: ToolbarOption) => void; + toolbarOption: ToolbarOption; } export const NumberlineToolbarContext = createContext(null); diff --git a/src/plugins/numberline/components/numberline-toolbar-registration.tsx b/src/plugins/numberline/components/numberline-toolbar-registration.tsx index 74a4bae0e4..6ffdcb906d 100644 --- a/src/plugins/numberline/components/numberline-toolbar-registration.tsx +++ b/src/plugins/numberline/components/numberline-toolbar-registration.tsx @@ -8,12 +8,18 @@ import PointIcon from "../assets/numberline-toolbar-point-icon.svg"; import PointOpenIcon from "../assets/numberline-toolbar-point-open-icon.svg"; import ResetIcon from "../assets/numberline-toolbar-reset-icon.svg"; import DeleteIcon from "../assets/numberline-toolbar-delete-icon.svg"; +import { ToolbarOption } from "./numberline-tile"; const SelectButton = ({name}: IToolbarButtonComponentProps) => { + const context = useContext(NumberlineToolbarContext); + const selected = context?.toolbarOption === ToolbarOption.Selection; function handleClick() { console.log("select clicked"); + if (context) { + context.handleCreatePointType(ToolbarOption.Selection); + } } return ( @@ -21,6 +27,8 @@ const SelectButton = ({name}: IToolbarButtonComponentProps) => { name={name} title="Select Point" onClick={handleClick} + selected={selected} + > @@ -30,11 +38,11 @@ const SelectButton = ({name}: IToolbarButtonComponentProps) => { const PointButton = ({name}: IToolbarButtonComponentProps) => { const context = useContext(NumberlineToolbarContext); - const selected = !context?.pointTypeIsOpen; + const selected = context?.toolbarOption === ToolbarOption.Filled; function handleClick() { if (context) { - context.handleCreatePointType(false); + context.handleCreatePointType(ToolbarOption.Filled); } } @@ -54,11 +62,11 @@ const PointButton = ({name}: IToolbarButtonComponentProps) => { const PointOpenButton = ({name}: IToolbarButtonComponentProps) => { const context = useContext(NumberlineToolbarContext); - const selected = context?.pointTypeIsOpen; + const selected = context?.toolbarOption === ToolbarOption.Open; function handleClick() { if (context) { - context.handleCreatePointType(true); + context.handleCreatePointType(ToolbarOption.Open); } } From a04ef2976089a9fe2e64b98ab2ae7a3cc9043118 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Wed, 6 Mar 2024 14:25:23 -0500 Subject: [PATCH 25/78] add authored comments tag as a property of the document --- src/models/stores/create-exemplar-docs.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index c31815b1cf..0ffd91db12 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -80,7 +80,10 @@ function createExemplarDocs( type: ExemplarDocument, visibility: "public", content: exemplarData.content, - key: exemplarDocId + key: exemplarDocId, + properties: { + authoredCommentTag: exemplarData.tag + } }; makeDocFromData(newDocParams, documents, appConfig); }); From 7b0de59fb9b91fbc71304f6ca3bae3cceca7ed06 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Wed, 6 Mar 2024 15:24:02 -0500 Subject: [PATCH 26/78] catalog documents that have authored tags --- src/models/stores/sorted-documents.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index 9bfb052ae0..fb0bb39657 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -137,6 +137,17 @@ export class SortedDocuments { //*************************************** Sort By Strategy ************************************** get sortByStrategy(): SortedDocument[]{ + const docKeysWithAuthoredTags: any = []; + this.documents.all.forEach(doc => { + if (doc.getProperty("authoredCommentTag")) { + docKeysWithAuthoredTags.push({ + tag: doc.getProperty("authoredCommentTag"), + keys: [doc.key] + }); + } + }); + console.log("| docKeysWithAuthoredTags: ", docKeysWithAuthoredTags); + const commentTags = this.commentTags; const tagsWithDocs: Record = {}; if (commentTags) { From 6c2178c1c34168f672343dc2f30631eb6b3429e9 Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Wed, 6 Mar 2024 13:38:08 -0800 Subject: [PATCH 27/78] Push for deployment --- .../numberline/components/numberline-tile.tsx | 72 ++++++++++--------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index 0bda2764d6..aa10befd80 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -31,24 +31,6 @@ export enum ToolbarOption { Open = "open" } - -//**********************✔️•↳******************* GUIDELINES ************************************************ - -// Numberline ticket - -//✔️ selection tool selected upon initialization -//✔️ first three are mutually exclusive - ie only one can be selected, and it deselects the others -//• HOVER BUG: in "select" & "filled" - the hover circle is "open", -// ↳ in "select" it should be no circle -// ↳ in filled it should be filled circled - -//• increase Active area for point placement -//• bug for scrolling the open point -//• examine any points disappearing when switching between filled/open(?) - make a new tile. - - -//Select tool - should have no hovering circle - but you can move - export const NumberlineTile: React.FC = observer(function NumberlineTile(props){ const { model, readOnly, tileElt, onRegisterTileApi } = props; @@ -160,6 +142,7 @@ export const NumberlineTile: React.FC = observer(function Numberline top: y - outerPointRadius, width: 2 * outerPointRadius }; + console.log("boundingBox:", boundingBox); return boundingBox; } }, [annotationPointCenter]); @@ -235,6 +218,7 @@ export const NumberlineTile: React.FC = observer(function Numberline // and toolbarOption is either filled or open const [mouseX, mouseY] = mousePos(e); if (mouseInBoundingBox(mouseX, mouseY)) { + console.log("inside bounding box!!!"); if(_toolbarOption !== ToolbarOption.Selection){ const isPointOpen = _toolbarOption === ToolbarOption.Open; createPoint(xScale.invert(mouseX), isPointOpen); @@ -285,8 +269,6 @@ export const NumberlineTile: React.FC = observer(function Numberline } }; - - const clearMouseFollowPoint = () => svg.selectAll(".mouse-follow-point").remove(); const handleMouseMove = (e: MouseEvent) => { @@ -425,18 +407,44 @@ export const NumberlineTile: React.FC = observer(function Numberline /* =========================== [ Blue White Circles] ============================= */ - content.pointsArr.forEach(p => { - if(p.isOpen) { - svg.append("circle") - .attr("class", "inner-white-point") - .attr("cx", xScale(p.currentXValue)) - .attr("cy", yMidPoint) - .attr("r", innerPointRadius * 0.5) - .attr("fill", "white") - .attr('id', `inner-white-${p.id}`); - } - }); - }; + // Filter the points that should have an inner white circle + + + const openPoints = content.pointsArr.filter(p => p.isOpen); + const innerWhitePoints = svg.selectAll('.circle,.inner-white-point') + .data(openPoints); + + // content.pointsArr.forEach(p => { + // if(p.isOpen) { + // svg.append("circle") + // .attr("class", "inner-white-point") + // .attr("cx", xScale(p.currentXValue)) + // .attr("cy", yMidPoint) + // .attr("r", innerPointRadius * 0.5) + // .attr("fill", "white") + // .attr('id', `inner-white-${p.id}`); + // } + // }); + + innerWhitePoints.enter() + .append("circle") + .attr("class", "inner-white-point") + .attr("cx", (p) => xScale(p.currentXValue)) + .attr("cy", yMidPoint) + .attr("r", innerPointRadius * 0.5) + .attr("fill", "white") + .attr('id', (p) => `inner-white-${p.id}`); + + // Update selection: Update circles' position if necessary + innerWhitePoints + .attr("cx", (p) => xScale(p.currentXValue)) + .attr("cy", yMidPoint); + + // Exit selection: Remove circles for data that's no longer present + innerWhitePoints.exit().remove(); + + + }; //end of updateCircles() updateCircles(); } From 35adba6ad2c10bfb78083289ddd350eecb6969b8 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Wed, 6 Mar 2024 22:16:40 -0500 Subject: [PATCH 28/78] wip, notes --- src/models/stores/sorted-documents.ts | 43 ++++++++++++++++++++------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index fb0bb39657..7c362f0ce5 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -137,16 +137,16 @@ export class SortedDocuments { //*************************************** Sort By Strategy ************************************** get sortByStrategy(): SortedDocument[]{ - const docKeysWithAuthoredTags: any = []; - this.documents.all.forEach(doc => { - if (doc.getProperty("authoredCommentTag")) { - docKeysWithAuthoredTags.push({ - tag: doc.getProperty("authoredCommentTag"), - keys: [doc.key] - }); - } - }); - console.log("| docKeysWithAuthoredTags: ", docKeysWithAuthoredTags); + // const docsWithAuthoredTags: any = []; + // this.documents.all.forEach(doc => { + // if (doc.getProperty("authoredCommentTag")) { + // // NOTE: we are assuming only one tag from CMS per document? + // docsWithAuthoredTags.push({ + // tag: doc.getProperty("authoredCommentTag"), + // doc + // }); + // } + // }); const commentTags = this.commentTags; const tagsWithDocs: Record = {}; @@ -186,6 +186,13 @@ export class SortedDocuments { if (tagsWithDocs[""]) { tagsWithDocs[""].docKeysFoundWithTag.push(doc.key); } + // HEY: I think you can do everything right here + if (doc.getProperty("authoredCommentTag")) { + const labelStr = doc.getProperty("authoredCommentTag"); + if (tagsWithDocs[labelStr]) { + tagsWithDocs[labelStr].docKeysFoundWithTag.push(doc.key); + } + } } }); @@ -200,6 +207,22 @@ export class SortedDocuments { documents }); }); + // console.log("\n\n| docsWithAuthoredTags: ", docsWithAuthoredTags); + // console.log("| tagsWithDocs: ", tagsWithDocs); + // console.log("| sortedDocsArr: ", sortedDocsArr); + + // this.documents.all.forEach(doc => { + // if (doc.getProperty("authoredCommentTag")) { + // const labelStr = doc.getProperty("authoredCommentTag"); + // sortedDocsArr.forEach((section: SortedDocument) => { + // console.log("section.sectionLabel: ", section.sectionLabel, "vs labelStr: ", labelStr); + // if (section.sectionLabel === labelStr) { + // section.documents.push(doc); + // } + // }); + // } + // }); + return sortedDocsArr; } From 8f8132df916acb6b4d2bd16c485e773407c94896 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Wed, 6 Mar 2024 22:17:20 -0500 Subject: [PATCH 29/78] ok --- src/models/stores/sorted-documents.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index 7c362f0ce5..f6658b2a6b 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -188,10 +188,10 @@ export class SortedDocuments { } // HEY: I think you can do everything right here if (doc.getProperty("authoredCommentTag")) { - const labelStr = doc.getProperty("authoredCommentTag"); - if (tagsWithDocs[labelStr]) { - tagsWithDocs[labelStr].docKeysFoundWithTag.push(doc.key); - } + // const labelStr = doc.getProperty("authoredCommentTag"); + // if (tagsWithDocs[labelStr]) { + // tagsWithDocs[labelStr].docKeysFoundWithTag.push(doc.key); + // } } } }); From ae569e0c22b9c990b4893994b18190404c0c81c4 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Wed, 6 Mar 2024 22:28:52 -0500 Subject: [PATCH 30/78] cleanup --- src/models/stores/sorted-documents.ts | 37 ++++----------------------- 1 file changed, 5 insertions(+), 32 deletions(-) diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index f6658b2a6b..f008e5d548 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -137,17 +137,6 @@ export class SortedDocuments { //*************************************** Sort By Strategy ************************************** get sortByStrategy(): SortedDocument[]{ - // const docsWithAuthoredTags: any = []; - // this.documents.all.forEach(doc => { - // if (doc.getProperty("authoredCommentTag")) { - // // NOTE: we are assuming only one tag from CMS per document? - // docsWithAuthoredTags.push({ - // tag: doc.getProperty("authoredCommentTag"), - // doc - // }); - // } - // }); - const commentTags = this.commentTags; const tagsWithDocs: Record = {}; if (commentTags) { @@ -186,12 +175,11 @@ export class SortedDocuments { if (tagsWithDocs[""]) { tagsWithDocs[""].docKeysFoundWithTag.push(doc.key); } - // HEY: I think you can do everything right here - if (doc.getProperty("authoredCommentTag")) { - // const labelStr = doc.getProperty("authoredCommentTag"); - // if (tagsWithDocs[labelStr]) { - // tagsWithDocs[labelStr].docKeysFoundWithTag.push(doc.key); - // } + const foundTagKey = doc.getProperty("authoredCommentTag"); + if (foundTagKey !== undefined && foundTagKey !== "") { + if (tagsWithDocs[foundTagKey]) { + tagsWithDocs[foundTagKey].docKeysFoundWithTag.push(doc.key); + } } } }); @@ -207,21 +195,6 @@ export class SortedDocuments { documents }); }); - // console.log("\n\n| docsWithAuthoredTags: ", docsWithAuthoredTags); - // console.log("| tagsWithDocs: ", tagsWithDocs); - // console.log("| sortedDocsArr: ", sortedDocsArr); - - // this.documents.all.forEach(doc => { - // if (doc.getProperty("authoredCommentTag")) { - // const labelStr = doc.getProperty("authoredCommentTag"); - // sortedDocsArr.forEach((section: SortedDocument) => { - // console.log("section.sectionLabel: ", section.sectionLabel, "vs labelStr: ", labelStr); - // if (section.sectionLabel === labelStr) { - // section.documents.push(doc); - // } - // }); - // } - // }); return sortedDocsArr; } From eed93dd8ebe6e714182c01514f072ec6ca550882 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Wed, 6 Mar 2024 22:43:21 -0500 Subject: [PATCH 31/78] move out of test for empty tag --- src/models/stores/sorted-documents.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index f008e5d548..f3dfc676f6 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -175,11 +175,11 @@ export class SortedDocuments { if (tagsWithDocs[""]) { tagsWithDocs[""].docKeysFoundWithTag.push(doc.key); } - const foundTagKey = doc.getProperty("authoredCommentTag"); - if (foundTagKey !== undefined && foundTagKey !== "") { - if (tagsWithDocs[foundTagKey]) { - tagsWithDocs[foundTagKey].docKeysFoundWithTag.push(doc.key); - } + } + const foundTagKey = doc.getProperty("authoredCommentTag"); + if (foundTagKey !== undefined && foundTagKey !== "") { + if (tagsWithDocs[foundTagKey]) { + tagsWithDocs[foundTagKey].docKeysFoundWithTag.push(doc.key); } } }); From 9ff298ef6dbcc2af94185ee9148813d383542cd5 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Wed, 6 Mar 2024 22:45:41 -0500 Subject: [PATCH 32/78] cleanup --- src/models/stores/sorted-documents.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index f3dfc676f6..c6b90d6bfb 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -195,7 +195,6 @@ export class SortedDocuments { documents }); }); - return sortedDocsArr; } From bf592ad5a031f885fd220927f949c87bccf8e3ab Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Thu, 7 Mar 2024 10:46:57 -0500 Subject: [PATCH 33/78] if doc has cms-sourced tag, remove it's key from list of docs with empty tag --- src/models/stores/sorted-documents.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index c6b90d6bfb..be8f435c31 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -180,6 +180,9 @@ export class SortedDocuments { if (foundTagKey !== undefined && foundTagKey !== "") { if (tagsWithDocs[foundTagKey]) { tagsWithDocs[foundTagKey].docKeysFoundWithTag.push(doc.key); + // if tagsWithDocs[""] has this doc.key, remove it + const index = tagsWithDocs[""].docKeysFoundWithTag.indexOf(doc.key); + if (index > -1) tagsWithDocs[""].docKeysFoundWithTag.splice(index, 1); } } }); From 42b201002fdc6f9a77bdc42adc3576428f91a67e Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Thu, 7 Mar 2024 12:52:00 -0500 Subject: [PATCH 34/78] add exemplar tagging to cypress test - add tags in content.json - add comment/tag routines to test --- .../teacher_sort_work_view_spec.js | 44 +++++++++++++++++-- cypress/support/elements/common/ChatPanel.js | 10 ++--- cypress/support/elements/common/SortedWork.js | 7 ++- .../demo/units/qa-config-subtabs/content.json | 3 +- .../problem-1/exemplar-1/content.json | 2 +- 5 files changed, 54 insertions(+), 12 deletions(-) diff --git a/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js b/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js index 4ea5450d02..70a2fbf771 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js @@ -3,11 +3,14 @@ import SortedWork from "../../../support/elements/common/SortedWork"; import ResourcesPanel from "../../../support/elements/common/ResourcesPanel"; import Canvas from '../../../support/elements/common/Canvas'; import ClueHeader from '../../../support/elements/common/cHeader'; +import ChatPanel from "../../../support/elements/common/ChatPanel"; let sortWork = new SortedWork; let resourcesPanel = new ResourcesPanel; let dashboard = new TeacherDashboard; let header = new ClueHeader; +let chatPanel = new ChatPanel; + const canvas = new Canvas; const title = "1.1 Unit Toolbar Configuration"; const copyTitle = "Personal Workspace"; @@ -45,13 +48,13 @@ describe('SortWorkView Tests', () => { sortWork.getSortByMenu().click(); // Open the sort menu cy.wait(1000); - sortWork.getListItemByName().click(); //Select 'Name' sort type + sortWork.getSortByNameOption().click(); //Select 'Name' sort type cy.wait(1000); sortWork.getSortByMenu().click(); // Open the sort menu again cy.wait(1000); - sortWork.getListItemByGroup().click(); // Select 'Group' sort type + sortWork.getSortByGroupOption().click(); // Select 'Group' sort type cy.wait(1000); cy.log('verify opening and closing a document from the sort work view'); @@ -147,13 +150,48 @@ describe('SortWorkView Tests', () => { cy.log("check that problem and exemplar documents can be sorted by name"); sortWork.getSortByMenu().click(); cy.wait(1000); - sortWork.getListItemByName().click(); + sortWork.getSortByNameOption().click(); sortWork.checkSectionHeaderLabelsExist([ "1, Student", "1, Teacher", "2, Student", "3, Student", "4, Student", "Idea, Ivan" ]); sortWork.checkDocumentInGroup("Idea, Ivan", exemplarDocs[0]); sortWork.checkDocumentInGroup("1, Student", studentProblemDocs[0]); + cy.log("check that exemplar document is displayed in strategy tag sourced from CMS"); + sortWork.getSortByMenu().click(); + cy.wait(1000); + sortWork.getSortByTagOption().click(); + sortWork.checkDocumentInGroup("Unit Rate", exemplarDocs[0]); + + cy.log("check that exemplar document can also be assigned tag by teacher"); + sortWork.getSortWorkItem().contains(exemplarDocs[0]).click(); + chatPanel.getChatPanelToggle().click(); + chatPanel.addCommentTagAndVerify("Diverging Designs"); + + cy.log("check that exemplar document is displayed in new tag"); + chatPanel.getChatCloseButton().click(); + cy.openTopTab('sort-work'); + // at the moment this is required to refresh the sort + sortWork.getSortByMenu().click(); + sortWork.getSortByNameOption().click(); + sortWork.getSortByMenu().click(); + sortWork.getSortByTagOption().click(); + sortWork.checkDocumentInGroup("Diverging Designs", exemplarDocs[0]); + + cy.log("remove the teacher added tag and reload"); + sortWork.getSortWorkItem().contains(exemplarDocs[0]).click(); + chatPanel.getChatPanelToggle().click(); + chatPanel.deleteTeacherComments(); + cy.wait(1000); + cy.visit(queryParams2); + cy.waitForLoad(); + cy.openTopTab('sort-work'); + + cy.log("check that exemplar document is still displayed in strategy tag sourced from CMS"); + sortWork.getSortByMenu().click(); + sortWork.getSortByTagOption().click(); + sortWork.checkDocumentInGroup("Unit Rate", exemplarDocs[0]); + cy.log("run CLUE as a student:1 and join group 6"); runClueAsStudent(students[0], 6); diff --git a/cypress/support/elements/common/ChatPanel.js b/cypress/support/elements/common/ChatPanel.js index 56d82ed426..cda61aff23 100644 --- a/cypress/support/elements/common/ChatPanel.js +++ b/cypress/support/elements/common/ChatPanel.js @@ -16,7 +16,7 @@ class ChatPanel{ this.getChatPanelToggle().click(); cy.wait(10000); } - }) + }); } getChatPanel() { return cy.get('.chat-panel'); @@ -202,9 +202,9 @@ class ChatPanel{ cy.wait(1000); } }) - ) + ); } else { - cy.log("No Comments to Delete"); + cy.log("No Comments to Delete"); } }); } @@ -219,7 +219,7 @@ class ChatPanel{ this.getCommentedDocumentList().find('.document-box').then(((value) => { totalCount = Cypress.$(value).length; expect(value).to.have.length(totalCount); - for(i=0; i < totalCount; i++) { + for(i=0; i < totalCount; i++) { this.getCommentedDocumentList().find('.document-box').eq(i).click({ force: true }); cy.wait(3000); this.addDocumentCommentAndVerify("This is " + (i+1) + " document list comment"); @@ -229,7 +229,7 @@ class ChatPanel{ cy.wait(3000); } }) - ) + ); } } export default ChatPanel; diff --git a/cypress/support/elements/common/SortedWork.js b/cypress/support/elements/common/SortedWork.js index 03089b63e3..c8ba071244 100644 --- a/cypress/support/elements/common/SortedWork.js +++ b/cypress/support/elements/common/SortedWork.js @@ -2,12 +2,15 @@ class SortedWork { getSortByMenu() { return cy.get('.custom-select.sort-work-sort-menu'); } - getListItemByName() { + getSortByNameOption() { return cy.get('[data-test="list-item-name"]'); } - getListItemByGroup() { + getSortByGroupOption() { return cy.get('[data-test="list-item-group"]'); } + getSortByTagOption(){ + return cy.get('[data-test="list-item-identify-design approach"]'); + } getSortWorkItem() { return cy.get(".sort-work-view .sorted-sections .list-item .footer .info"); } diff --git a/src/public/demo/units/qa-config-subtabs/content.json b/src/public/demo/units/qa-config-subtabs/content.json index c61d4c38f4..6a541f153b 100644 --- a/src/public/demo/units/qa-config-subtabs/content.json +++ b/src/public/demo/units/qa-config-subtabs/content.json @@ -57,7 +57,8 @@ "materials": "Varies Material/Surface", "physical": "Varies Physical Factors", "user": "Varies Human Factors", - "system": "Varies System Factors" + "system": "Varies System Factors", + "unit-rate": "Unit Rate" }, "tagPrompt": "Identify Design Approach", "tools": ["Simulator"], diff --git a/src/public/demo/units/qa-config-subtabs/exemplars/investigation-1/problem-1/exemplar-1/content.json b/src/public/demo/units/qa-config-subtabs/exemplars/investigation-1/problem-1/exemplar-1/content.json index b490a50de6..6678afc796 100644 --- a/src/public/demo/units/qa-config-subtabs/exemplars/investigation-1/problem-1/exemplar-1/content.json +++ b/src/public/demo/units/qa-config-subtabs/exemplars/investigation-1/problem-1/exemplar-1/content.json @@ -1,6 +1,6 @@ { "title": "First Exemplar", - "tag": "building-up", + "tag": "unit-rate", "content": { "tiles": [ { From 0b084e6651952dc9ffc808c9a7873019ce3bc23f Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Thu, 7 Mar 2024 14:22:00 -0500 Subject: [PATCH 35/78] also assert that teacher created tag is removed from doc --- .../functional/teacher_tests/teacher_sort_work_view_spec.js | 3 ++- cypress/support/elements/common/SortedWork.js | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js b/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js index 70a2fbf771..eb28c8560f 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js @@ -187,10 +187,11 @@ describe('SortWorkView Tests', () => { cy.waitForLoad(); cy.openTopTab('sort-work'); - cy.log("check that exemplar document is still displayed in strategy tag sourced from CMS"); + cy.log("check that exemplar document is still displayed in strategy tag sourced from CMS but not in teacher added tag"); sortWork.getSortByMenu().click(); sortWork.getSortByTagOption().click(); sortWork.checkDocumentInGroup("Unit Rate", exemplarDocs[0]); + sortWork.checkGroupIsEmpty("Diverging Designs"); cy.log("run CLUE as a student:1 and join group 6"); runClueAsStudent(students[0], 6); diff --git a/cypress/support/elements/common/SortedWork.js b/cypress/support/elements/common/SortedWork.js index c8ba071244..6a6af6fcb0 100644 --- a/cypress/support/elements/common/SortedWork.js +++ b/cypress/support/elements/common/SortedWork.js @@ -20,6 +20,10 @@ class SortedWork { checkDocumentNotInGroup(groupName, doc) { cy.get(".sort-work-view .sorted-sections .section-header-label").contains(groupName).parent().parent().find(".list .list-item .footer .info").should("not.contain", doc); } + checkGroupIsEmpty(groupName){ + cy.get(".sort-work-view .sorted-sections .section-header-label") + .contains(groupName).parent().parent().find(".list").should('be.empty'); + } checkGroupDoesNotExist(group) { cy.get(".sort-work-view .sorted-sections .section-header-label").should("not.contain", group); } From 3d5fca390c2ce2af879a2655370239c054ae273d Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Thu, 7 Mar 2024 12:01:44 -0800 Subject: [PATCH 36/78] Clean up for deployment, fix test method, add documentation --- .../elements/tile/NumberlineToolTile.js | 6 +-- docs/unit-configuration.md | 10 +++-- .../numberline/components/numberline-tile.tsx | 2 +- .../components/numberline-toolbar-context.ts | 2 +- .../numberline-toolbar-registration.tsx | 40 ++----------------- 5 files changed, 15 insertions(+), 45 deletions(-) diff --git a/cypress/support/elements/tile/NumberlineToolTile.js b/cypress/support/elements/tile/NumberlineToolTile.js index 108d4eff40..4b24e075e6 100644 --- a/cypress/support/elements/tile/NumberlineToolTile.js +++ b/cypress/support/elements/tile/NumberlineToolTile.js @@ -26,10 +26,10 @@ class NumberlineToolTile { return cy.get(".numberline-tool-container .point-inner-circle").not(".mouse-follow-point"); } deleteAllPointsOnNumberline(){ - this.getClearButton().click(); + this.getResetButton().click(); } - getClearButton(){ - return cy.get(`.clear-points`); //TODO: fix this + getResetButton(){ + return cy.get(".reset"); } setMaxValue(max){ this.getMaxBox().dblclick().clear().type(`${max}{enter}`); diff --git a/docs/unit-configuration.md b/docs/unit-configuration.md index 3804122cf4..536be4156e 100644 --- a/docs/unit-configuration.md +++ b/docs/unit-configuration.md @@ -181,9 +181,13 @@ Not updated to common toolbar framework and does not support toolbar configurati #### Numberline -Uses common toolbar framework. -Default buttons: - - "place-point", "clear-points", "delete-points" +Common toolbar framework; default toolbar buttons: + +- `select` - selected by default - can't create points, only move filled or open points. +- `point` - create a filled point by clicking on the numberline. +- `point-open` - create an open point by clicking on the numberline. `select`, `point`, `point-open` are mutually exclusive +- `reset` - clear all points from the numberline +- `delete` - delete selected point(s) from the numberline #### Simulation diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index aa10befd80..84b4028e6f 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -452,7 +452,7 @@ export const NumberlineTile: React.FC = observer(function Numberline // * ================================= [ Register Toolbar ] ================================== */ const toolbarFunctions: INumberlineToolbarContext = { - handleClearPoints: () => content.deleteAllPoints(), + handleResetPoints: () => content.deleteAllPoints(), handleDeletePoint: deleteSelectedPoints, handleCreatePointType, toolbarOption diff --git a/src/plugins/numberline/components/numberline-toolbar-context.ts b/src/plugins/numberline/components/numberline-toolbar-context.ts index bf783d6393..47cd80dbd4 100644 --- a/src/plugins/numberline/components/numberline-toolbar-context.ts +++ b/src/plugins/numberline/components/numberline-toolbar-context.ts @@ -2,7 +2,7 @@ import { createContext } from "react"; import { ToolbarOption } from "./numberline-tile"; export interface INumberlineToolbarContext { - handleClearPoints: () => void; + handleResetPoints: () => void; handleDeletePoint: () => void; handleCreatePointType: (isOpen: ToolbarOption) => void; toolbarOption: ToolbarOption; diff --git a/src/plugins/numberline/components/numberline-toolbar-registration.tsx b/src/plugins/numberline/components/numberline-toolbar-registration.tsx index 6ffdcb906d..40bd5800c1 100644 --- a/src/plugins/numberline/components/numberline-toolbar-registration.tsx +++ b/src/plugins/numberline/components/numberline-toolbar-registration.tsx @@ -83,21 +83,13 @@ const PointOpenButton = ({name}: IToolbarButtonComponentProps) => { }; -// export interface INumberlineToolbarContext { -// handleClearPoints: () => void; -// handleDeletePoint: () => void; -// handleCreatePointType: (isOpen: boolean) => void; -// pointTypeIsOpen: boolean; -// } - const ResetButton = ({name}: IToolbarButtonComponentProps) => { - const context = useContext(NumberlineToolbarContext); - const handleClearPoints = context?.handleClearPoints; + const handleResetPoints = context?.handleResetPoints; function handleClick() { - if (handleClearPoints) { - handleClearPoints(); + if (handleResetPoints) { + handleResetPoints(); } } @@ -106,7 +98,6 @@ const ResetButton = ({name}: IToolbarButtonComponentProps) => { name={name} title="Reset" onClick={handleClick} - // selected={selected} > @@ -129,7 +120,6 @@ const DeleteButton = ({name}: IToolbarButtonComponentProps) => { name={name} title="Delete Point(s)" onClick={handleClick} - // selected={selected} > @@ -137,30 +127,6 @@ const DeleteButton = ({name}: IToolbarButtonComponentProps) => { }; - - -// export const ResetButton = ({ onClick }: ISetNumberlineHandler) => ( -// } -// onClick={onClick} -// tooltipOptions={{ title: "Reset"}} -// /> -// ); - - -// export const DeleteButton = ({ onClick }: ISetNumberlineHandler) => ( -// } -// onClick={onClick} -// tooltipOptions={{ title: "Delete Point(s)"}} -// /> -// ); - - - - registerTileToolbarButtons("numberline", [ { From b4e820e7e013f36515c4b5689533b548eee76952 Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Thu, 7 Mar 2024 12:25:42 -0800 Subject: [PATCH 37/78] Clean up --- src/plugins/numberline/models/numberline-content.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/numberline/models/numberline-content.ts b/src/plugins/numberline/models/numberline-content.ts index 2e635e05d7..73b63239c0 100644 --- a/src/plugins/numberline/models/numberline-content.ts +++ b/src/plugins/numberline/models/numberline-content.ts @@ -124,7 +124,7 @@ export const NumberlineContentModel = TileContentModel }, })) .actions(self => ({ - createAndSelectPoint(xValue: number, isOpen: boolean) { //TODO: we need to pass the "filled" or "open" + createAndSelectPoint(xValue: number, isOpen: boolean) { const newPoint = self.createNewPoint(xValue, isOpen); self.setSelectedPoint(newPoint); return newPoint; From 5f96c3aedf818cceb894861b7a89721557b061a7 Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Thu, 7 Mar 2024 14:48:00 -0800 Subject: [PATCH 38/78] Fix failing cypress tests --- cypress/e2e/functional/tile_tests/numberline_tool_spec.js | 3 ++- cypress/support/elements/tile/NumberlineToolTile.js | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/numberline_tool_spec.js b/cypress/e2e/functional/tile_tests/numberline_tool_spec.js index 3246ad5ab3..f658657e82 100644 --- a/cypress/e2e/functional/tile_tests/numberline_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/numberline_tool_spec.js @@ -28,6 +28,7 @@ context('Numberline Tile', function () { numberlineToolTile.getNumberlineTileTitleText().should("contain", newName); cy.log('will test adding points to a numberline'); + numberlineToolTile.setToolbarPoint(); //click Point in order to add points to numberline numberlineToolTile.addPointOnNumberlineTick(-4.0); numberlineToolTile.addPointOnNumberlineTick(2.0); numberlineToolTile.getPointsOnGraph().should('have.length', 2); @@ -40,7 +41,7 @@ context('Numberline Tile', function () { cy.log("will delete all points"); cy.log("delete all points in the numberline"); - numberlineToolTile.deleteAllPointsOnNumberline(); + numberlineToolTile.setToolbarReset(); numberlineToolTile.getPointsOnGraph().should('have.length', 0); //Numberline tile restore upon page reload diff --git a/cypress/support/elements/tile/NumberlineToolTile.js b/cypress/support/elements/tile/NumberlineToolTile.js index 4b24e075e6..deb759ecfb 100644 --- a/cypress/support/elements/tile/NumberlineToolTile.js +++ b/cypress/support/elements/tile/NumberlineToolTile.js @@ -25,12 +25,18 @@ class NumberlineToolTile { //exclude the hovering circle that follows the mouse on the numberline return cy.get(".numberline-tool-container .point-inner-circle").not(".mouse-follow-point"); } - deleteAllPointsOnNumberline(){ + setToolbarReset(){ this.getResetButton().click(); } + setToolbarPoint(){ + this.getPointButton().click(); + } getResetButton(){ return cy.get(".reset"); } + getPointButton(){ + return cy.get(".point"); + } setMaxValue(max){ this.getMaxBox().dblclick().clear().type(`${max}{enter}`); } From 90c6c2e63f164f601668bd53091161a38a7b7b25 Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Thu, 7 Mar 2024 15:22:55 -0800 Subject: [PATCH 39/78] Clean up --- .../numberline/components/numberline-toolbar-registration.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plugins/numberline/components/numberline-toolbar-registration.tsx b/src/plugins/numberline/components/numberline-toolbar-registration.tsx index 40bd5800c1..28b5d31244 100644 --- a/src/plugins/numberline/components/numberline-toolbar-registration.tsx +++ b/src/plugins/numberline/components/numberline-toolbar-registration.tsx @@ -16,7 +16,6 @@ const SelectButton = ({name}: IToolbarButtonComponentProps) => { const selected = context?.toolbarOption === ToolbarOption.Selection; function handleClick() { - console.log("select clicked"); if (context) { context.handleCreatePointType(ToolbarOption.Selection); } From 8be94a4aacb6a44968b2f2d3789b1d9c544feba3 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Fri, 8 Mar 2024 11:04:19 -0500 Subject: [PATCH 40/78] cleanup --- src/models/curriculum/problem.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/models/curriculum/problem.ts b/src/models/curriculum/problem.ts index 33dae88eaa..ba8274668f 100644 --- a/src/models/curriculum/problem.ts +++ b/src/models/curriculum/problem.ts @@ -30,12 +30,11 @@ const ModernProblemModel = types * with all of the other sections in this problem, or this problem's unit */ sectionsFromSnapshot: types.frozen(), - exemplarPaths: types.array(types.string), // HMM: is above ever not string[]? + exemplarPaths: types.array(types.string), config: types.maybe(types.frozen>()) }) .volatile(self => ({ - sections: observable.array() as SectionModelType[], - // exemplars: observable.array() as any[] // HMM: I may need this later? + sections: observable.array() as SectionModelType[] })) .views(self => ({ get fullTitle() { From 8ab70d1b36fd5e3297bdefc945169632ce09453f Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Fri, 8 Mar 2024 21:20:50 -0500 Subject: [PATCH 41/78] code review fixes #1 - consolidate makeDocFromData into parent function - display exemplar user name instead of hard coding name string --- src/models/stores/create-exemplar-docs.ts | 12 ++---------- src/models/stores/sorted-documents.ts | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index c31815b1cf..c1274422e6 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -82,15 +82,7 @@ function createExemplarDocs( content: exemplarData.content, key: exemplarDocId }; - makeDocFromData(newDocParams, documents, appConfig); + const newDoc = createDocumentModelWithEnv(appConfig, newDocParams); + documents.add(newDoc); }); } - -function makeDocFromData( - newDocParams: DocumentModelSnapshotType, - documents: DocumentsModelType, - appConfig: AppConfigModelType -) { - const newDoc = createDocumentModelWithEnv(appConfig, newDocParams); - documents.add(newDoc); -} diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index 9bfb052ae0..a00dba3166 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -106,7 +106,7 @@ export class SortedDocuments { const documentMap = new Map(); this.filteredDocsByType.forEach((doc) => { const user = this.class.getUserById(doc.uid); - const sectionLabel = user ? `${user.lastName}, ${user.firstName}` : "Idea, Ivan"; + const sectionLabel = user && `${user.lastName}, ${user.firstName}`; if (!documentMap.has(sectionLabel)) { documentMap.set(sectionLabel, { sectionLabel, From 8fd0a389924a991a98301786f3d7fb8940d3493a Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Sat, 9 Mar 2024 11:13:55 -0500 Subject: [PATCH 42/78] do not rename exemplars reference unecessarily --- src/models/curriculum/problem.ts | 9 ++++----- src/models/stores/create-exemplar-docs.ts | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/models/curriculum/problem.ts b/src/models/curriculum/problem.ts index ba8274668f..20ebf05e99 100644 --- a/src/models/curriculum/problem.ts +++ b/src/models/curriculum/problem.ts @@ -30,7 +30,7 @@ const ModernProblemModel = types * with all of the other sections in this problem, or this problem's unit */ sectionsFromSnapshot: types.frozen(), - exemplarPaths: types.array(types.string), + exemplars: types.array(types.string), config: types.maybe(types.frozen>()) }) .volatile(self => ({ @@ -122,12 +122,11 @@ export const ProblemModel = types.snapshotProcessor(ModernProblemModel, { // creating a exemplarsFromSnapshot that will be passed in similar way, // but no volatile property yet const sectionsFromSnapshot = sections || []; - const exemplarPaths = exemplars || []; if (isLegacySnapshot(sn)) { const { disabled: disabledFeatures, settings, ...others } = sn; return { ...others, sectionsFromSnapshot, - exemplarPaths, + exemplars, config: { disabledFeatures, settings } } as ModernProblemSnapshot; } @@ -136,11 +135,11 @@ export const ProblemModel = types.snapshotProcessor(ModernProblemModel, { return { ...others, sectionsFromSnapshot, - exemplarPaths, + exemplars, config: { disabledFeatures, settings, ...config } } as ModernProblemSnapshot; } - return { ...nonSectionProps, sectionsFromSnapshot, exemplarPaths }; + return { ...nonSectionProps, sectionsFromSnapshot, exemplars }; } }); export interface ProblemModelType extends Instance {} diff --git a/src/models/stores/create-exemplar-docs.ts b/src/models/stores/create-exemplar-docs.ts index c1274422e6..19da8895d7 100644 --- a/src/models/stores/create-exemplar-docs.ts +++ b/src/models/stores/create-exemplar-docs.ts @@ -36,8 +36,8 @@ export async function createAndLoadExemplarDocs({ curriculumConfig, appConfig }: ICreateExemplarDocsParams) { - const { exemplarPaths } = problem; - const exemplarsData = await getExemplarsData(unitUrl, exemplarPaths); + const { exemplars } = problem; + const exemplarsData = await getExemplarsData(unitUrl, exemplars); classStore.addUser(ClassUserModel.create(kExemplarUserParams)); createExemplarDocs(documents, exemplarsData, curriculumConfig, appConfig); } From 18da53c088c9722559dc35875766b8afa643f75a Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Sun, 10 Mar 2024 10:28:33 -0400 Subject: [PATCH 43/78] sortable should not be the criterion for having a name prefix --- src/hooks/use-document-caption.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/hooks/use-document-caption.tsx b/src/hooks/use-document-caption.tsx index 5fe1e9608f..492570a320 100644 --- a/src/hooks/use-document-caption.tsx +++ b/src/hooks/use-document-caption.tsx @@ -1,7 +1,7 @@ import { useFirestoreTeacher } from "./firestore-hooks"; import { useAppConfig, useClassStore, useProblemStore, useUserStore } from "./use-stores"; import { DocumentModelType } from "../models/document/document"; -import { isPublishedType, isSortableType, isUnpublishedType } from "../models/document/document-types"; +import { isPublishedType, isUnpublishedType } from "../models/document/document-types"; import { getDocumentDisplayTitle } from "../models/document/document-utils"; export function useDocumentCaption(document: DocumentModelType, isStudentWorkspaceDoc?: boolean) { @@ -10,6 +10,7 @@ export function useDocumentCaption(document: DocumentModelType, isStudentWorkspa const classStore = useClassStore(); const user = useUserStore(); const { type, uid } = document; + console.log("type:", type); const pubVersion = document.pubVersion; const teacher = useFirestoreTeacher(uid, user.network || ""); const userName = classStore.getUserById(uid)?.displayName @@ -20,7 +21,7 @@ export function useDocumentCaption(document: DocumentModelType, isStudentWorkspa const hasNamePrefix = document.isRemote || isPublishedType(type) || isUnpublishedType(type) - || isSortableType(type) + || type === "exemplar" || isStudentWorkspaceDoc; const namePrefix = hasNamePrefix ? `${userName}: ` : ""; const dateSuffix = document.isRemote && document.createdAt From 9e5d0920a21c0a242d620251f37e7f11f1deb2f6 Mon Sep 17 00:00:00 2001 From: Joseph Bacal Date: Mon, 11 Mar 2024 09:37:26 -0400 Subject: [PATCH 44/78] adjust test to match var name --- src/models/curriculum/problem.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/models/curriculum/problem.test.ts b/src/models/curriculum/problem.test.ts index dd104fb5ad..debdc5c2df 100644 --- a/src/models/curriculum/problem.test.ts +++ b/src/models/curriculum/problem.test.ts @@ -17,7 +17,7 @@ describe("problem model", () => { title: "test", subtitle: "", sectionsFromSnapshot: [], - exemplarPaths: [], + exemplars: [], }); expect(problem.fullTitle).toBe("test"); }); @@ -50,7 +50,7 @@ describe("problem model", () => { } ], config: {}, - exemplarPaths: [] + exemplars: [] }); expect(problem.fullTitle).toBe("test: sub"); }); @@ -219,7 +219,7 @@ describe("problem model", () => { disabledFeatures: ["foo"], settings: { foo: "bar" } }, - exemplarPaths: [] + exemplars: [] }); }); @@ -242,7 +242,7 @@ describe("problem model", () => { disabledFeatures: ["foo"], settings: { foo: "bar" } }, - exemplarPaths: [] + exemplars: [] }); }); @@ -267,7 +267,7 @@ describe("problem model", () => { disabledFeatures: ["foo"], settings: { foo: "bar" } }, - exemplarPaths: [] + exemplars: [] }); }); }); From 3baf515602415061a771aca4252e6a04cb7a122d Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Mon, 11 Mar 2024 09:51:47 -0700 Subject: [PATCH 45/78] Push Scott's changes for numberline-tile refactor --- .../numberline/components/numberline-tile.tsx | 51 +++++-------------- .../components/numberline-toolbar-context.ts | 6 +-- .../numberline-toolbar-registration.tsx | 14 ++--- 3 files changed, 22 insertions(+), 49 deletions(-) diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index 84b4028e6f..fbfc129076 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -25,7 +25,7 @@ import "./numberline-toolbar-registration"; import "./numberline-tile.scss"; -export enum ToolbarOption { +export enum CreatePointType { Selection = "selection", Filled = "filled", Open = "open" @@ -42,25 +42,16 @@ export const NumberlineTile: React.FC = observer(function Numberline /* ========================== [ Determine Point is Open or Filled ] ========================= */ - const [toolbarOption, setToolbarOption] = useState(ToolbarOption.Selection); //"selection" + const toolbarOptionRef = useRef(CreatePointType.Selection); //"selection" - const handleCreatePointType = (_isOpen: ToolbarOption) => { - console.log("--------------------------------------"); - console.log("initial STATE:", toolbarOption); - setToolbarOption(_isOpen); + const handleCreatePointType = (pointType: CreatePointType) => { + toolbarOptionRef.current = pointType; }; - useEffect(() => { - console.log("Updated STATE:", toolbarOption); - }, [toolbarOption]); - /* ============================ [ Model Manipulation Functions ] ============================ */ const createPoint = (xValue: number, _pointTypeIsOpen: boolean) => { - const pointType = (_pointTypeIsOpen) ? "OPEN" : "FILLED"; - // console.log("createPoint with xValue", xValue, "pointTypeisOpen:", pointType); if (!readOnly) { const point = content.createAndSelectPoint(xValue, _pointTypeIsOpen); - console.log("pointCreated:", point.id, "-----", pointType); setHoverPointId(point.id); } }; @@ -142,7 +133,6 @@ export const NumberlineTile: React.FC = observer(function Numberline top: y - outerPointRadius, width: 2 * outerPointRadius }; - console.log("boundingBox:", boundingBox); return boundingBox; } }, [annotationPointCenter]); @@ -205,7 +195,7 @@ export const NumberlineTile: React.FC = observer(function Numberline return isBetweenYBounds && isBetweenXBounds; }; - const handleMouseClick = (e: Event, _toolbarOption: ToolbarOption) => { + const handleMouseClick = (e: Event, _toolbarOption: CreatePointType) => { if (!readOnly){ if (hoverPointId) { const hoverPoint = content.getPoint(hoverPointId); @@ -218,9 +208,8 @@ export const NumberlineTile: React.FC = observer(function Numberline // and toolbarOption is either filled or open const [mouseX, mouseY] = mousePos(e); if (mouseInBoundingBox(mouseX, mouseY)) { - console.log("inside bounding box!!!"); - if(_toolbarOption !== ToolbarOption.Selection){ - const isPointOpen = _toolbarOption === ToolbarOption.Open; + if(_toolbarOption !== CreatePointType.Selection){ + const isPointOpen = _toolbarOption === CreatePointType.Open; createPoint(xScale.invert(mouseX), isPointOpen); } } @@ -245,7 +234,7 @@ export const NumberlineTile: React.FC = observer(function Numberline const drawMouseFollowPoint = (mouseX: number) => { // When in selection mode - do not draw any hover circle - if (toolbarOption === ToolbarOption.Selection) { + if (toolbarOptionRef.current === CreatePointType.Selection) { clearMouseFollowPoint(); return; } @@ -258,7 +247,7 @@ export const NumberlineTile: React.FC = observer(function Numberline .classed("point-inner-circle", true); //For open mode - draw inner white circle - if (toolbarOption === ToolbarOption.Open) { + if (toolbarOptionRef.current === CreatePointType.Open) { svg.append("circle") .attr("fill", "white") .attr("cx", mouseX) @@ -283,7 +272,7 @@ export const NumberlineTile: React.FC = observer(function Numberline } }; - svg.on("click", (e) => handleMouseClick(e, toolbarOption)); + svg.on("click", (e) => handleMouseClick(e, toolbarOptionRef.current)); svg.on("mousemove", (e) => handleMouseMove(e)); // * ================================ [ Construct Numberline ] =============================== */ @@ -364,7 +353,6 @@ export const NumberlineTile: React.FC = observer(function Numberline outerPoints.enter() .append("circle").attr("class", "outer-point") .attr('cx', (p) => { - // console.log("line 364:", p); return xScale(p.currentXValue); }) //mapped to axis width .attr('cy', yMidPoint).attr('r', outerPointRadius).attr('id', p => p.id) @@ -414,18 +402,6 @@ export const NumberlineTile: React.FC = observer(function Numberline const innerWhitePoints = svg.selectAll('.circle,.inner-white-point') .data(openPoints); - // content.pointsArr.forEach(p => { - // if(p.isOpen) { - // svg.append("circle") - // .attr("class", "inner-white-point") - // .attr("cx", xScale(p.currentXValue)) - // .attr("cy", yMidPoint) - // .attr("r", innerPointRadius * 0.5) - // .attr("fill", "white") - // .attr('id', `inner-white-${p.id}`); - // } - // }); - innerWhitePoints.enter() .append("circle") .attr("class", "inner-white-point") @@ -435,15 +411,12 @@ export const NumberlineTile: React.FC = observer(function Numberline .attr("fill", "white") .attr('id', (p) => `inner-white-${p.id}`); - // Update selection: Update circles' position if necessary + // Update circle positions innerWhitePoints .attr("cx", (p) => xScale(p.currentXValue)) .attr("cy", yMidPoint); - // Exit selection: Remove circles for data that's no longer present innerWhitePoints.exit().remove(); - - }; //end of updateCircles() updateCircles(); @@ -455,7 +428,7 @@ export const NumberlineTile: React.FC = observer(function Numberline handleResetPoints: () => content.deleteAllPoints(), handleDeletePoint: deleteSelectedPoints, handleCreatePointType, - toolbarOption + toolbarOption: toolbarOptionRef.current }; return ( diff --git a/src/plugins/numberline/components/numberline-toolbar-context.ts b/src/plugins/numberline/components/numberline-toolbar-context.ts index 47cd80dbd4..a996ee3f33 100644 --- a/src/plugins/numberline/components/numberline-toolbar-context.ts +++ b/src/plugins/numberline/components/numberline-toolbar-context.ts @@ -1,11 +1,11 @@ import { createContext } from "react"; -import { ToolbarOption } from "./numberline-tile"; +import { CreatePointType } from "./numberline-tile"; export interface INumberlineToolbarContext { handleResetPoints: () => void; handleDeletePoint: () => void; - handleCreatePointType: (isOpen: ToolbarOption) => void; - toolbarOption: ToolbarOption; + handleCreatePointType: (isOpen: CreatePointType) => void; + toolbarOption: CreatePointType; } export const NumberlineToolbarContext = createContext(null); diff --git a/src/plugins/numberline/components/numberline-toolbar-registration.tsx b/src/plugins/numberline/components/numberline-toolbar-registration.tsx index 28b5d31244..74a42aa1c3 100644 --- a/src/plugins/numberline/components/numberline-toolbar-registration.tsx +++ b/src/plugins/numberline/components/numberline-toolbar-registration.tsx @@ -8,16 +8,16 @@ import PointIcon from "../assets/numberline-toolbar-point-icon.svg"; import PointOpenIcon from "../assets/numberline-toolbar-point-open-icon.svg"; import ResetIcon from "../assets/numberline-toolbar-reset-icon.svg"; import DeleteIcon from "../assets/numberline-toolbar-delete-icon.svg"; -import { ToolbarOption } from "./numberline-tile"; +import { CreatePointType } from "./numberline-tile"; const SelectButton = ({name}: IToolbarButtonComponentProps) => { const context = useContext(NumberlineToolbarContext); - const selected = context?.toolbarOption === ToolbarOption.Selection; + const selected = context?.toolbarOption === CreatePointType.Selection; function handleClick() { if (context) { - context.handleCreatePointType(ToolbarOption.Selection); + context.handleCreatePointType(CreatePointType.Selection); } } @@ -37,11 +37,11 @@ const SelectButton = ({name}: IToolbarButtonComponentProps) => { const PointButton = ({name}: IToolbarButtonComponentProps) => { const context = useContext(NumberlineToolbarContext); - const selected = context?.toolbarOption === ToolbarOption.Filled; + const selected = context?.toolbarOption === CreatePointType.Filled; function handleClick() { if (context) { - context.handleCreatePointType(ToolbarOption.Filled); + context.handleCreatePointType(CreatePointType.Filled); } } @@ -61,11 +61,11 @@ const PointButton = ({name}: IToolbarButtonComponentProps) => { const PointOpenButton = ({name}: IToolbarButtonComponentProps) => { const context = useContext(NumberlineToolbarContext); - const selected = context?.toolbarOption === ToolbarOption.Open; + const selected = context?.toolbarOption === CreatePointType.Open; function handleClick() { if (context) { - context.handleCreatePointType(ToolbarOption.Open); + context.handleCreatePointType(CreatePointType.Open); } } From 102e1a654490f0cb2e022a2091eed5f68484acaa Mon Sep 17 00:00:00 2001 From: Joe Bacal Date: Mon, 11 Mar 2024 13:10:07 -0400 Subject: [PATCH 46/78] use constant for Exemplar test --- src/hooks/use-document-caption.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hooks/use-document-caption.tsx b/src/hooks/use-document-caption.tsx index 492570a320..d7e9552031 100644 --- a/src/hooks/use-document-caption.tsx +++ b/src/hooks/use-document-caption.tsx @@ -1,7 +1,7 @@ import { useFirestoreTeacher } from "./firestore-hooks"; import { useAppConfig, useClassStore, useProblemStore, useUserStore } from "./use-stores"; import { DocumentModelType } from "../models/document/document"; -import { isPublishedType, isUnpublishedType } from "../models/document/document-types"; +import { ExemplarDocument, isPublishedType, isUnpublishedType } from "../models/document/document-types"; import { getDocumentDisplayTitle } from "../models/document/document-utils"; export function useDocumentCaption(document: DocumentModelType, isStudentWorkspaceDoc?: boolean) { @@ -10,7 +10,6 @@ export function useDocumentCaption(document: DocumentModelType, isStudentWorkspa const classStore = useClassStore(); const user = useUserStore(); const { type, uid } = document; - console.log("type:", type); const pubVersion = document.pubVersion; const teacher = useFirestoreTeacher(uid, user.network || ""); const userName = classStore.getUserById(uid)?.displayName @@ -21,7 +20,8 @@ export function useDocumentCaption(document: DocumentModelType, isStudentWorkspa const hasNamePrefix = document.isRemote || isPublishedType(type) || isUnpublishedType(type) - || type === "exemplar" + // TODO: ExemplarDocument could be in "isPublishedType" or "isUnpiblishedType" test arrays + || type === ExemplarDocument || isStudentWorkspaceDoc; const namePrefix = hasNamePrefix ? `${userName}: ` : ""; const dateSuffix = document.isRemote && document.createdAt From 8d897e1e1f520a493adb0aab06b197e49464d2b5 Mon Sep 17 00:00:00 2001 From: Joe Bacal Date: Mon, 11 Mar 2024 13:53:31 -0400 Subject: [PATCH 47/78] refactor identifying document with and without tags for sort work --- src/models/stores/sorted-documents.ts | 37 +++++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index be8f435c31..f7536dc747 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -41,7 +41,7 @@ interface IMatchPropertiesOptions { } export class SortedDocuments { stores: ISortedDocumentsStores; - tempTagDocumentMap = new Map>(); + firestoreTagDocumentMap = new Map>(); constructor(stores: ISortedDocumentsStores) { makeAutoObservable(this); @@ -156,33 +156,36 @@ export class SortedDocuments { // Find all unique document keys in tagsWithDocs. Compare this with all sortable documents // in store to find "Documents with no comments" then place those doc keys to "Not Tagged" - const uniqueDocKeysWithComments = new Set(); + const uniqueDocKeysWithTags = new Set(); - this.tempTagDocumentMap.forEach((docKeysSet, tag) => { - docKeysSet.forEach((docKey: string) =>{ - uniqueDocKeysWithComments.add(docKey); - }); + // grouping documents based on firestore comment tags + this.firestoreTagDocumentMap.forEach((docKeysSet, tag) => { const docKeysArray = Array.from(docKeysSet); // Convert the Set to an array if (tagsWithDocs[tag]) { + docKeysSet.forEach((docKey: string) =>{ + uniqueDocKeysWithTags.add(docKey); + }); tagsWithDocs[tag].docKeysFoundWithTag = docKeysArray; } }); + // adding in (exemplar) documents with authored tags const allSortableDocKeys = this.filteredDocsByType; allSortableDocKeys.forEach(doc => { - if (!uniqueDocKeysWithComments.has(doc.key)) { - // This document has no comments - if (tagsWithDocs[""]) { - tagsWithDocs[""].docKeysFoundWithTag.push(doc.key); - } - } const foundTagKey = doc.getProperty("authoredCommentTag"); if (foundTagKey !== undefined && foundTagKey !== "") { if (tagsWithDocs[foundTagKey]) { tagsWithDocs[foundTagKey].docKeysFoundWithTag.push(doc.key); - // if tagsWithDocs[""] has this doc.key, remove it - const index = tagsWithDocs[""].docKeysFoundWithTag.indexOf(doc.key); - if (index > -1) tagsWithDocs[""].docKeysFoundWithTag.splice(index, 1); + uniqueDocKeysWithTags.add(doc.key); + } + } + }); + + allSortableDocKeys.forEach(doc => { + if (!uniqueDocKeysWithTags.has(doc.key)) { + // This document has no comments + if (tagsWithDocs[""]) { + tagsWithDocs[""].docKeysFoundWithTag.push(doc.key); } } }); @@ -214,10 +217,10 @@ export class SortedDocuments { const commentData = commentDoc.data(); if (commentData?.tags) { commentData.tags.forEach((tag: string) => { - let docKeysSet = this.tempTagDocumentMap.get(tag); + let docKeysSet = this.firestoreTagDocumentMap.get(tag); if (!docKeysSet) { docKeysSet = new ObservableSet(); - this.tempTagDocumentMap.set(tag, docKeysSet); + this.firestoreTagDocumentMap.set(tag, docKeysSet); } docKeysSet.add(doc.key); }); From 2642a608ae0a1441576fb64036c729105c4d485b Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Mon, 11 Mar 2024 14:49:56 -0700 Subject: [PATCH 48/78] Refactored back to useState instead of useRef --- .../numberline/components/numberline-tile.tsx | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index fbfc129076..805f6eac33 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -42,10 +42,10 @@ export const NumberlineTile: React.FC = observer(function Numberline /* ========================== [ Determine Point is Open or Filled ] ========================= */ - const toolbarOptionRef = useRef(CreatePointType.Selection); //"selection" + const [toolbarOption, settoolbarOption] = useState(CreatePointType.Selection); //"selection" const handleCreatePointType = (pointType: CreatePointType) => { - toolbarOptionRef.current = pointType; + settoolbarOption(pointType); }; /* ============================ [ Model Manipulation Functions ] ============================ */ @@ -195,7 +195,7 @@ export const NumberlineTile: React.FC = observer(function Numberline return isBetweenYBounds && isBetweenXBounds; }; - const handleMouseClick = (e: Event, _toolbarOption: CreatePointType) => { + const handleMouseClick = (e: Event, optionClicked: CreatePointType) => { if (!readOnly){ if (hoverPointId) { const hoverPoint = content.getPoint(hoverPointId); @@ -208,8 +208,8 @@ export const NumberlineTile: React.FC = observer(function Numberline // and toolbarOption is either filled or open const [mouseX, mouseY] = mousePos(e); if (mouseInBoundingBox(mouseX, mouseY)) { - if(_toolbarOption !== CreatePointType.Selection){ - const isPointOpen = _toolbarOption === CreatePointType.Open; + if(optionClicked !== CreatePointType.Selection){ + const isPointOpen = optionClicked === CreatePointType.Open; createPoint(xScale.invert(mouseX), isPointOpen); } } @@ -234,7 +234,7 @@ export const NumberlineTile: React.FC = observer(function Numberline const drawMouseFollowPoint = (mouseX: number) => { // When in selection mode - do not draw any hover circle - if (toolbarOptionRef.current === CreatePointType.Selection) { + if (toolbarOption === CreatePointType.Selection) { clearMouseFollowPoint(); return; } @@ -247,7 +247,7 @@ export const NumberlineTile: React.FC = observer(function Numberline .classed("point-inner-circle", true); //For open mode - draw inner white circle - if (toolbarOptionRef.current === CreatePointType.Open) { + if (toolbarOption === CreatePointType.Open) { svg.append("circle") .attr("fill", "white") .attr("cx", mouseX) @@ -272,7 +272,7 @@ export const NumberlineTile: React.FC = observer(function Numberline } }; - svg.on("click", (e) => handleMouseClick(e, toolbarOptionRef.current)); + svg.on("click", (e) => handleMouseClick(e, toolbarOption)); svg.on("mousemove", (e) => handleMouseMove(e)); // * ================================ [ Construct Numberline ] =============================== */ @@ -422,13 +422,12 @@ export const NumberlineTile: React.FC = observer(function Numberline updateCircles(); } - // * ================================= [ Register Toolbar ] ================================== */ const toolbarFunctions: INumberlineToolbarContext = { handleResetPoints: () => content.deleteAllPoints(), handleDeletePoint: deleteSelectedPoints, handleCreatePointType, - toolbarOption: toolbarOptionRef.current + toolbarOption }; return ( From 9c16d7d7df3fbf9e78661adcd4cbdd6ba0ef8e5a Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Mon, 11 Mar 2024 15:00:54 -0700 Subject: [PATCH 49/78] Push Cypress fixes --- .../e2e/functional/document_tests/copy_doc_test_spec.js | 7 ++++--- .../document_tests/student_teacher_4up_readonly_spec.js | 9 ++++++--- .../functional/document_tests/tiles_copy_test_spec.js | 1 + .../e2e/functional/tile_tests/arrow_annotation_spec.js | 1 + 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/cypress/e2e/functional/document_tests/copy_doc_test_spec.js b/cypress/e2e/functional/document_tests/copy_doc_test_spec.js index a27946c160..6df1478a67 100644 --- a/cypress/e2e/functional/document_tests/copy_doc_test_spec.js +++ b/cypress/e2e/functional/document_tests/copy_doc_test_spec.js @@ -76,6 +76,7 @@ context('Copy Document', () => { cy.log("Add number line tile"); clueCanvas.addTile("numberline"); + numberlineTile.setToolbarPoint(); //click Point in order to add points to numberline numberlineTile.addPointOnNumberlineTick(-4.0); numberlineTile.addPointOnNumberlineTick(2.0); numberlineTile.getPointsOnGraph().should('have.length', 2); @@ -124,7 +125,7 @@ context('Copy Document', () => { simulatorTile.getSimulatorTile().should("contain.text", "Surface Pressure Sensor"); simulatorTile.getSimulatorTile().should("contain.text", "Temperature Sensor"); simulatorTile.getSimulatorTile().should("contain.text", "Gripper Output"); - + cy.log("Add Graph tool tile"); clueCanvas.addTile("graph"); xyTile.getTile().should('be.visible'); @@ -156,7 +157,7 @@ context('Copy Document', () => { numberlineTile.getNumberlineTile().scrollIntoView().should('be.visible'); numberlineTile.getPointsOnGraph().should('have.length', 2); - + imageTile.getImageTile().scrollIntoView().should('be.visible'); datacardTile.getTile().scrollIntoView().should('be.visible'); @@ -180,4 +181,4 @@ context('Copy Document', () => { xyTile.getGraphDot().should('have.length.greaterThan', 3); }); }); - + diff --git a/cypress/e2e/functional/document_tests/student_teacher_4up_readonly_spec.js b/cypress/e2e/functional/document_tests/student_teacher_4up_readonly_spec.js index 020a59f665..b3cd7d51b2 100644 --- a/cypress/e2e/functional/document_tests/student_teacher_4up_readonly_spec.js +++ b/cypress/e2e/functional/document_tests/student_teacher_4up_readonly_spec.js @@ -23,7 +23,7 @@ const simulatorTile = new SimulatorTile; let dashboard = new TeacherDashboard(); let students = [15, 16]; -const teacherUrl = `${Cypress.config("qaUnitTeacher6")}&mouseSensor` +const teacherUrl = `${Cypress.config("qaUnitTeacher6")}&mouseSensor`; function testTilesNotReadOnly(tab, position) { cy.get('.'+tab).find(position + ' .text-tool').should('not.have.class', 'read-only'); @@ -96,10 +96,13 @@ function setupTest(studentIndex) { drawToolTile.getRectangleDrawing().should("exist").and("have.length", 1); clueCanvas.addTile("expression"); exp.getMathField().should("have.value", "a=\\pi r^2"); + clueCanvas.addTile("numberline"); + numberlineToolTile.setToolbarPoint(); //click Point in order to add points to numberline numberlineToolTile.addPointOnNumberlineTick(-4.0); numberlineToolTile.addPointOnNumberlineTick(2.0); numberlineToolTile.getPointsOnGraph().should('have.length', 2); + const newName = "Image Tile"; clueCanvas.addTile('image'); cy.get('.primary-workspace .image-tool .editable-tile-title-text').first().should("contain", "Image 1"); @@ -143,7 +146,7 @@ function setupTestBrain(studentIndex) { context('Test 4-up and 1-up views tiles read only functionalities', function () { it('4-up and 1-up views read-only text, table, geometry, drawing, expression, numberline, image, datacard tiles', function () { - + cy.clearQAData('all'); setupTest(0); @@ -173,7 +176,7 @@ context('Test 4-up and 1-up views tiles read only functionalities', function () testTilesReadOnly("student-group-view", ".north-east"); }); it('4-up and 1-up views read-only dataflow, expression, xy plot tiles', function () { - + cy.clearQAData('all'); setupTestBrain(0); diff --git a/cypress/e2e/functional/document_tests/tiles_copy_test_spec.js b/cypress/e2e/functional/document_tests/tiles_copy_test_spec.js index 22fd3bd2f0..9129f79414 100644 --- a/cypress/e2e/functional/document_tests/tiles_copy_test_spec.js +++ b/cypress/e2e/functional/document_tests/tiles_copy_test_spec.js @@ -161,6 +161,7 @@ context('Test copy tiles from one document to other document', function () { cy.log("Add number line tile"); clueCanvas.addTile("numberline"); + numberlineToolTile.setToolbarPoint(); //click Point in order to add points to numberline numberlineToolTile.addPointOnNumberlineTick(-4.0); numberlineToolTile.addPointOnNumberlineTick(2.0); numberlineToolTile.getPointsOnGraph().should('have.length', 2); diff --git a/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js b/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js index 5cd9af1748..75fa592b26 100644 --- a/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js +++ b/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js @@ -230,6 +230,7 @@ context('Arrow Annotations (Sparrows)', function () { aa.getAnnotationLayer().should("have.class", "editing"); aa.getAnnotationButtons().should("not.exist"); aa.clickArrowToolbarButton(); + numberlineToolTile.setToolbarPoint(); //click Point in order to add points to numberline numberlineToolTile.addPointOnNumberlineTick(-4); numberlineToolTile.addPointOnNumberlineTick(2); aa.clickArrowToolbarButton(); From c52a5c3c8191d1feb8ad9eafe58313a20e32114c Mon Sep 17 00:00:00 2001 From: Dennis Cao Date: Mon, 11 Mar 2024 17:09:38 -0700 Subject: [PATCH 50/78] Fixed Cypress arrow_annotation_spec.js --- .../tile_tests/arrow_annotation_spec.js | 16 +++++++++------- .../support/elements/tile/NumberlineToolTile.js | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js b/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js index 75fa592b26..98a1336e38 100644 --- a/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js +++ b/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js @@ -221,22 +221,24 @@ context('Arrow Annotations (Sparrows)', function () { aa.getAnnotationArrows().should("have.length", 1); }); - it("can add arrows to numberline tiles", () => { + it("annotations buttons don't exist when empty numberline", () => { beforeTest(queryParams); clueCanvas.addTile("numberline"); - - cy.log("Annotation buttons appear for points"); aa.clickArrowToolbarButton(); aa.getAnnotationLayer().should("have.class", "editing"); aa.getAnnotationButtons().should("not.exist"); aa.clickArrowToolbarButton(); + }); + + it("can add annotations to numberline tiles", () => { + beforeTest(queryParams); + clueCanvas.addTile("numberline"); numberlineToolTile.setToolbarPoint(); //click Point in order to add points to numberline - numberlineToolTile.addPointOnNumberlineTick(-4); - numberlineToolTile.addPointOnNumberlineTick(2); + numberlineToolTile.addPointOnNumberlineTick(-4.0); + numberlineToolTile.addPointOnNumberlineTick(2.0); aa.clickArrowToolbarButton(); + aa.getAnnotationButtons().should("exist"); aa.getAnnotationButtons().should("have.length", 2); - - cy.log("Can add an arrow to numberline points"); aa.getAnnotationArrows().should("not.exist"); aa.getAnnotationButtons().eq(1).click(); aa.getAnnotationButtons().eq(0).click(); diff --git a/cypress/support/elements/tile/NumberlineToolTile.js b/cypress/support/elements/tile/NumberlineToolTile.js index deb759ecfb..bfe333ebbc 100644 --- a/cypress/support/elements/tile/NumberlineToolTile.js +++ b/cypress/support/elements/tile/NumberlineToolTile.js @@ -12,7 +12,7 @@ class NumberlineToolTile { return cy.get(`${workspaceClass || ".primary-workspace"} .editable-tile-title`); } addPointOnNumberlineTick(num){ - this.getNumberlineTick(num).parent().click(); //we need to click the element that is the parent of the text element + this.getNumberlineTick(num).parent().click({force: true}); //we need to click the element that is the parent of the text element } getNumberlineTick(num){ const tickIndex = numberlineVals.indexOf(num); From 497ae90e1279d887b736546610e4414542d36d04 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 12 Mar 2024 09:05:42 -0400 Subject: [PATCH 51/78] combine test for a single number line --- .../tile_tests/arrow_annotation_spec.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js b/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js index 98a1336e38..6ad98db4ae 100644 --- a/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js +++ b/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js @@ -221,22 +221,27 @@ context('Arrow Annotations (Sparrows)', function () { aa.getAnnotationArrows().should("have.length", 1); }); - it("annotations buttons don't exist when empty numberline", () => { + it.only("can add annotations to numberline tiles", () => { beforeTest(queryParams); clueCanvas.addTile("numberline"); + + cy.log("annotations buttons don't exist when empty numberline"); aa.clickArrowToolbarButton(); aa.getAnnotationLayer().should("have.class", "editing"); aa.getAnnotationButtons().should("not.exist"); + // Disable the annotation tool again so there isn't a layer on top + // of the numberline tile aa.clickArrowToolbarButton(); - }); - it("can add annotations to numberline tiles", () => { - beforeTest(queryParams); - clueCanvas.addTile("numberline"); - numberlineToolTile.setToolbarPoint(); //click Point in order to add points to numberline + cy.log("add points so we can add annotations"); + // Click on tile to get the tool bar to show up + numberlineToolTile.getNumberlineTile().click(); + // Switch to point adding mode + numberlineToolTile.setToolbarPoint(); numberlineToolTile.addPointOnNumberlineTick(-4.0); numberlineToolTile.addPointOnNumberlineTick(2.0); aa.clickArrowToolbarButton(); + aa.getAnnotationButtons().should("exist"); aa.getAnnotationButtons().should("have.length", 2); aa.getAnnotationArrows().should("not.exist"); From 762a61d2cd2c61c7079449e3dd77c897711fe443 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 12 Mar 2024 10:33:59 -0400 Subject: [PATCH 52/78] add tests for open points and delete points --- .../tile_tests/numberline_tool_spec.js | 18 ++++++++++--- .../elements/tile/NumberlineToolTile.js | 26 ++++++++++++------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/numberline_tool_spec.js b/cypress/e2e/functional/tile_tests/numberline_tool_spec.js index f658657e82..335ff2ba01 100644 --- a/cypress/e2e/functional/tile_tests/numberline_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/numberline_tool_spec.js @@ -27,22 +27,34 @@ context('Numberline Tile', function () { numberlineToolTile.getNumberlineTileTitle().type(newName + '{enter}'); numberlineToolTile.getNumberlineTileTitleText().should("contain", newName); - cy.log('will test adding points to a numberline'); + cy.log('will add closed points to a numberline'); numberlineToolTile.setToolbarPoint(); //click Point in order to add points to numberline numberlineToolTile.addPointOnNumberlineTick(-4.0); numberlineToolTile.addPointOnNumberlineTick(2.0); numberlineToolTile.getPointsOnGraph().should('have.length', 2); + cy.log('will add open points to a numberline'); + numberlineToolTile.setToolbarOpenPoint(); + numberlineToolTile.addPointOnNumberlineTick(-3.0); + numberlineToolTile.addPointOnNumberlineTick(1.0); + numberlineToolTile.getPointsOnGraph().should('have.length', 4); + cy.log('will change min and max value of numberline and recalculate ticks'); numberlineToolTile.setMaxValue(10); numberlineToolTile.getAllNumberlineTicks().should('contain', 8.5); numberlineToolTile.setMinValue(-10); numberlineToolTile.getAllNumberlineTicks().should('contain', 6.0); + cy.log("will delete a single point"); + numberlineToolTile.setToolbarSelect(); + // Just to select it + numberlineToolTile.addPointOnNumberlineTick(2.0); + numberlineToolTile.setToolbarDelete(); + numberlineToolTile.getPointsOnGraph().should('have.length', 3); + cy.log("will delete all points"); - cy.log("delete all points in the numberline"); numberlineToolTile.setToolbarReset(); - numberlineToolTile.getPointsOnGraph().should('have.length', 0); + numberlineToolTile.hasNoPoints(); //Numberline tile restore upon page reload cy.wait(2000); diff --git a/cypress/support/elements/tile/NumberlineToolTile.js b/cypress/support/elements/tile/NumberlineToolTile.js index bfe333ebbc..1fc1bd6f78 100644 --- a/cypress/support/elements/tile/NumberlineToolTile.js +++ b/cypress/support/elements/tile/NumberlineToolTile.js @@ -25,26 +25,34 @@ class NumberlineToolTile { //exclude the hovering circle that follows the mouse on the numberline return cy.get(".numberline-tool-container .point-inner-circle").not(".mouse-follow-point"); } - setToolbarReset(){ - this.getResetButton().click(); + hasNoPoints(){ + return cy.get(".numberline-tool-container").should("not.have.descendants", ".point-inner-circle:not(.mouse-follow-point)"); + } + setToolbarSelect(){ + cy.get(".numberline-toolbar .select").click(); } setToolbarPoint(){ - this.getPointButton().click(); + cy.get(".numberline-toolbar .point").click(); } - getResetButton(){ - return cy.get(".reset"); + setToolbarOpenPoint(){ + cy.get(".numberline-toolbar .point-open").click(); + } + setToolbarReset(){ + cy.get(".numberline-toolbar .reset").click(); } - getPointButton(){ - return cy.get(".point"); + setToolbarDelete(){ + cy.get(".numberline-toolbar .delete").click(); } setMaxValue(max){ - this.getMaxBox().dblclick().clear().type(`${max}{enter}`); + this.getMaxBox().dblclick(); + this.getMaxBox().clear().type(`${max}{enter}`); } getMaxBox(){ return cy.get(".numberline-tool-container .border-box").eq(1); //second element is max } setMinValue(min){ - this.getMinBox().dblclick().clear().type(`${min}{enter}`); + this.getMinBox().dblclick(); + this.getMinBox().clear().type(`${min}{enter}`); } getMinBox(){ return cy.get(".numberline-tool-container .border-box").eq(0); //first element is min From 82c4d866993237e685d59cc3adcad83a2886e358 Mon Sep 17 00:00:00 2001 From: Joe Bacal Date: Wed, 13 Mar 2024 20:36:03 -0400 Subject: [PATCH 53/78] remove exemplars from problem snapshot preprocessor --- src/models/curriculum/problem.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/models/curriculum/problem.ts b/src/models/curriculum/problem.ts index 20ebf05e99..1c185f87fd 100644 --- a/src/models/curriculum/problem.ts +++ b/src/models/curriculum/problem.ts @@ -117,16 +117,13 @@ const isAmbiguousSnapshot = (sn: ModernProblemSnapshot | LegacyProblemSnapshot) export const ProblemModel = types.snapshotProcessor(ModernProblemModel, { preProcessor(sn: ModernProblemSnapshot | LegacyProblemSnapshot) { - const { sections, exemplars, ...nonSectionProps } = sn as any; + const { sections, ...nonSectionProps } = sn as any; // Move sections to sectionsFromSnapshot so we load them into a volatile property `sections` - // creating a exemplarsFromSnapshot that will be passed in similar way, - // but no volatile property yet const sectionsFromSnapshot = sections || []; if (isLegacySnapshot(sn)) { const { disabled: disabledFeatures, settings, ...others } = sn; return { ...others, sectionsFromSnapshot, - exemplars, config: { disabledFeatures, settings } } as ModernProblemSnapshot; } @@ -135,11 +132,10 @@ export const ProblemModel = types.snapshotProcessor(ModernProblemModel, { return { ...others, sectionsFromSnapshot, - exemplars, config: { disabledFeatures, settings, ...config } } as ModernProblemSnapshot; } - return { ...nonSectionProps, sectionsFromSnapshot, exemplars }; + return { ...nonSectionProps, sectionsFromSnapshot }; } }); export interface ProblemModelType extends Instance {} From d05f0dda667d0ce52aaaa9975ecf81ce82bf89ee Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 15 Mar 2024 11:13:03 -0400 Subject: [PATCH 54/78] Improve autoscaling for variables --- .../plotted-variables-adornment-component.tsx | 103 ++++++++++-------- 1 file changed, 56 insertions(+), 47 deletions(-) diff --git a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx index 9ff716f306..5f216ae2ba 100644 --- a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx +++ b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useRef } from "react"; +import React, { useCallback, useContext, useEffect, useMemo, useRef } from "react"; import { drag, select, Selection } from "d3"; import { observer } from "mobx-react-lite"; import { VariableType } from "@concord-consortium/diagram-view"; @@ -7,17 +7,18 @@ import { useTileModelContext } from "../../../../components/tiles/hooks/use-tile import { getSharedModelManager } from "../../../../models/tiles/tile-environment"; import { mstAutorun } from "../../../../utilities/mst-autorun"; import { mstReaction } from "../../../../utilities/mst-reaction"; -import { useDataConfigurationContext } from "../../../graph/hooks/use-data-configuration-context"; import { useGraphModelContext } from "../../../graph/hooks/use-graph-model-context"; import { ScaleNumericBaseType } from "../../../graph/imports/components/axis/axis-types"; import { useAxisLayoutContext } from "../../../graph/imports/components/axis/models/axis-layout-context"; -import { IAxisModel, INumericAxisModel } from "../../../graph/imports/components/axis/models/axis-model"; -import { curveBasis, setNiceDomain } from "../../../graph/utilities/graph-utils"; +import { INumericAxisModel } from "../../../graph/imports/components/axis/models/axis-model"; +import { curveBasis } from "../../../graph/utilities/graph-utils"; import { SharedVariables } from "../../shared-variables"; import { IPlottedVariablesAdornmentModel } from "./plotted-variables-adornment-model"; import { useReadOnlyContext } from "../../../../components/document/read-only-context"; import { isFiniteNumber } from "../../../../utilities/math-utils"; import { useUIStore } from "../../../../hooks/use-stores"; +import { useGraphSettingsContext } from "../../../graph/hooks/use-graph-settings-context"; +import { GraphControllerContext } from "../../../graph/models/graph-controller"; import "../../../graph/adornments/plotted-function/plotted-function-adornment-component.scss"; import "./plotted-variables.scss"; @@ -36,23 +37,17 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria const {model, cellKey = {}, plotWidth, plotHeight, xAxis, yAxis} = props; const { tile } = useTileModelContext(); const graphModel = useGraphModelContext(); - const dataConfig = useDataConfigurationContext(); const readOnly = useReadOnlyContext(); const layout = useAxisLayoutContext(); const ui = useUIStore(); - const isTileSelected = !!tile && ui.isSelectedTile(tile); + const settings = useGraphSettingsContext(); + const controller = useContext(GraphControllerContext); const xScale = layout.getAxisScale("bottom") as ScaleNumericBaseType; const yScale = layout.getAxisScale("left") as ScaleNumericBaseType; - const xAttrType = dataConfig?.attributeType("x"); - const yAttrType = dataConfig?.attributeType("y"); const xSubAxesCount = layout.getAxisMultiScale("bottom")?.repetitions ?? 1; const ySubAxesCount = layout.getAxisMultiScale("left")?.repetitions ?? 1; - const xCatSet = layout.getAxisMultiScale("bottom")?.categorySet; - const xCats = xAttrType === "categorical" && xCatSet ? Array.from(xCatSet.values) : [""]; - const yCatSet = layout.getAxisMultiScale("left")?.categorySet; - const yCats = yAttrType === "categorical" && yCatSet ? Array.from(yCatSet.values) : [""]; - const xCellCount = xCats.length * xSubAxesCount; - const yCellCount = yCats.length * ySubAxesCount; + const xCellCount = xSubAxesCount; + const yCellCount = ySubAxesCount; const classFromKey = model.classNameFromKey(cellKey); const plottedFunctionRef = useRef(null); const smm = getSharedModelManager(graphModel); @@ -62,7 +57,8 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria const offsetFromPoint = 14; const highlightStrokeWidth = 5; const labelRectHeight = textHeight + 2 * padding; - const numberFormatter = Intl.NumberFormat(undefined, { maximumFractionDigits: 4, useGrouping: false}); + const numberFormatter + = useMemo(() => Intl.NumberFormat(undefined, { maximumFractionDigits: 4, useGrouping: false}), []); // Set the positions of the point-related SVG objects and the contents of the label when the variable value changes. const positionPointMarkers = useCallback((xValue: number, yValue: number, @@ -105,6 +101,7 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria const tPixelMin = xScale(xMin); const tPixelMax = xScale(xMax); const kPixelGap = 1; + const isTileSelected = !!tile && ui.isSelectedTile(tile); for (const instanceKey of model.plottedVariables.keys()) { const plottedVar = model.plottedVariables.get(instanceKey); const variable = plottedVar && plottedVar.xVariableId && sharedVariables?.getVariableById(plottedVar.xVariableId); @@ -194,8 +191,8 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria } } } - }, [xScale, model, xCellCount, yCellCount, yScale, graphModel, - labelRectHeight, positionPointMarkers, readOnly, sharedVariables, isTileSelected, setVariableValue]); + }, [tile, ui, model, graphModel, xScale, xCellCount, yScale, yCellCount, + labelRectHeight, positionPointMarkers, readOnly, sharedVariables, setVariableValue]); // Add the lines and their associated covers and labels const refreshValues = useCallback(() => { @@ -217,24 +214,46 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria }, { name: "PlottedVariablesAdornmentComponent.refreshExpressionChange" }, model); }, [graphModel, model, xScale, xSubAxesCount, yScale]); - // Refresh values on axis or expression change + // Refresh display when axis domains change useEffect(function refreshAxisChange() { - return mstAutorun(() => { - // We observe changes to the axis domains within the autorun by extracting them from the axes below. - // We do this instead of including domains in the useEffect dependency array to prevent domain changes - // from triggering a reinstall of the autorun. - if (xAxis && yAxis) { - const { domain: xDomain } = xAxis; // eslint-disable-line unused-imports/no-unused-vars - const { domain: yDomain } = yAxis; // eslint-disable-line unused-imports/no-unused-vars - } - // Trigger an autorun if any inputs or the expression of y change, or if the x variable changes - Array.from(model.plottedVariables.values()).forEach(plottedVariables => { - plottedVariables.yVariable?.computedValueIncludingMessageAndError; // eslint-disable-line no-unused-expressions - plottedVariables.xVariable; // eslint-disable-line no-unused-expressions - }); - refreshValues(); - }, { name: "PlottedVariablesAdornmentComponent.refreshAxisChange" }, model); - }, [dataConfig, model, plotWidth, plotHeight, sharedVariables, xAxis, yAxis, refreshValues]); + return mstReaction( + () => { + if (xAxis && yAxis) { + const { domain: xDomain } = xAxis; + const { domain: yDomain } = yAxis; + // Return a primitive value (string, not array) so that equality will work + // and effect will only be called when these numbers actually change. + return `${xDomain[0]}, ${xDomain[1]}, ${yDomain[0]}, ${yDomain[1]}`; + } + return ''; + }, + (limits) => { + refreshValues(); + }, + { fireImmediately: true, name: "PlottedVariablesAdornmentComponent.refreshAxisChange" }, + model); + }, [model, plotWidth, plotHeight, xAxis, yAxis, refreshValues, controller]); + + // If desired, rescale graph when variable values change + useEffect(function rescaleOnValuesChange() { + return mstReaction( + () => { + return Array.from(model.plottedVariables.values()).map(plottedVariables => { + return [ + plottedVariables.xVariable, + plottedVariables.yVariable?.computedValueIncludingMessageAndError + ]; + }); + }, + (any) => { + if (settings.scalePlotOnValueChange && !graphModel.lockAxes) { + controller?.autoscaleAllAxes(); + } + }, + { name: "PlottedVariablesAdornmentComponent.rescaleOnValuesChange" }, + model); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [controller, model]); // Scale graph when a new X or Y variable is selected useEffect(function scaleOnVariableChange() { @@ -242,23 +261,13 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria return Array.from(model.plottedVariables.values()).map((pvi) => [pvi.xVariableId, pvi.yVariableId]); }, (varlist) => { - if (graphModel.lockAxes) return; - // Set a range that includes 0 to 2x for all the given values. - function fitValues(values: number[], axis: IAxisModel) { - if (values.length) { - setNiceDomain([0, ...values.map(x => 2 * x)], axis); - } - } - - const variableValues = model.variableValues; - if (xAxis && yAxis) { - fitValues(variableValues.x, xAxis); - fitValues(variableValues.y, yAxis); + if (!graphModel.lockAxes) { + controller?.autoscaleAllAxes(); } }, { name: "PlottedVariablesAdornmentComponent.scaleOnVariableChange" }, model); - }, [graphModel.lockAxes, model, xAxis, yAxis]); + }, [graphModel.lockAxes, model, xAxis, yAxis, controller]); return ( From 51884023ca72ac9b3c68f9f47c48b0eb16e6ac25 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 15 Mar 2024 12:25:32 -0400 Subject: [PATCH 55/78] Tweak setDomain so that very narrow regions don't get truncated --- .../graph/imports/components/axis/models/axis-model.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plugins/graph/imports/components/axis/models/axis-model.ts b/src/plugins/graph/imports/components/axis/models/axis-model.ts index ecfebd72fd..e5f50de094 100644 --- a/src/plugins/graph/imports/components/axis/models/axis-model.ts +++ b/src/plugins/graph/imports/components/axis/models/axis-model.ts @@ -100,8 +100,9 @@ export const NumericAxisModel = AxisModel } else if ((min < 0) && (Math.abs(max) < Math.abs(min / snapFactor))) { max = 0; } - self.min = parseFloat(min.toFixed(2)); - self.max = parseFloat(max.toFixed(2)); + // Truncate to 2 decimal digits, but without making the range smaller + self.min = Math.floor(min*100)/100; + self.max = Math.ceil(max*100)/100; } })); export interface INumericAxisModel extends Instance {} From c5cf999713436ebbf0172dfb6996656c6a1249f8 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 15 Mar 2024 12:27:15 -0400 Subject: [PATCH 56/78] Use the 'fake point in the middle' to autoscale variables without values. --- .../plotted-variables-adornment-model.ts | 54 +++++++++++++++---- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-model.ts b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-model.ts index a3026b509e..1829f98815 100644 --- a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-model.ts +++ b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-model.ts @@ -1,4 +1,4 @@ -import { destroy, getSnapshot, IAnyStateTreeNode, Instance, types } from "mobx-state-tree"; +import { destroy, getParentOfType, getSnapshot, IAnyStateTreeNode, Instance, types } from "mobx-state-tree"; import { Variable, VariableType } from "@concord-consortium/diagram-view"; import { getTileModel } from "../../../../models/document/shared-model-document-manager"; @@ -10,8 +10,10 @@ import { } from "../../../graph/adornments/plotted-function/plotted-function-adornment-model"; import { SharedVariables, SharedVariablesType } from "../../shared-variables"; import { kPlottedVariablesType } from "./plotted-variables-adornment-types"; -import { GraphAttrRole, Point } from "../../../graph/graph-types"; +import { GraphAttrRole, Point, PrimaryAttrRole } from "../../../graph/graph-types"; import { IClueObject } from "../../../../models/annotations/clue-object"; +import { GraphModel } from "../../../graph/models/graph-model"; +import { isNumericAxisModel } from "../../../graph/imports/components/axis/models/axis-model"; function getSharedVariables(node: IAnyStateTreeNode) { const sharedModelManager = getSharedModelManager(node); @@ -39,6 +41,9 @@ function decipherAnnotationId(id: string) { return {}; } +/** + * A single X,Y pair of variables to be plotted on the graph. + */ export const PlottedVariables = types.model("PlottedVariables", {}) .props({ xVariableId: types.maybe(types.string), @@ -108,6 +113,9 @@ export const PlottedVariables = types.model("PlottedVariables", {}) } })); +/** + * An Adornment that holds one or more PlottedVariables to be shown on the graph. + */ export const PlottedVariablesAdornmentModel = PlottedFunctionAdornmentModel .named("PlottedVariablesAdornmentModel") .props({ @@ -162,16 +170,40 @@ export const PlottedVariablesAdornmentModel = PlottedFunctionAdornmentModel } })) .views(self => ({ - numericValuesForAttrRole(role: GraphAttrRole) { - const values = self.variableValues; - if (role in values) { - // We don't return the actual variable values, but rather 0 and 2 times each value. - // This is because of how autoscale is defined for variables - not just the current-value point - // has to fit in the graph, but a range of values around it so the function line can be seen. - return [0, ...values[role as 'x'|'y'].map(x => 2*x)]; - } else { - return [] as number[]; + numericValuesForAttrRole(_role: GraphAttrRole) { + // We only have any values for X and Y + if (!['x','y'].includes(_role)) return []; + + const role = _role as PrimaryAttrRole; + const result = [] as number[]; + for (const pvi of self.plottedVariables.values()) { + const vals = pvi.variableValues; + if (vals && role in vals) { + // We return 2 times each value because of how autoscale is defined for + // variables. Not just the current-value point has to fit in the graph, + // but a range of values around it so the function line can be seen clearly. + result.push(2*vals[role]); + } else { + // If the variable does not have a value, we pretend that the 'X' value is in the middle of the graph's + // X axis, and return a 'Y' range that will bring that part of the variable trace into view. + if (role === 'y') { + const graph = getParentOfType(self, GraphModel); + const bottomAxis = graph.getAxis("bottom"); + const fakeX = isNumericAxisModel(bottomAxis) ? (bottomAxis.min + bottomAxis.max) / 2 : undefined; + if (fakeX) { + const { computeY, dispose } = pvi.setupCompute(); + const fakeY = computeY(fakeX); + result.push(2*fakeY); + dispose(); + } + } + } + } + // The region we want to be visible is from 0 to twice the value(s) + if (result.length > 0) { + result.push(0); } + return result; } })) .actions(self => ({ From 8153f2f14359dda79ea67671d00c49062e8b9698 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 18 Mar 2024 09:11:41 -0400 Subject: [PATCH 57/78] Switch point dragging to use d3 drag library --- src/plugins/graph/components/scatterdots.tsx | 123 ++++++++----------- src/plugins/graph/d3-types.ts | 2 +- 2 files changed, 51 insertions(+), 74 deletions(-) diff --git a/src/plugins/graph/components/scatterdots.tsx b/src/plugins/graph/components/scatterdots.tsx index 2c7cd4ca5f..e3dbdd754c 100644 --- a/src/plugins/graph/components/scatterdots.tsx +++ b/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(), - [dragID, setDragID] = useState(''), - currPos = useRef({x: 0, y: 0}), target = useRef(), plotNumRef = useRef(0); @@ -40,85 +37,65 @@ 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, 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, - 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, + 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) => { + if (event.dx!==0 || event.dy!==0) { + updatePositions(event.dx, event.dy); + } }, [updatePositions]), - onDragEnd = useCallback((event: MouseEvent) => { - if (dragID !== '') { + onDragEnd = useCallback((event: D3DragEvent) => { 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]); + + selectGraphDots(dotsRef.current) + ?.call(drag() + .filter(() => { return graphModel.editingMode !== "none"; }) + .subject((event) => { return { x: event.x, y: event.y }; }) + .on('start', onDragStart) + .on('drag', onDrag) + .on('end', onDragEnd) + ); const refreshPointSelection = useCallback(() => { const { pointColor, pointStrokeColor } = graphModel; diff --git a/src/plugins/graph/d3-types.ts b/src/plugins/graph/d3-types.ts index 4f6b0c931d..e393ff0d17 100644 --- a/src/plugins/graph/d3-types.ts +++ b/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; +export type DotSelection = Selection; export const graphDotSelector = "g.graph-dot"; From ac463f05b7c3da1d80a3ff184d73d307c6932bef Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 18 Mar 2024 09:26:34 -0400 Subject: [PATCH 58/78] isTileSelected not updated live in this context --- .../plotted-variables-adornment-component.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx index 5f216ae2ba..176aeacefc 100644 --- a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx +++ b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx @@ -101,7 +101,6 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria const tPixelMin = xScale(xMin); const tPixelMax = xScale(xMax); const kPixelGap = 1; - const isTileSelected = !!tile && ui.isSelectedTile(tile); for (const instanceKey of model.plottedVariables.keys()) { const plottedVar = model.plottedVariables.get(instanceKey); const variable = plottedVar && plottedVar.xVariableId && sharedVariables?.getVariableById(plottedVar.xVariableId); @@ -169,7 +168,7 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria pointHighlight .call(drag() .container(() => { return plottedFunctionRef.current!; }) - .filter((e) => { return !e.ctrlKey && !e.button && isTileSelected; }) + .filter((e) => { return !e.ctrlKey && !e.button; }) .on('start', (e) => traceGroup.classed('dragging', true)) .on('drag', (e) => { const newX = Math.round(e.x); From 24375d494139f370108a8096a54c60f140b9b130 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 18 Mar 2024 10:14:35 -0400 Subject: [PATCH 59/78] Fix listening to changes in dynamically added layers. Remove static layer listeners from graph-axis Call autoscale in use-plot (which is per-layer) instead --- src/plugins/graph/components/graph-axis.tsx | 19 --------------- src/plugins/graph/hooks/use-graph-model.ts | 27 --------------------- src/plugins/graph/hooks/use-plot.ts | 22 ++++++++++++++--- 3 files changed, 18 insertions(+), 50 deletions(-) diff --git a/src/plugins/graph/components/graph-axis.tsx b/src/plugins/graph/components/graph-axis.tsx index 6d83921121..9d64104415 100644 --- a/src/plugins/graph/components/graph-axis.tsx +++ b/src/plugins/graph/components/graph-axis.tsx @@ -17,7 +17,6 @@ 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"; @@ -87,24 +86,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. diff --git a/src/plugins/graph/hooks/use-graph-model.ts b/src/plugins/graph/hooks/use-graph-model.ts index 29979f4f14..86479a56b3 100644 --- a/src/plugins/graph/hooks/use-graph-model.ts +++ b/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 => { diff --git a/src/plugins/graph/hooks/use-plot.ts b/src/plugins/graph/hooks/use-plot.ts index b02dc5a745..3f51966bbb 100644 --- a/src/plugins/graph/hooks/use-plot.ts +++ b/src/plugins/graph/hooks/use-plot.ts @@ -1,4 +1,4 @@ -import React, {useCallback, useEffect, useRef} from "react"; +import React, {useCallback, useContext, useEffect, useRef} from "react"; import {autorun, reaction} from "mobx"; import { isAddCasesAction, isRemoveAttributeAction, isRemoveCasesAction, isSetCaseValuesAction } from "../../../models/data/data-set-actions"; @@ -13,6 +13,7 @@ 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"; +import { GraphControllerContext } from "../models/graph-controller"; interface IDragHandlers { start: (event: MouseEvent) => void @@ -52,6 +53,7 @@ export const usePlotResponders = (props: IPlotResponderProps) => { graphModel = useGraphModelContext(), layout = useGraphLayoutContext(), instanceId = useInstanceIdContext(), + controller = useContext(GraphControllerContext), refreshPointPositionsRef = useCurrent(refreshPointPositions); useEffect(function respondToLayerDotsEltCreation() { @@ -104,6 +106,14 @@ export const usePlotResponders = (props: IPlotResponderProps) => { }; }, []); + const callRescaleIfPermitted = useCallback(() => { + if (controller!.autoAdjustAxes && + !graphModel.lockAxes && + !graphModel.interactionInProgress) { + controller!.autoscaleAllAxes(); + } + }, [controller, graphModel]); + // respond to numeric axis domain changes (e.g. axis dragging) useEffect(() => { const disposer = reaction( @@ -143,7 +153,8 @@ export const usePlotResponders = (props: IPlotResponderProps) => { dataConfiguration ); return () => disposer(); - }, [callRefreshPointPositions, dataConfiguration, enableAnimation]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [callRefreshPointPositions, dataConfiguration]); // respond to axis range changes (e.g. component resizing) useEffect(() => { @@ -162,6 +173,7 @@ export const usePlotResponders = (props: IPlotResponderProps) => { const disposer = onAnyAction(dataset, action => { if (isSetCaseValuesAction(action)) { // assumes that if we're caching then only selected cases are being updated + callRescaleIfPermitted(); callRefreshPointPositions(dataset.isCaching); // TODO: handling of add/remove cases was added specifically for the case plot. // Bill has expressed a desire to refactor the case plot to behave more like the @@ -173,7 +185,7 @@ export const usePlotResponders = (props: IPlotResponderProps) => { }); return () => disposer(); } - }, [dataset, callRefreshPointPositions]); + }, [dataset, callRefreshPointPositions, callRescaleIfPermitted]); // respond to color changes useEffect(() => { @@ -210,11 +222,13 @@ export const usePlotResponders = (props: IPlotResponderProps) => { dotsElement: dotsRef.current, enableAnimation, instanceId }); + callRescaleIfPermitted(); callRefreshPointPositions(false); } }) || (() => true); return () => disposer(); - }, [dataset, dataConfiguration, enableAnimation, graphModel, callRefreshPointPositions, dotsRef, instanceId]); + }, [controller, dataset, dataConfiguration, enableAnimation, graphModel, + callRefreshPointPositions, dotsRef, instanceId, callRescaleIfPermitted]); // respond to pointsNeedUpdating becoming false; that is when the points have been updated // Happens when the number of plots has changed for now. Possibly other situations in the future. From 22eed8beb75e62e6db8aa03eee17774e23b84389 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 18 Mar 2024 13:36:01 -0400 Subject: [PATCH 60/78] Make sure we're not using stale scale values. Draw initial state of variable trace when it becomes visible. Don't draw it when there are no variables. --- .../plotted-variables-adornment-component.tsx | 62 ++++++++++++------- 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx index 176aeacefc..e37f2070fa 100644 --- a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx +++ b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx @@ -16,7 +16,6 @@ import { SharedVariables } from "../../shared-variables"; import { IPlottedVariablesAdornmentModel } from "./plotted-variables-adornment-model"; import { useReadOnlyContext } from "../../../../components/document/read-only-context"; import { isFiniteNumber } from "../../../../utilities/math-utils"; -import { useUIStore } from "../../../../hooks/use-stores"; import { useGraphSettingsContext } from "../../../graph/hooks/use-graph-settings-context"; import { GraphControllerContext } from "../../../graph/models/graph-controller"; @@ -39,15 +38,8 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria const graphModel = useGraphModelContext(); const readOnly = useReadOnlyContext(); const layout = useAxisLayoutContext(); - const ui = useUIStore(); const settings = useGraphSettingsContext(); const controller = useContext(GraphControllerContext); - const xScale = layout.getAxisScale("bottom") as ScaleNumericBaseType; - const yScale = layout.getAxisScale("left") as ScaleNumericBaseType; - const xSubAxesCount = layout.getAxisMultiScale("bottom")?.repetitions ?? 1; - const ySubAxesCount = layout.getAxisMultiScale("left")?.repetitions ?? 1; - const xCellCount = xSubAxesCount; - const yCellCount = ySubAxesCount; const classFromKey = model.classNameFromKey(cellKey); const plottedFunctionRef = useRef(null); const smm = getSharedModelManager(graphModel); @@ -87,15 +79,21 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria // Assign a new value to the Variable based on the given pixel position const setVariableValue = useCallback((variable: VariableType, position: number) => { + const xScale = layout.getAxisScale("bottom") as ScaleNumericBaseType; + const xCellCount = layout.getAxisMultiScale("bottom")?.repetitions ?? 1; const newValue = model.valueForPosition(position, xScale, xCellCount); if (isFiniteNumber(newValue)) { // Truncate extra decimals to match the value that is displayed. variable.setValue(+numberFormatter.format(newValue)); } - }, [model, numberFormatter, xCellCount, xScale]); + }, [layout, model, numberFormatter]); // Draw the variable traces const addPath = useCallback(() => { + const xScale = layout.getAxisScale("bottom") as ScaleNumericBaseType; + const xCellCount = layout.getAxisMultiScale("bottom")?.repetitions ?? 1; + const yScale = layout.getAxisScale("left") as ScaleNumericBaseType; + const yCellCount = layout.getAxisMultiScale("left")?.repetitions ?? 1; const xMin = xScale.domain()[0]; const xMax = xScale.domain()[1]; const tPixelMin = xScale(xMin); @@ -190,8 +188,7 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria } } } - }, [tile, ui, model, graphModel, xScale, xCellCount, yScale, yCellCount, - labelRectHeight, positionPointMarkers, readOnly, sharedVariables, setVariableValue]); + }, [layout, model, sharedVariables, readOnly, graphModel, setVariableValue, labelRectHeight, positionPointMarkers]); // Add the lines and their associated covers and labels const refreshValues = useCallback(() => { @@ -211,7 +208,20 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria return mstAutorun(() => { model.updateCategories(graphModel.layers[0].getUpdateCategoriesOptions(false)); }, { name: "PlottedVariablesAdornmentComponent.refreshExpressionChange" }, model); - }, [graphModel, model, xScale, xSubAxesCount, yScale]); + }, [graphModel, model]); + + // Draw when becoming visible + useEffect(function refreshOnVisibility() { + return mstReaction( + () => { return model.isVisible; }, + (visible) => { + if (visible) { + refreshValues(); + } + }, + { name: "PlottedVariablesAdornment.refreshOnVisibility" }, + model); + }, [model, refreshValues]); // Refresh display when axis domains change useEffect(function refreshAxisChange() { @@ -233,20 +243,28 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria model); }, [model, plotWidth, plotHeight, xAxis, yAxis, refreshValues, controller]); - // If desired, rescale graph when variable values change - useEffect(function rescaleOnValuesChange() { + // Refresh display when variable values change + useEffect(function udpateOnValuesChange() { return mstReaction( () => { - return Array.from(model.plottedVariables.values()).map(plottedVariables => { - return [ - plottedVariables.xVariable, - plottedVariables.yVariable?.computedValueIncludingMessageAndError - ]; + let hasAnyVars = false; + const varValues = Array.from(model.plottedVariables.values()).map(plottedVariables => { + if (plottedVariables.xVariable || plottedVariables.yVariable) { + hasAnyVars = true; + } + return { + x: plottedVariables.xVariable, + y: plottedVariables.yVariable?.computedValueIncludingMessageAndError + }; }); + return {hasAnyVars, varValues}; }, - (any) => { - if (settings.scalePlotOnValueChange && !graphModel.lockAxes) { - controller?.autoscaleAllAxes(); + (args: {hasAnyVars: boolean, varValues: any}) => { + if (args.hasAnyVars) { + if (settings.scalePlotOnValueChange && !graphModel.lockAxes) { + controller?.autoscaleAllAxes(); + } + refreshValues(); } }, { name: "PlottedVariablesAdornmentComponent.rescaleOnValuesChange" }, From 3913a0dfa527194bb0649c4a5d706e1eb7f8ef92 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 18 Mar 2024 14:51:05 -0400 Subject: [PATCH 61/78] Remove 'autoAdjust' from controller. It was getting passed around but never set to a value other than 'true. --- src/plugins/graph/components/graph-axis.tsx | 3 +-- src/plugins/graph/components/graph-component.tsx | 3 +-- src/plugins/graph/components/graph.tsx | 3 +-- src/plugins/graph/hooks/use-plot.ts | 6 ++++-- src/plugins/graph/models/graph-controller.ts | 5 +---- 5 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/plugins/graph/components/graph-axis.tsx b/src/plugins/graph/components/graph-axis.tsx index 9d64104415..5ac60ec487 100644 --- a/src/plugins/graph/components/graph-axis.tsx +++ b/src/plugins/graph/components/graph-axis.tsx @@ -24,7 +24,6 @@ import { GraphController } from "../models/graph-controller"; interface IProps { place: AxisPlace; enableAnimation: MutableRefObject; - autoAdjust?: React.MutableRefObject; controller: GraphController; onDropAttribute?: (place: GraphPlace, dataSet: IDataSet, attrId: string) => void; onRemoveAttribute?: (place: GraphPlace, attrId: string) => void; @@ -32,7 +31,7 @@ interface IProps { } 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), diff --git a/src/plugins/graph/components/graph-component.tsx b/src/plugins/graph/components/graph-component.tsx index b8e77b80ed..79acb7814b 100644 --- a/src/plugins/graph/components/graph-component.tsx +++ b/src/plugins/graph/components/graph-component.tsx @@ -29,9 +29,8 @@ export const GraphComponent = observer( const graphRef = useRef(null); const {width, height} = useResizeDetector({ 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] ); diff --git a/src/plugins/graph/components/graph.tsx b/src/plugins/graph/components/graph.tsx index 4a94675785..9390b9dd61 100644 --- a/src/plugins/graph/components/graph.tsx +++ b/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(() => new MarqueeState(), []), @@ -185,7 +185,6 @@ export const Graph = observer( place={place} controller={graphController} enableAnimation={enableAnimation} - autoAdjust={autoAdjustAxes} onDropAttribute={handleChangeAttribute} onRemoveAttribute={handleRemoveAttribute} onTreatAttributeAs={handleTreatAttrAs} diff --git a/src/plugins/graph/hooks/use-plot.ts b/src/plugins/graph/hooks/use-plot.ts index 3f51966bbb..188a89932a 100644 --- a/src/plugins/graph/hooks/use-plot.ts +++ b/src/plugins/graph/hooks/use-plot.ts @@ -14,6 +14,7 @@ import { IGraphLayerModel } from "../models/graph-layer-model"; import { mstReaction } from "../../../utilities/mst-reaction"; import { useReadOnlyContext } from "../../../components/document/read-only-context"; import { GraphControllerContext } from "../models/graph-controller"; +import { useGraphSettingsContext } from "./use-graph-settings-context"; interface IDragHandlers { start: (event: MouseEvent) => void @@ -54,6 +55,7 @@ export const usePlotResponders = (props: IPlotResponderProps) => { layout = useGraphLayoutContext(), instanceId = useInstanceIdContext(), controller = useContext(GraphControllerContext), + graphSettings = useGraphSettingsContext(), refreshPointPositionsRef = useCurrent(refreshPointPositions); useEffect(function respondToLayerDotsEltCreation() { @@ -107,12 +109,12 @@ export const usePlotResponders = (props: IPlotResponderProps) => { }, []); const callRescaleIfPermitted = useCallback(() => { - if (controller!.autoAdjustAxes && + if (graphSettings.scalePlotOnValueChange && !graphModel.lockAxes && !graphModel.interactionInProgress) { controller!.autoscaleAllAxes(); } - }, [controller, graphModel]); + }, [controller, graphModel, graphSettings]); // respond to numeric axis domain changes (e.g. axis dragging) useEffect(() => { diff --git a/src/plugins/graph/models/graph-controller.ts b/src/plugins/graph/models/graph-controller.ts index 897603f1d6..f2e1310264 100644 --- a/src/plugins/graph/models/graph-controller.ts +++ b/src/plugins/graph/models/graph-controller.ts @@ -27,7 +27,6 @@ interface IGraphControllerConstructorProps { layout: GraphLayout enableAnimation: React.MutableRefObject instanceId: string - autoAdjustAxes: React.MutableRefObject } interface IGraphControllerProps { @@ -39,13 +38,11 @@ export class GraphController { layout: GraphLayout; enableAnimation: React.MutableRefObject; instanceId: string; - autoAdjustAxes: React.MutableRefObject; - constructor({layout, enableAnimation, instanceId, autoAdjustAxes}: IGraphControllerConstructorProps) { + constructor({layout, enableAnimation, instanceId}: IGraphControllerConstructorProps) { this.layout = layout; this.instanceId = instanceId; this.enableAnimation = enableAnimation; - this.autoAdjustAxes = autoAdjustAxes; } setProperties(props: IGraphControllerProps) { From 7452d52e73d7808d2cd8cb8166010116015c1275 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 18 Mar 2024 15:40:45 -0400 Subject: [PATCH 62/78] Don't autoscale in 0's for empty variables adornment. Clean up plottedVariables when disconnecting. --- .../plotted-variables-adornment-model.ts | 44 +++++++++++-------- .../shared-variables-registration.ts | 22 +++++++--- 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-model.ts b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-model.ts index 1829f98815..6d106fac70 100644 --- a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-model.ts +++ b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-model.ts @@ -172,29 +172,32 @@ export const PlottedVariablesAdornmentModel = PlottedFunctionAdornmentModel .views(self => ({ numericValuesForAttrRole(_role: GraphAttrRole) { // We only have any values for X and Y - if (!['x','y'].includes(_role)) return []; + if (!['x', 'y'].includes(_role)) return []; const role = _role as PrimaryAttrRole; const result = [] as number[]; for (const pvi of self.plottedVariables.values()) { - const vals = pvi.variableValues; - if (vals && role in vals) { - // We return 2 times each value because of how autoscale is defined for - // variables. Not just the current-value point has to fit in the graph, - // but a range of values around it so the function line can be seen clearly. - result.push(2*vals[role]); - } else { - // If the variable does not have a value, we pretend that the 'X' value is in the middle of the graph's - // X axis, and return a 'Y' range that will bring that part of the variable trace into view. - if (role === 'y') { - const graph = getParentOfType(self, GraphModel); - const bottomAxis = graph.getAxis("bottom"); - const fakeX = isNumericAxisModel(bottomAxis) ? (bottomAxis.min + bottomAxis.max) / 2 : undefined; - if (fakeX) { - const { computeY, dispose } = pvi.setupCompute(); - const fakeY = computeY(fakeX); - result.push(2*fakeY); - dispose(); + if (pvi.xVariableId && pvi.yVariableId) { + const vals = pvi.variableValues; + if (vals && role in vals) { + // We return 2 times each value because of how autoscale is defined for + // variables. Not just the current-value point has to fit in the graph, + // but a range of values around it so the function line can be seen clearly. + result.push(2 * vals[role]); + } else { + // If the variable exists but does not have a value, we pretend that + // the 'X' value is in the middle of the graph's X axis, and return + // a 'Y' range that will bring that part of the variable trace into view. + if (role === 'y') { + const graph = getParentOfType(self, GraphModel); + const bottomAxis = graph.getAxis("bottom"); + const fakeX = isNumericAxisModel(bottomAxis) ? (bottomAxis.min + bottomAxis.max) / 2 : undefined; + if (fakeX) { + const { computeY, dispose } = pvi.setupCompute(); + const fakeY = computeY(fakeX); + result.push(2 * fakeY); + dispose(); + } } } } @@ -216,6 +219,9 @@ export const PlottedVariablesAdornmentModel = PlottedFunctionAdornmentModel removePlottedVariables(key: string) { self.plottedVariables.delete(key); }, + clearPlottedVariables() { + self.plottedVariables.clear(); + }, setupCompute(instanceKey: string) { return self.plottedVariables.get(instanceKey)?.setupCompute(); } diff --git a/src/plugins/shared-variables/shared-variables-registration.ts b/src/plugins/shared-variables/shared-variables-registration.ts index 2ca5bb14f6..789559ec4c 100644 --- a/src/plugins/shared-variables/shared-variables-registration.ts +++ b/src/plugins/shared-variables/shared-variables-registration.ts @@ -7,7 +7,7 @@ import { IGraphModel, registerGraphSharedModelUpdateFunction } from "../graph/mo import { EditVariableButton, InsertVariableButton, NewVariableButton, VariableChipComponent, VariableChipObject } from "./drawing/variable-object"; -import { PlottedVariablesAdornmentModel +import { isPlottedVariablesAdornment, PlottedVariablesAdornmentModel } from "./graph/plotted-variables-adornment/plotted-variables-adornment-model"; import "./graph/plotted-variables-adornment/plotted-variables-adornment-registration"; import { @@ -74,16 +74,26 @@ registerGraphSharedModelUpdateFunction( // Display a plotted variables adornment when this is linked to a shared variables model const sharedVariableModels = smm.getTileSharedModelsByType(graphModel, SharedVariables); if (sharedVariableModels && sharedVariableModels.length > 0) { - // We're connected to variables; show adornment or create if not there already. - if (graphModel.getAdornmentOfType(kPlottedVariablesType)) { + // We're connected to variables; make sure adornment is showing, or create if not there already. + let adornment = graphModel.getAdornmentOfType(kPlottedVariablesType); + if (adornment) { graphModel.showAdornment(kPlottedVariablesType); } else { - const plottedVariablesAdornment = PlottedVariablesAdornmentModel.create(); - plottedVariablesAdornment.addPlottedVariables(); - graphModel.addAdornment(plottedVariablesAdornment); + adornment = PlottedVariablesAdornmentModel.create(); + graphModel.addAdornment(adornment); + } + // Make sure there's at least one PlottedVariables in it. + if (adornment && isPlottedVariablesAdornment(adornment)) { + if (adornment.plottedVariables.size === 0) { + adornment.addPlottedVariables(); + } } } else { // Disconnected + const adornment = graphModel.getAdornmentOfType(kPlottedVariablesType); + if (adornment && isPlottedVariablesAdornment(adornment)) { + adornment.clearPlottedVariables(); + } graphModel.hideAdornment(kPlottedVariablesType); } } From c2c261e313b4189c88de563815171c805135204d Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Tue, 19 Mar 2024 08:21:33 -0400 Subject: [PATCH 63/78] Nicer niceNumericBounds --- src/plugins/graph/utilities/graph-utils.ts | 24 ++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/plugins/graph/utilities/graph-utils.ts b/src/plugins/graph/utilities/graph-utils.ts index 8cd5885241..3de2c99151 100644 --- a/src/plugins/graph/utilities/graph-utils.ts +++ b/src/plugins/graph/utilities/graph-utils.ts @@ -84,21 +84,29 @@ export function computeNiceNumericBounds(min: number, max: number): { min: numbe bounds.min -= kAddend; bounds.max += kAddend; } else if (min === max) { - bounds.min = bounds.min + 0.1 * Math.abs(bounds.min); - bounds.max = bounds.max - 0.1 * Math.abs(bounds.max); - } else if (min > 0 && max > 0 && min <= max / kFactor) { // Snap to zero - bounds.min = 0; - } else if (min < 0 && max < 0 && max >= min / kFactor) { // Snap to zero - bounds.max = 0; + // Make a range around a single point by moving limits 10% in each direction + bounds.min = bounds.min - 0.1 * Math.abs(bounds.min); + bounds.max = bounds.max + 0.1 * Math.abs(bounds.max); } + + // Find location of tick marks, move limits to a tick mark + // making sure there is at least 1/2 of a tick mark space beyond the last data point const tickGap = computeTickGap(bounds.min, bounds.max); + if (tickGap !== 0) { - bounds.min = (Math.floor(bounds.min / tickGap) - 0.5) * tickGap; - bounds.max = (Math.floor(bounds.max / tickGap) + 1.5) * tickGap; + bounds.min = Math.floor(bounds.min / tickGap - 0.5) * tickGap; + bounds.max = Math.ceil(bounds.max / tickGap + 0.5) * tickGap; } else { bounds.min -= 1; bounds.max += 1; } + + // Snap to zero if close to 0. + if (min > 0 && max > 0 && min <= max / kFactor) { + bounds.min = 0; + } else if (min < 0 && max < 0 && max >= min / kFactor) { + bounds.max = 0; + } return bounds; } From 2df1f8b3608aca291cc8108dc18c6eba27c1696e Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Tue, 19 Mar 2024 08:57:13 -0400 Subject: [PATCH 64/78] Don't shrink graph on adding points. --- src/plugins/graph/hooks/use-plot.ts | 13 +++---- src/plugins/graph/models/graph-controller.ts | 4 +-- src/plugins/graph/models/graph-layer-model.ts | 3 -- src/plugins/graph/utilities/graph-utils.ts | 34 +++++++++++++++---- 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/plugins/graph/hooks/use-plot.ts b/src/plugins/graph/hooks/use-plot.ts index 188a89932a..7f70f8f774 100644 --- a/src/plugins/graph/hooks/use-plot.ts +++ b/src/plugins/graph/hooks/use-plot.ts @@ -108,11 +108,11 @@ export const usePlotResponders = (props: IPlotResponderProps) => { }; }, []); - const callRescaleIfPermitted = useCallback(() => { + const callRescaleIfNeeded = useCallback((growOnly: boolean = false) => { if (graphSettings.scalePlotOnValueChange && !graphModel.lockAxes && !graphModel.interactionInProgress) { - controller!.autoscaleAllAxes(); + controller!.autoscaleAllAxes(growOnly); } }, [controller, graphModel, graphSettings]); @@ -174,8 +174,8 @@ export const usePlotResponders = (props: IPlotResponderProps) => { if (dataset) { const disposer = onAnyAction(dataset, action => { if (isSetCaseValuesAction(action)) { + callRescaleIfNeeded(); // assumes that if we're caching then only selected cases are being updated - callRescaleIfPermitted(); callRefreshPointPositions(dataset.isCaching); // TODO: handling of add/remove cases was added specifically for the case plot. // Bill has expressed a desire to refactor the case plot to behave more like the @@ -187,7 +187,7 @@ export const usePlotResponders = (props: IPlotResponderProps) => { }); return () => disposer(); } - }, [dataset, callRefreshPointPositions, callRescaleIfPermitted]); + }, [dataset, callRefreshPointPositions, callRescaleIfNeeded]); // respond to color changes useEffect(() => { @@ -224,13 +224,14 @@ export const usePlotResponders = (props: IPlotResponderProps) => { dotsElement: dotsRef.current, enableAnimation, instanceId }); - callRescaleIfPermitted(); + const growOnly = isAddCasesAction(action); + callRescaleIfNeeded(growOnly); callRefreshPointPositions(false); } }) || (() => true); return () => disposer(); }, [controller, dataset, dataConfiguration, enableAnimation, graphModel, - callRefreshPointPositions, dotsRef, instanceId, callRescaleIfPermitted]); + callRefreshPointPositions, dotsRef, instanceId, callRescaleIfNeeded]); // respond to pointsNeedUpdating becoming false; that is when the points have been updated // Happens when the number of plots has changed for now. Possibly other situations in the future. diff --git a/src/plugins/graph/models/graph-controller.ts b/src/plugins/graph/models/graph-controller.ts index f2e1310264..35a42312b8 100644 --- a/src/plugins/graph/models/graph-controller.ts +++ b/src/plugins/graph/models/graph-controller.ts @@ -194,14 +194,14 @@ export class GraphController { /** * Set the domains of all axes to fit all of the data points. */ - autoscaleAllAxes() { + autoscaleAllAxes(growOnly: boolean = false) { if (!this.graphModel) return; startAnimation(this.enableAnimation); for (const place of AxisPlaces) { const role = graphPlaceToAttrRole[place]; const axisModel = this.graphModel.getAxis(place); if (isNumericAxisModel(axisModel)) { - setNiceDomain(this.graphModel.numericValuesForAttrRole(role), axisModel); + setNiceDomain(this.graphModel.numericValuesForAttrRole(role), axisModel, growOnly); } } } diff --git a/src/plugins/graph/models/graph-layer-model.ts b/src/plugins/graph/models/graph-layer-model.ts index 0d508fa3fe..3eeb903c22 100644 --- a/src/plugins/graph/models/graph-layer-model.ts +++ b/src/plugins/graph/models/graph-layer-model.ts @@ -81,9 +81,6 @@ export const GraphLayerModel = types 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); return caseAdded[0]; } }, diff --git a/src/plugins/graph/utilities/graph-utils.ts b/src/plugins/graph/utilities/graph-utils.ts index 3de2c99151..f8bf253c03 100644 --- a/src/plugins/graph/utilities/graph-utils.ts +++ b/src/plugins/graph/utilities/graph-utils.ts @@ -10,7 +10,7 @@ import { } from "../d3-types"; import { IDotsRef, kGraphFont, Point, outerCircleSelectedRadius, outerCircleUnselectedRadius, - Rect,rTreeRect, transitionDuration + Rect,rTreeRect, transitionDuration, kDefaultNumericAxisBounds } from "../graph-types"; import {between} from "./math-utils"; import {IAxisModel, isNumericAxisModel} from "../imports/components/axis/models/axis-model"; @@ -51,7 +51,8 @@ export function ptInRect(pt: Point, iRect: Rect) { /** * This function closely follows V2's CellLinearAxisModel:_computeBoundsAndTickGap */ -export function computeNiceNumericBounds(min: number, max: number): { min: number, max: number } { +export function computeNiceNumericBounds( + min: number|undefined, max: number|undefined): { min: number, max: number } { function computeTickGap(iMin: number, iMax: number) { const range = (iMin >= iMax) ? Math.abs(iMin) : iMax - iMin, @@ -74,13 +75,15 @@ export function computeNiceNumericBounds(min: number, max: number): { min: numbe return Math.max(power * base, Number.MIN_VALUE); } + if (min === undefined || max === undefined || (min === max && min === 0)) { + return { min: kDefaultNumericAxisBounds[0], max: kDefaultNumericAxisBounds[1] }; + } + const kAddend = 5, // amount to extend scale kFactor = 2.5, bounds = {min, max}; - if (min === max && min === 0) { - bounds.min = -10; - bounds.max = 10; - } else if (min === max && isInteger(min)) { + + if (min === max && isInteger(min)) { bounds.min -= kAddend; bounds.max += kAddend; } else if (min === max) { @@ -110,9 +113,26 @@ export function computeNiceNumericBounds(min: number, max: number): { min: numbe return bounds; } -export function setNiceDomain(values: number[], axisModel: IAxisModel) { +/** + * Set the domain of the axis so that it fits all the given data points in a + * nice way. The values can be any numbers in any order; only the min and max in + * the array matter. The actual min and max set on the axis are derived from + * this considering factors such as making sure there is a non-zero range, + * starting and ending on tick marks, and snapping an end to 0 if it is close to 0. + * + * @param values data points to be fit. + * @param axisModel axis to update. + * @param growOnly if true, the range will not be reduced from its current limits, only expanded if necessary. + */ +export function setNiceDomain(values: number[], axisModel: IAxisModel, growOnly: boolean = false) { if (isNumericAxisModel(axisModel)) { const [minValue, maxValue] = extent(values, d => d) as [number, number]; + if (growOnly) { + const [currentMin, currentMax] = axisModel.domain; + if (minValue >= currentMin && maxValue <= currentMax) { + return; + } + } const {min: niceMin, max: niceMax} = computeNiceNumericBounds(minValue, maxValue); axisModel.setDomain(niceMin, niceMax); } From 8a6ca9e893598d19326fec2d1fc57d7dd09ff400 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Tue, 19 Mar 2024 10:16:58 -0400 Subject: [PATCH 65/78] Update cypress tests; comment some out --- .../tile_tests/xy_plot_tool_spec.js | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) 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 b520f34c0a..30b2c10c8a 100644 --- a/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js +++ b/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(); @@ -534,14 +541,19 @@ 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); + + // FIXME: I cannot make this test work. It used to work before switching to d3-drag for the points. + // cy.window().then(win => { + // xyTile.getGraphDot().eq(0) + // .trigger("mousedown", { force: true, eventConstructor: 'MouseEvent', button: 0, view: win, clientX: 150, clientY: 50 }) + // .trigger("dragstart", { force: true, eventConstructor: 'DragEvent' }) + // .trigger("drag") + // .trigger("mousemove", { force: true, eventConstructor: 'MouseEvent', clientX: 175, clientY: 75 }) + // .trigger("mouseup", 175, 75, { force: true, eventConstructor: 'MouseEvent', clientX: 175, clientY: 75, view: win }); + // }); + // 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'); From f965e1706d1af7f349d60ce30e08f7f069a1169c Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 19 Mar 2024 11:30:50 -0400 Subject: [PATCH 66/78] notify slack when the nightly build fails I haven't figured out a way to test this. Perhaps after it is merged, we can intentionally fail the build and see if we are notified. --- .github/workflows/regression-daily.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/workflows/regression-daily.yml b/.github/workflows/regression-daily.yml index 62fcde7c22..e598e4fe89 100644 --- a/.github/workflows/regression-daily.yml +++ b/.github/workflows/regression-daily.yml @@ -102,3 +102,19 @@ jobs: CYPRESS_coverage: true # Increase memory allocation NODE_OPTIONS: "--max_old_space_size=4096" + notify-slack: + if: ${{ failure() }} + needs: [all-tests] + runs-on: ubuntu-latest + steps: + - name: Send custom JSON data to Slack workflow + id: slack + uses: slackapi/slack-github-action@v1.25.0 + with: + # This data can be any valid JSON from a previous step in the GitHub Action + payload: | + { + "build_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}", + } + env: + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} From 556f4166ffc9b08768deb821b3eb736c19bde5cc Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 19 Mar 2024 16:08:00 -0400 Subject: [PATCH 67/78] Add slack notification on failure If this works it will be updated to only send the notifications on the master branch. --- .github/workflows/ci-regression.yml | 22 +++++++++++++++++++ .github/workflows/ci.yml | 22 +++++++++++++++++++ .github/workflows/regression-daily.yml | 6 +++-- .../tile_tests/diagram_tool_spec.js | 4 ++++ .../diagram-viewer/diagram-content.test.ts | 2 ++ 5 files changed, 54 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-regression.yml b/.github/workflows/ci-regression.yml index 277c21e3c3..af3e906e60 100644 --- a/.github/workflows/ci-regression.yml +++ b/.github/workflows/ci-regression.yml @@ -99,3 +99,25 @@ jobs: with: flags: cypress-regression token: ${{ secrets.CODECOV_TOKEN }} + notify-slack: + # TODO: Change this so it only runs when the branch is master. + # this ought then run when a PR is merged master which then causes the tests to fail + # This is intentally left off for the time being so we can test this job on the PR + # Branch + if: ${{ failure() }} + needs: [regression] + runs-on: ubuntu-latest + steps: + - name: Notify Slack the tests failed + id: slack + uses: slackapi/slack-github-action@v1.25.0 + with: + # This data can be any valid JSON from a previous step in the GitHub Action + payload: | + { + "ref_name": "${{ github.ref_name }}" + "workflow": "${{ github.workflow }}" + "run_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}", + } + env: + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 49f98b423e..905d1748dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -149,4 +149,26 @@ jobs: # be used. Instead the `|` turns the following lines into a string topBranches: | ["master"] + notify-slack: + # TODO: Change this so it only runs when the branch is master. + # this ought then run when a PR is merged master which then causes the tests to fail + # This is intentally left off for the time being so we can test this job on the PR + # Branch + if: ${{ failure() }} + needs: [jest,cypress] + runs-on: ubuntu-latest + steps: + - name: Notify Slack the tests failed + id: slack + uses: slackapi/slack-github-action@v1.25.0 + with: + # This data can be any valid JSON from a previous step in the GitHub Action + payload: | + { + "ref_name": "${{ github.ref_name }}" + "workflow": "${{ github.workflow }}" + "run_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}", + } + env: + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} diff --git a/.github/workflows/regression-daily.yml b/.github/workflows/regression-daily.yml index e598e4fe89..2528a08d8f 100644 --- a/.github/workflows/regression-daily.yml +++ b/.github/workflows/regression-daily.yml @@ -1,4 +1,4 @@ -name: Regression Tests +name: Daily Regression on: schedule: @@ -114,7 +114,9 @@ jobs: # This data can be any valid JSON from a previous step in the GitHub Action payload: | { - "build_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}", + "ref_name": "${{ github.ref_name }}" + "workflow": "${{ github.workflow }}" + "run_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}", } env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} diff --git a/cypress/e2e/functional/tile_tests/diagram_tool_spec.js b/cypress/e2e/functional/tile_tests/diagram_tool_spec.js index 64519e149f..7ef04773c6 100644 --- a/cypress/e2e/functional/tile_tests/diagram_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/diagram_tool_spec.js @@ -27,6 +27,10 @@ context('Diagram Tool Tile', function () { it("Shared Variable Tiles (Diagram, Drawing)", () => { beforeTest(); + + // Intentionally Fail to test notification system + diagramTile.getDiagramTile().should("exist"); + clueCanvas.addTile("diagram"); // Tile, toolbar, and buttons render diff --git a/src/plugins/diagram-viewer/diagram-content.test.ts b/src/plugins/diagram-viewer/diagram-content.test.ts index a62d5d4b22..2db766fec2 100644 --- a/src/plugins/diagram-viewer/diagram-content.test.ts +++ b/src/plugins/diagram-viewer/diagram-content.test.ts @@ -78,6 +78,8 @@ const setupContainer = (content: DiagramContentModelType, variables?: SharedVari describe("DiagramContent", () => { it("has default content", () => { + // Intentionally fail to test the slack notification + expect(false).toBeTruthy(); const content = defaultDiagramContent(); expect(content.root).toBeDefined(); }); From 9bd501dad0fc31bddffea3bf77931de19de78f0e Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 19 Mar 2024 16:13:54 -0400 Subject: [PATCH 68/78] Fix JSON formatting in payload to slack --- .github/workflows/ci-regression.yml | 6 +++--- .github/workflows/ci.yml | 6 +++--- .github/workflows/regression-daily.yml | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci-regression.yml b/.github/workflows/ci-regression.yml index af3e906e60..41aa8fa04f 100644 --- a/.github/workflows/ci-regression.yml +++ b/.github/workflows/ci-regression.yml @@ -115,9 +115,9 @@ jobs: # This data can be any valid JSON from a previous step in the GitHub Action payload: | { - "ref_name": "${{ github.ref_name }}" - "workflow": "${{ github.workflow }}" - "run_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}", + "ref_name": "${{ github.ref_name }}", + "workflow": "${{ github.workflow }}", + "run_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" } env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 905d1748dd..feda047d0f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -165,9 +165,9 @@ jobs: # This data can be any valid JSON from a previous step in the GitHub Action payload: | { - "ref_name": "${{ github.ref_name }}" - "workflow": "${{ github.workflow }}" - "run_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}", + "ref_name": "${{ github.ref_name }}", + "workflow": "${{ github.workflow }}", + "run_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" } env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} diff --git a/.github/workflows/regression-daily.yml b/.github/workflows/regression-daily.yml index 2528a08d8f..9b4c36cdc3 100644 --- a/.github/workflows/regression-daily.yml +++ b/.github/workflows/regression-daily.yml @@ -114,9 +114,9 @@ jobs: # This data can be any valid JSON from a previous step in the GitHub Action payload: | { - "ref_name": "${{ github.ref_name }}" - "workflow": "${{ github.workflow }}" - "run_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}", + "ref_name": "${{ github.ref_name }}", + "workflow": "${{ github.workflow }}", + "run_url": "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" } env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} From 7d5ecfb2f9b6f59b67e9fe40117f8d7dff181859 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 19 Mar 2024 17:09:03 -0400 Subject: [PATCH 69/78] revert intentional failing tests --- cypress/e2e/functional/tile_tests/diagram_tool_spec.js | 4 ---- src/plugins/diagram-viewer/diagram-content.test.ts | 2 -- 2 files changed, 6 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/diagram_tool_spec.js b/cypress/e2e/functional/tile_tests/diagram_tool_spec.js index 7ef04773c6..64519e149f 100644 --- a/cypress/e2e/functional/tile_tests/diagram_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/diagram_tool_spec.js @@ -27,10 +27,6 @@ context('Diagram Tool Tile', function () { it("Shared Variable Tiles (Diagram, Drawing)", () => { beforeTest(); - - // Intentionally Fail to test notification system - diagramTile.getDiagramTile().should("exist"); - clueCanvas.addTile("diagram"); // Tile, toolbar, and buttons render diff --git a/src/plugins/diagram-viewer/diagram-content.test.ts b/src/plugins/diagram-viewer/diagram-content.test.ts index 2db766fec2..a62d5d4b22 100644 --- a/src/plugins/diagram-viewer/diagram-content.test.ts +++ b/src/plugins/diagram-viewer/diagram-content.test.ts @@ -78,8 +78,6 @@ const setupContainer = (content: DiagramContentModelType, variables?: SharedVari describe("DiagramContent", () => { it("has default content", () => { - // Intentionally fail to test the slack notification - expect(false).toBeTruthy(); const content = defaultDiagramContent(); expect(content.root).toBeDefined(); }); From d3bb8ff6d6db6fb2f249d9debbfbf4fa925ca593 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 19 Mar 2024 17:21:25 -0400 Subject: [PATCH 70/78] only notify slack on builds of master also clean up some comments and a step name --- .github/workflows/ci-regression.yml | 6 +----- .github/workflows/ci.yml | 6 +----- .github/workflows/regression-daily.yml | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci-regression.yml b/.github/workflows/ci-regression.yml index 41aa8fa04f..cc09811d2f 100644 --- a/.github/workflows/ci-regression.yml +++ b/.github/workflows/ci-regression.yml @@ -100,11 +100,7 @@ jobs: flags: cypress-regression token: ${{ secrets.CODECOV_TOKEN }} notify-slack: - # TODO: Change this so it only runs when the branch is master. - # this ought then run when a PR is merged master which then causes the tests to fail - # This is intentally left off for the time being so we can test this job on the PR - # Branch - if: ${{ failure() }} + if: ${{ failure() && github.ref_name == 'master' }} needs: [regression] runs-on: ubuntu-latest steps: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index feda047d0f..7d74d898cd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -150,11 +150,7 @@ jobs: topBranches: | ["master"] notify-slack: - # TODO: Change this so it only runs when the branch is master. - # this ought then run when a PR is merged master which then causes the tests to fail - # This is intentally left off for the time being so we can test this job on the PR - # Branch - if: ${{ failure() }} + if: ${{ failure() && github.ref_name == 'master' }} needs: [jest,cypress] runs-on: ubuntu-latest steps: diff --git a/.github/workflows/regression-daily.yml b/.github/workflows/regression-daily.yml index 9b4c36cdc3..37d29ea302 100644 --- a/.github/workflows/regression-daily.yml +++ b/.github/workflows/regression-daily.yml @@ -107,7 +107,7 @@ jobs: needs: [all-tests] runs-on: ubuntu-latest steps: - - name: Send custom JSON data to Slack workflow + - name: Notify Slack the tests failed id: slack uses: slackapi/slack-github-action@v1.25.0 with: From 2f759248d50c7c3df26c148aaa3532212f9c8eb3 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 19 Mar 2024 20:32:50 -0400 Subject: [PATCH 71/78] use trashcan icon instead of x --- .../icons/delete/delete-selection-icon.svg | 18 +++++------------- src/clue/assets/icons/delete-tool.svg | 5 ++--- .../data-card/assets/delete-selection-icon.svg | 16 ++++------------ src/plugins/graph/assets/remove-data-icon.svg | 5 ++--- 4 files changed, 13 insertions(+), 31 deletions(-) diff --git a/src/assets/icons/delete/delete-selection-icon.svg b/src/assets/icons/delete/delete-selection-icon.svg index ec8b21c783..de3e9cfe17 100644 --- a/src/assets/icons/delete/delete-selection-icon.svg +++ b/src/assets/icons/delete/delete-selection-icon.svg @@ -1,14 +1,6 @@ - - - AE939FE4-E1D6-490E-9469-CEE71D1653CA - - - - - - - - - + + + + - \ No newline at end of file + diff --git a/src/clue/assets/icons/delete-tool.svg b/src/clue/assets/icons/delete-tool.svg index 9f4fab2737..fda35ec412 100644 --- a/src/clue/assets/icons/delete-tool.svg +++ b/src/clue/assets/icons/delete-tool.svg @@ -1,6 +1,5 @@ - + - - + diff --git a/src/plugins/data-card/assets/delete-selection-icon.svg b/src/plugins/data-card/assets/delete-selection-icon.svg index 7632313c5c..fda35ec412 100644 --- a/src/plugins/data-card/assets/delete-selection-icon.svg +++ b/src/plugins/data-card/assets/delete-selection-icon.svg @@ -1,13 +1,5 @@ - - - - - - - - - - - + + + - \ No newline at end of file + diff --git a/src/plugins/graph/assets/remove-data-icon.svg b/src/plugins/graph/assets/remove-data-icon.svg index 58c3721f15..4d1dbec71e 100644 --- a/src/plugins/graph/assets/remove-data-icon.svg +++ b/src/plugins/graph/assets/remove-data-icon.svg @@ -1,7 +1,6 @@ - - - + + From cddfb9ff5c438a6535ae20805d948f1e9f7af0a7 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 19 Mar 2024 21:06:38 -0400 Subject: [PATCH 72/78] Added dynamic centering function (cherry picked from commit fe0ea28bd897b641d1f497fd54d10d53e1630d58) --- .../numberline/components/editable-numberline-value.tsx | 7 ++++--- src/plugins/numberline/components/numberline-tile.scss | 2 ++ src/plugins/numberline/components/numberline-tile.tsx | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/plugins/numberline/components/editable-numberline-value.tsx b/src/plugins/numberline/components/editable-numberline-value.tsx index de7bfcce77..5d12a3b4b0 100644 --- a/src/plugins/numberline/components/editable-numberline-value.tsx +++ b/src/plugins/numberline/components/editable-numberline-value.tsx @@ -8,13 +8,13 @@ interface IEditableValueProps { readOnly?: boolean; isTileSelected: boolean; value: number; - offset: number; + arrowOffset: number; minOrMax: "min" | "max"; onValueChange: (newValue: number) => void; } export const EditableNumberlineValue: React.FC = observer(function NumberlineTile(props) { - const { readOnly, isTileSelected, value, offset, minOrMax, onValueChange } = props; + const { readOnly, isTileSelected, value, arrowOffset, minOrMax, onValueChange } = props; const [isEditing, setIsEditing] = useState(false); const inputRef = useRef(null); @@ -58,7 +58,8 @@ export const EditableNumberlineValue: React.FC = observer(f }; //----------------------- Determine Styling for Border Box ----------------------- - const borderBoxOffset = `${offset + 4}px`; + const numCharToOffset = -3 * value.toString().length + 7; //additional offset to center value with tick + const borderBoxOffset = `${arrowOffset + numCharToOffset}px`; const borderBoxStyle = (minOrMax === "min") ? { left: borderBoxOffset } : { right: borderBoxOffset }; const borderClasses = classNames("border-box", {hide: !isTileSelected}); diff --git a/src/plugins/numberline/components/numberline-tile.scss b/src/plugins/numberline/components/numberline-tile.scss index 6d00db4047..8521389a99 100644 --- a/src/plugins/numberline/components/numberline-tile.scss +++ b/src/plugins/numberline/components/numberline-tile.scss @@ -89,6 +89,8 @@ margin-top: 8px; border: 1.5px solid #949494; background-color: #f0f9fb; + padding-left: 5px; + padding-right: 5px; &.hide{ border: none; diff --git a/src/plugins/numberline/components/numberline-tile.tsx b/src/plugins/numberline/components/numberline-tile.tsx index edcc9edbb1..a51382b004 100644 --- a/src/plugins/numberline/components/numberline-tile.tsx +++ b/src/plugins/numberline/components/numberline-tile.tsx @@ -395,7 +395,7 @@ export const NumberlineTile: React.FC = observer(function Numberline handleMinMaxChange("min", newValue)} @@ -403,7 +403,7 @@ export const NumberlineTile: React.FC = observer(function Numberline handleMinMaxChange("max", newValue)} From ab1b3f4a10497d5b02631b36ac14a5d2a7954bd8 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Tue, 19 Mar 2024 22:31:49 -0400 Subject: [PATCH 73/78] fix clipping in expression toolbar, replace numberline delete icon --- src/plugins/expression/expression-toolbar.scss | 3 ++- .../numberline/assets/numberline-toolbar-delete-icon.svg | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plugins/expression/expression-toolbar.scss b/src/plugins/expression/expression-toolbar.scss index 8be2eab743..7948b60eb9 100644 --- a/src/plugins/expression/expression-toolbar.scss +++ b/src/plugins/expression/expression-toolbar.scss @@ -9,10 +9,10 @@ } button { - height: 100%; width: 37px; background-color: white; border: none; + vertical-align: top; cursor: pointer; padding:4px; &:hover { @@ -28,6 +28,7 @@ .delete-expression svg { transform: translate(-3px, -2px); + overflow: visible; } .mixed-fraction svg { transform: scale(0.9) translate(-2px, 2px); diff --git a/src/plugins/numberline/assets/numberline-toolbar-delete-icon.svg b/src/plugins/numberline/assets/numberline-toolbar-delete-icon.svg index c68a0b5161..ee6e68efdb 100644 --- a/src/plugins/numberline/assets/numberline-toolbar-delete-icon.svg +++ b/src/plugins/numberline/assets/numberline-toolbar-delete-icon.svg @@ -1,6 +1,5 @@ - - - - + + + From 77156b96b59c8500eea96a4283db29352c8a39b9 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Wed, 20 Mar 2024 14:01:50 -0400 Subject: [PATCH 74/78] Address PR comments. --- src/plugins/graph/components/scatterdots.tsx | 6 +++--- src/plugins/graph/hooks/use-plot.ts | 3 +-- .../plotted-variables-adornment-component.tsx | 12 ++++++------ 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/plugins/graph/components/scatterdots.tsx b/src/plugins/graph/components/scatterdots.tsx index e3dbdd754c..7e14e0fc56 100644 --- a/src/plugins/graph/components/scatterdots.tsx +++ b/src/plugins/graph/components/scatterdots.tsx @@ -76,7 +76,7 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { }, [layout, dataConfiguration, dataset]), onDrag = useCallback((event: D3DragEvent) => { - if (event.dx!==0 || event.dy!==0) { + if (event.dx !== 0 || event.dy !== 0) { updatePositions(event.dx, event.dy); } }, [updatePositions]), @@ -90,8 +90,8 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { selectGraphDots(dotsRef.current) ?.call(drag() - .filter(() => { return graphModel.editingMode !== "none"; }) - .subject((event) => { return { x: event.x, y: event.y }; }) + .filter(() => graphModel.editingMode !== "none") + .subject((event) => ({ x: event.x, y: event.y })) .on('start', onDragStart) .on('drag', onDrag) .on('end', onDragEnd) diff --git a/src/plugins/graph/hooks/use-plot.ts b/src/plugins/graph/hooks/use-plot.ts index 7f70f8f774..25d213752e 100644 --- a/src/plugins/graph/hooks/use-plot.ts +++ b/src/plugins/graph/hooks/use-plot.ts @@ -155,8 +155,7 @@ export const usePlotResponders = (props: IPlotResponderProps) => { dataConfiguration ); return () => disposer(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [callRefreshPointPositions, dataConfiguration]); + }, [callRefreshPointPositions, dataConfiguration, enableAnimation]); // respond to axis range changes (e.g. component resizing) useEffect(() => { diff --git a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx index e37f2070fa..747313574f 100644 --- a/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx +++ b/src/plugins/shared-variables/graph/plotted-variables-adornment/plotted-variables-adornment-component.tsx @@ -1,5 +1,6 @@ import React, { useCallback, useContext, useEffect, useMemo, useRef } from "react"; import { drag, select, Selection } from "d3"; +import { comparer } from "mobx"; import { observer } from "mobx-react-lite"; import { VariableType } from "@concord-consortium/diagram-view"; @@ -243,8 +244,7 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria model); }, [model, plotWidth, plotHeight, xAxis, yAxis, refreshValues, controller]); - // Refresh display when variable values change - useEffect(function udpateOnValuesChange() { + useEffect(function updateOnValuesChange() { return mstReaction( () => { let hasAnyVars = false; @@ -267,10 +267,10 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria refreshValues(); } }, - { name: "PlottedVariablesAdornmentComponent.rescaleOnValuesChange" }, + { name: "PlottedVariablesAdornmentComponent.rescaleOnValuesChange", + equals: comparer.structural }, model); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [controller, model]); + }, [controller, graphModel, model, refreshValues, settings]); // Scale graph when a new X or Y variable is selected useEffect(function scaleOnVariableChange() { @@ -284,7 +284,7 @@ export const PlottedVariablesAdornmentComponent = observer(function PlottedVaria }, { name: "PlottedVariablesAdornmentComponent.scaleOnVariableChange" }, model); - }, [graphModel.lockAxes, model, xAxis, yAxis, controller]); + }, [graphModel, model, xAxis, yAxis, controller]); return ( From b8ea9e267008cde4dfc927f5933ec76914dfde56 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 21 Mar 2024 09:49:57 -0400 Subject: [PATCH 75/78] Update drag handlers (too often) --- .../tile_tests/xy_plot_tool_spec.js | 23 ++++++++------- src/plugins/graph/components/scatterdots.tsx | 28 +++++++++++-------- src/plugins/graph/utilities/graph-utils.ts | 1 + 3 files changed, 29 insertions(+), 23 deletions(-) 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 9d1ec930bf..9cb791a520 100644 --- a/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js @@ -567,19 +567,18 @@ 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); + clueCanvas.clickToolbarButton('graph', 'toggle-lock'); // so that we can test position without rescale happening - // FIXME: I cannot make this test work. It used to work before switching to d3-drag for the points. - // cy.window().then(win => { - // xyTile.getGraphDot().eq(0) - // .trigger("mousedown", { force: true, eventConstructor: 'MouseEvent', button: 0, view: win, clientX: 150, clientY: 50 }) - // .trigger("dragstart", { force: true, eventConstructor: 'DragEvent' }) - // .trigger("drag") - // .trigger("mousemove", { force: true, eventConstructor: 'MouseEvent', clientX: 175, clientY: 75 }) - // .trigger("mouseup", 175, 75, { force: true, eventConstructor: 'MouseEvent', clientX: 175, clientY: 75, view: win }); - // }); - // cy.wait(1000); - // xAttributeOfTransform(xyTile.getGraphDot().eq(0)).should("be.closeTo", 175, 10); - // yAttributeOfTransform(xyTile.getGraphDot().eq(0)).should("be.closeTo", 75, 10); + cy.window().then(win => { + xyTile.getGraphDot().eq(0).children('circle').eq(1) + .trigger("mousedown", 150, 50, { force: true, view: win }) + .trigger("mousemove", 175, 75, { force: true, view: win }) + .trigger("mouseup", 175, 75, { force: true, view: win }); + }); + + 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'); diff --git a/src/plugins/graph/components/scatterdots.tsx b/src/plugins/graph/components/scatterdots.tsx index 7e14e0fc56..59b488eb41 100644 --- a/src/plugins/graph/components/scatterdots.tsx +++ b/src/plugins/graph/components/scatterdots.tsx @@ -39,6 +39,7 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { const onDragStart = useCallback((event: D3DragEvent, datum) => { + console.log('onDragStart'); const targetDot = event.sourceEvent.target && inGraphDot(event.sourceEvent.target as SVGSVGElement); if (!targetDot) return; target.current = select(targetDot as SVGSVGElement); @@ -86,16 +87,19 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { // Final update does a rescale if appropriate updatePositions(event.dx, event.dy); target.current = null; - }, [graphModel, updatePositions]); - - selectGraphDots(dotsRef.current) - ?.call(drag() - .filter(() => graphModel.editingMode !== "none") - .subject((event) => ({ x: event.x, y: event.y })) - .on('start', onDragStart) - .on('drag', onDrag) - .on('end', onDragEnd) - ); + }, [graphModel, updatePositions]), + + updateDragHandler = useCallback(() => { + console.log('updateDragHandlers', dotsRef); + selectGraphDots(dotsRef.current) + ?.call(drag() + .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; @@ -108,6 +112,7 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { }, [dataConfiguration, dotsRef, graphModel]); const refreshPointPositionsD3 = useCallback((selectedOnly: boolean) => { + console.log("refreshPointPos"); if (!dataConfiguration) return; const getScreenX = (anID: string) => { const xAttrID = dataConfiguration?.attributeID('x') ?? '', @@ -146,8 +151,9 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { selectedOnly, getScreenX, getScreenY, getLegendColor, getColorForId: graphModel.getColorForId, enableAnimation, pointColor, pointStrokeColor }); + updateDragHandler(); }, [dataConfiguration, dataset, dotsRef, layout, legendAttrID, - enableAnimation, graphModel, yScaleRef]); + enableAnimation, graphModel, yScaleRef, updateDragHandler]); // const refreshPointPositionsSVG = useCallback((selectedOnly: boolean) => { // const xAttrID = dataConfiguration?.attributeID('x') ?? '', diff --git a/src/plugins/graph/utilities/graph-utils.ts b/src/plugins/graph/utilities/graph-utils.ts index f8bf253c03..500493c0b6 100644 --- a/src/plugins/graph/utilities/graph-utils.ts +++ b/src/plugins/graph/utilities/graph-utils.ts @@ -225,6 +225,7 @@ export interface IMatchCirclesProps { } export function matchCirclesToData(props: IMatchCirclesProps) { + console.log("MCTD"); const { dataConfiguration, enableAnimation, instanceId, dotsElement } = props; const allCaseData = dataConfiguration.joinedCaseDataArrays; const caseDataKeyFunc = (d: CaseData) => `${d.dataConfigID}_${instanceId}_${d.plotNum}_${d.caseID}`; From a211ebe0378e3d0b51d923d16968f2c555c01076 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 21 Mar 2024 10:19:01 -0400 Subject: [PATCH 76/78] Fix test --- .../tile_tests/xy_plot_tool_spec.js | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) 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 9cb791a520..edc6f74ff4 100644 --- a/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js @@ -569,17 +569,20 @@ context('XYPlot Tool Tile', function () { yAttributeOfTransform(xyTile.getGraphDot().eq(0)).should("be.closeTo", 50, 10); clueCanvas.clickToolbarButton('graph', 'toggle-lock'); // so that we can test position without rescale happening - cy.window().then(win => { - xyTile.getGraphDot().eq(0).children('circle').eq(1) - .trigger("mousedown", 150, 50, { force: true, view: win }) - .trigger("mousemove", 175, 75, { force: true, view: win }) - .trigger("mouseup", 175, 75, { force: true, view: win }); + 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 }); - 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'); clueCanvas.toolbarButtonIsNotSelected('graph', 'move-points'); From 49b920f5fcdea0519a93d5008ee0e2e0a91f337f Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 21 Mar 2024 14:50:30 -0400 Subject: [PATCH 77/78] Remove extra logging --- src/plugins/graph/components/scatterdots.tsx | 3 --- src/plugins/graph/utilities/graph-utils.ts | 1 - 2 files changed, 4 deletions(-) diff --git a/src/plugins/graph/components/scatterdots.tsx b/src/plugins/graph/components/scatterdots.tsx index 59b488eb41..6a4231c475 100644 --- a/src/plugins/graph/components/scatterdots.tsx +++ b/src/plugins/graph/components/scatterdots.tsx @@ -39,7 +39,6 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { const onDragStart = useCallback((event: D3DragEvent, datum) => { - console.log('onDragStart'); const targetDot = event.sourceEvent.target && inGraphDot(event.sourceEvent.target as SVGSVGElement); if (!targetDot) return; target.current = select(targetDot as SVGSVGElement); @@ -90,7 +89,6 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { }, [graphModel, updatePositions]), updateDragHandler = useCallback(() => { - console.log('updateDragHandlers', dotsRef); selectGraphDots(dotsRef.current) ?.call(drag() .filter(() => graphModel.editingMode !== "none") @@ -112,7 +110,6 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { }, [dataConfiguration, dotsRef, graphModel]); const refreshPointPositionsD3 = useCallback((selectedOnly: boolean) => { - console.log("refreshPointPos"); if (!dataConfiguration) return; const getScreenX = (anID: string) => { const xAttrID = dataConfiguration?.attributeID('x') ?? '', diff --git a/src/plugins/graph/utilities/graph-utils.ts b/src/plugins/graph/utilities/graph-utils.ts index 500493c0b6..f8bf253c03 100644 --- a/src/plugins/graph/utilities/graph-utils.ts +++ b/src/plugins/graph/utilities/graph-utils.ts @@ -225,7 +225,6 @@ export interface IMatchCirclesProps { } export function matchCirclesToData(props: IMatchCirclesProps) { - console.log("MCTD"); const { dataConfiguration, enableAnimation, instanceId, dotsElement } = props; const allCaseData = dataConfiguration.joinedCaseDataArrays; const caseDataKeyFunc = (d: CaseData) => `${d.dataConfigID}_${instanceId}_${d.plotNum}_${d.caseID}`; From 2ca6ef7a585be0891b00838ad2954cd3cbc3b9e2 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 22 Mar 2024 10:56:41 -0400 Subject: [PATCH 78/78] Naming consistency --- src/plugins/graph/components/scatterdots.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/graph/components/scatterdots.tsx b/src/plugins/graph/components/scatterdots.tsx index 6a4231c475..409f6be183 100644 --- a/src/plugins/graph/components/scatterdots.tsx +++ b/src/plugins/graph/components/scatterdots.tsx @@ -88,7 +88,7 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { target.current = null; }, [graphModel, updatePositions]), - updateDragHandler = useCallback(() => { + refreshDragHandlers = useCallback(() => { selectGraphDots(dotsRef.current) ?.call(drag() .filter(() => graphModel.editingMode !== "none") @@ -148,9 +148,9 @@ export const ScatterDots = function ScatterDots(props: PlotProps) { selectedOnly, getScreenX, getScreenY, getLegendColor, getColorForId: graphModel.getColorForId, enableAnimation, pointColor, pointStrokeColor }); - updateDragHandler(); + refreshDragHandlers(); }, [dataConfiguration, dataset, dotsRef, layout, legendAttrID, - enableAnimation, graphModel, yScaleRef, updateDragHandler]); + enableAnimation, graphModel, yScaleRef, refreshDragHandlers]); // const refreshPointPositionsSVG = useCallback((selectedOnly: boolean) => { // const xAttrID = dataConfiguration?.attributeID('x') ?? '',