From f36fa6190f858a788580396ded3054ed2c38f5cb Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 5 Sep 2024 13:26:18 -0400 Subject: [PATCH 01/36] Remove inadvertent dependency on dataflow --- .../dataflow/assets/icons => assets}/dropdown-caret.svg | 0 src/plugins/bar-graph/bar-graph-chart.tsx | 6 +++--- src/plugins/bar-graph/category-pulldown.tsx | 2 +- .../dataflow/nodes/controls/dropdown-list-control.tsx | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename src/{plugins/dataflow/assets/icons => assets}/dropdown-caret.svg (100%) diff --git a/src/plugins/dataflow/assets/icons/dropdown-caret.svg b/src/assets/dropdown-caret.svg similarity index 100% rename from src/plugins/dataflow/assets/icons/dropdown-caret.svg rename to src/assets/dropdown-caret.svg diff --git a/src/plugins/bar-graph/bar-graph-chart.tsx b/src/plugins/bar-graph/bar-graph-chart.tsx index 42e15c71bd..6fb723d0d8 100644 --- a/src/plugins/bar-graph/bar-graph-chart.tsx +++ b/src/plugins/bar-graph/bar-graph-chart.tsx @@ -1,11 +1,11 @@ +import React, { useMemo } from "react"; +import { observer } from "mobx-react"; +import { isNumber } from "lodash"; import { AxisBottom, AxisLeft } from "@visx/axis"; import { GridRows } from "@visx/grid"; import { Group } from "@visx/group"; import { scaleBand, scaleLinear } from "@visx/scale"; import { Bar, BarGroup } from "@visx/shape"; -import { isNumber } from "lodash"; -import { observer } from "mobx-react"; -import React, { useMemo } from "react"; import { clueDataColorInfo } from "../../utilities/color-utils"; import { useBarGraphModelContext } from "./bar-graph-content-context"; import { CategoryPulldown } from "./category-pulldown"; diff --git a/src/plugins/bar-graph/category-pulldown.tsx b/src/plugins/bar-graph/category-pulldown.tsx index d3a78264c5..c740e24249 100644 --- a/src/plugins/bar-graph/category-pulldown.tsx +++ b/src/plugins/bar-graph/category-pulldown.tsx @@ -2,7 +2,7 @@ import React from "react"; import { Menu, MenuButton, MenuItem, MenuList, Portal } from "@chakra-ui/react"; import { useReadOnlyContext } from "../../components/document/read-only-context"; -import DropdownCaretIcon from "../dataflow/assets/icons/dropdown-caret.svg"; +import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; interface IProps { diff --git a/src/plugins/dataflow/nodes/controls/dropdown-list-control.tsx b/src/plugins/dataflow/nodes/controls/dropdown-list-control.tsx index f9c9d07cb5..87aedf9c70 100644 --- a/src/plugins/dataflow/nodes/controls/dropdown-list-control.tsx +++ b/src/plugins/dataflow/nodes/controls/dropdown-list-control.tsx @@ -6,7 +6,7 @@ import classNames from "classnames"; import { useStopEventPropagation, useCloseDropdownOnOutsideEvent } from "./custom-hooks"; import { IBaseNode, IBaseNodeModel } from "../base-node"; -import DropdownCaretIcon from "../../assets/icons/dropdown-caret.svg"; +import DropdownCaretIcon from "../../../../assets/dropdown-caret.svg"; import "./dropdown-list-control.scss"; From c242d5922c173dee1fd11d7fd48091ff7b4686af Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 5 Sep 2024 16:24:08 -0400 Subject: [PATCH 02/36] Toolbar, basic linking --- src/clue/app-config.json | 5 +++ src/hooks/use-provider-tile-linking.tsx | 18 +++++--- src/plugins/bar-graph/bar-graph-chart.tsx | 47 ++++++--------------- src/plugins/bar-graph/bar-graph-content.ts | 44 +++++++++++++++++++ src/plugins/bar-graph/bar-graph-tile.tsx | 4 ++ src/plugins/bar-graph/bar-graph-toolbar.tsx | 40 ++++++++++++++++++ src/plugins/bar-graph/category-pulldown.tsx | 18 +++++--- 7 files changed, 131 insertions(+), 45 deletions(-) create mode 100644 src/plugins/bar-graph/bar-graph-toolbar.tsx diff --git a/src/clue/app-config.json b/src/clue/app-config.json index c2a22e074d..e0868b88bb 100644 --- a/src/clue/app-config.json +++ b/src/clue/app-config.json @@ -241,6 +241,11 @@ "placeholderText": "", "stamps": [], "settings": { + "bargraph": { + "tools": [ + "link-tile" + ] + }, "dataset": { "cellsSelectCases": false }, diff --git a/src/hooks/use-provider-tile-linking.tsx b/src/hooks/use-provider-tile-linking.tsx index 314e050acc..7ed3c562b6 100644 --- a/src/hooks/use-provider-tile-linking.tsx +++ b/src/hooks/use-provider-tile-linking.tsx @@ -5,8 +5,9 @@ import { isGraphModel } from "../plugins/graph/models/graph-model"; import { getSharedModelManager } from "../models/tiles/tile-environment"; import { SharedModelType } from "../models/shared/shared-model"; import { LogEventName } from "../lib/logger-types"; - import { logSharedModelDocEvent } from "../models/document/log-shared-model-document-event"; +import { getTileContentInfo } from "../models/tiles/tile-content-info"; +import { useAppConfig } from "./use-stores"; interface IProps { actionHandlers?: any; @@ -36,6 +37,7 @@ interface IProps { export const useProviderTileLinking = ({ actionHandlers, model, readOnly, sharedModelTypes, allowMultipleGraphDatasets }: IProps) => { + const appConfig = useAppConfig(); const {handleRequestTileLink, handleRequestTileUnlink} = actionHandlers || {}; const sharedModelManager = getSharedModelManager(model); const sharedModels: SharedModelType[] = []; @@ -55,10 +57,14 @@ export const useProviderTileLinking = ({ const linkTile = useCallback((sharedModel: SharedModelType) => { if (!readOnly && sharedModelManager?.isReady) { - // TODO: this is temporary while we are working on getting Graph to work with multiple datasets - // Once multiple datasets are fully implemented, we should look at the "consumesMultipleDataSets" - // setting for the tile type; but for now graph has to allow multiples while not having that be the default. - if (!allowMultipleGraphDatasets && isGraphModel(model.content)) { + // Depending on the unit configuration, graphs sometimes allow multiple datasets and sometimes not. + // Other tiles register their ability to consume multiple datasets as part of their content info. + const allowsMultiple = isGraphModel(model.content) + ? allowMultipleGraphDatasets + : getTileContentInfo(model.content.type)?.consumesMultipleDataSets?.(appConfig); + + if (!allowsMultiple) { + // Remove any existing shared models before adding the new one for (const shared of sharedModelManager.getTileSharedModels(model.content)) { sharedModelManager.removeTileSharedModel(model.content, shared); } @@ -68,7 +74,7 @@ export const useProviderTileLinking = ({ logSharedModelDocEvent(LogEventName.TILE_LINK, model, sharedTiles, sharedModel); } - }, [readOnly, sharedModelManager, model, allowMultipleGraphDatasets]); + }, [appConfig, readOnly, sharedModelManager, model, allowMultipleGraphDatasets]); const unlinkTile = useCallback((sharedModel: SharedModelType) => { if (!readOnly && sharedModelManager?.isReady) { diff --git a/src/plugins/bar-graph/bar-graph-chart.tsx b/src/plugins/bar-graph/bar-graph-chart.tsx index 6fb723d0d8..78dc9bf9f8 100644 --- a/src/plugins/bar-graph/bar-graph-chart.tsx +++ b/src/plugins/bar-graph/bar-graph-chart.tsx @@ -18,24 +18,6 @@ const margin = { right: 80, }; -const demoCases: Record[] = [ - { date: '6/23/24', location: 'deck' }, - { date: '6/23/24', location: 'porch' }, - { date: '6/23/24', location: 'tree' }, - { date: '6/24/24', location: 'porch' }, - { date: '6/24/24', location: 'porch' }, - { date: '6/25/24', location: 'backyard' }, - { date: '6/25/24', location: 'deck' }, - { date: '6/25/24', location: 'deck' }, - { date: '6/25/24', location: 'deck' }, - { date: '6/25/24', location: 'tree' }, - { date: '6/26/24', location: 'backyard' }, - { date: '6/26/24', location: 'deck' }, - { date: '6/26/24', location: 'deck' }, - { date: '6/26/24', location: 'porch' }, - { date: '6/26/24', location: 'tree' } -]; - function roundTo5(n: number): number { return Math.ceil(n/5)*5; } @@ -57,27 +39,26 @@ interface IBarGraphChartProps { export const BarGraphChart = observer(function BarGraphChart({ width, height }: IBarGraphChartProps) { const model = useBarGraphModelContext(); + model?.cacheSharedDataSet(); // FIXME should be just on initialization const primary = model?.primaryAttribute || "date"; - const secondary = model?.secondaryAttribute || "location"; + const secondary = model?.secondaryAttribute || "location"; // FIXME const xMax = width - margin.left - margin.right; const yMax = height - margin.top - margin.bottom; function setDemoCategory(catname: string) { - if (catname === "date") { - model?.setPrimaryAttribute("date"); - model?.setSecondaryAttribute("location"); - } else{ - model?.setPrimaryAttribute("location"); - model?.setSecondaryAttribute("date"); - } + model?.setPrimaryAttribute(catname); + model?.setSecondaryAttribute(""); } // Count cases and make the data array const data = useMemo( - () => demoCases.reduce((acc, row) => { - const cat = primary in row ? row[primary] : ""; - const subCat = row[secondary] || ""; + () => { + const dataSet = model?.dataSet; + if (!dataSet || !primary || !model.cases) return []; + return model.cases.reduce((acc, caseID) => { + const cat = dataSet?.dataSet.getStrValue(caseID.__id__, primary); + const subCat = "A"; const index = acc.findIndex(r => r[primary] === cat); if (index >= 0) { const cur = acc[index][subCat]; @@ -87,8 +68,10 @@ export const BarGraphChart = observer(function BarGraphChart({ width, height }: acc.push(newRow); } return acc; - }, [] as { [key: string]: number|string }[]), - [primary, secondary]); + }, [] as { [key: string]: number|string }[]); + }, + [model?.cases, model?.dataSet, primary]); + console.log(data); const primaryKeys: string[] = useMemo(() => data.map(d => d[primary] as string), @@ -201,8 +184,6 @@ export const BarGraphChart = observer(function BarGraphChart({ width, height }: setText={(text) => model?.setYAxisLabel(text)} /> ({ + dataSet: undefined as SharedDataSetType|undefined + })) .views(self => ({ get isUserResizable() { return true; + }, + get cases() { + return self.dataSet?.dataSet.cases; + }, + /** Query the SharedModelManager to find a connected DataSet */ + sharedModelDataSet() { + const smm = getSharedModelManager(self); + if (!smm || !smm.isReady) return; + const sharedDataSets = smm.getTileSharedModelsByType(self, SharedDataSet); + console.log('sharedDataSets',sharedDataSets); + if (sharedDataSets.length > 0 && isSharedDataSet(sharedDataSets[0])) { + return sharedDataSets[0]; + } else { + return undefined; + } } })) .actions(self => ({ @@ -29,6 +50,29 @@ export const BarGraphContentModel = TileContentModel setSecondaryAttribute(attrId: string) { self.secondaryAttribute = attrId; } + })) + .actions(self => ({ + /** + * Sets the volatile self.dataSet property if it hasn't been set yet. + */ + cacheSharedDataSet() { + if (!self.dataSet) { + self.dataSet = self.sharedModelDataSet(); + } + }, + updateAfterSharedModelChanges(sharedModel?: SharedModelType) { + const dataSet = self.sharedModelDataSet(); + if (self.dataSet !== dataSet) { + self.dataSet = dataSet; + if (dataSet) { + const atts = dataSet.dataSet.attributes; + if (atts.length > 0) { + self.setPrimaryAttribute(atts[0].id); + console.log('set primary attribute to',atts[0].name); + } + } + } + } })); export interface BarGraphContentModelType extends Instance {} diff --git a/src/plugins/bar-graph/bar-graph-tile.tsx b/src/plugins/bar-graph/bar-graph-tile.tsx index d307016faf..a34f481338 100644 --- a/src/plugins/bar-graph/bar-graph-tile.tsx +++ b/src/plugins/bar-graph/bar-graph-tile.tsx @@ -8,9 +8,12 @@ import { ITileProps } from "../../components/tiles/tile-component"; import { BarGraphChart } from "./bar-graph-chart"; import { BarGraphModelContext } from "./bar-graph-content-context"; import { isBarGraphModel } from "./bar-graph-content"; +import { TileToolbar } from "../../components/toolbar/tile-toolbar"; import "./bar-graph.scss"; +import "./bar-graph-toolbar"; + export const BarGraphComponent: React.FC = observer((props: ITileProps) => { const {height: resizeHeight, width: resizeWidth, ref} = useResizeDetector(); @@ -23,6 +26,7 @@ export const BarGraphComponent: React.FC = observer((props: ITilePro return ( +
{ + isLinkEnabled && showLinkTileDialog(); + e.stopPropagation(); + }; + + return ( + + + + ); +} + +registerTileToolbarButtons("bargraph", +[ + { + name: 'link-tile', + component: LinkTileButton + } +]); diff --git a/src/plugins/bar-graph/category-pulldown.tsx b/src/plugins/bar-graph/category-pulldown.tsx index c740e24249..585401f6e2 100644 --- a/src/plugins/bar-graph/category-pulldown.tsx +++ b/src/plugins/bar-graph/category-pulldown.tsx @@ -3,11 +3,10 @@ import { Menu, MenuButton, MenuItem, MenuList, Portal } from "@chakra-ui/react"; import { useReadOnlyContext } from "../../components/document/read-only-context"; import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; +import { useBarGraphModelContext } from "./bar-graph-content-context"; interface IProps { - categoryList: string[]; - category: string; setCategory: (category: string) => void; x: number; y: number; @@ -15,22 +14,29 @@ interface IProps { height: number; } -export function CategoryPulldown({categoryList, category, setCategory, x, y, width, height}: IProps) { +export function CategoryPulldown({setCategory, x, y, width, height}: IProps) { const readOnly = useReadOnlyContext(); + const model = useBarGraphModelContext(); + + const dataSet = model?.dataSet?.dataSet; + const attributes = dataSet?.attributes || []; + const current = (dataSet && model.primaryAttribute) + ? dataSet.attrFromID(model.primaryAttribute)?.name + : "Choose one"; return ( - {category} + {current} - {categoryList.map((c) => ( - setCategory(c)}>{c} + {attributes.map((a) => ( + setCategory(a.id)}>{a.name} ))} From 0f6418672703c5c94dc22deaf65b6644808e01a0 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 6 Sep 2024 11:23:10 -0400 Subject: [PATCH 03/36] Basic legend implementation. Link/unlink handling. --- .../graph => }/assets/remove-data-icon.svg | 0 src/plugins/bar-graph/bar-graph-chart.tsx | 13 ++-- src/plugins/bar-graph/bar-graph-content.ts | 43 ++++++++---- src/plugins/bar-graph/bar-graph-legend.tsx | 40 +++++++++++ src/plugins/bar-graph/bar-graph-tile.test.tsx | 68 ------------------- src/plugins/bar-graph/bar-graph-tile.tsx | 20 ++++-- src/plugins/bar-graph/bar-graph.scss | 64 +++++++++++++++-- .../graph/components/legend/layer-legend.tsx | 2 +- .../graph/legend/variable-function-legend.tsx | 2 +- 9 files changed, 152 insertions(+), 100 deletions(-) rename src/{plugins/graph => }/assets/remove-data-icon.svg (100%) create mode 100644 src/plugins/bar-graph/bar-graph-legend.tsx delete mode 100644 src/plugins/bar-graph/bar-graph-tile.test.tsx diff --git a/src/plugins/graph/assets/remove-data-icon.svg b/src/assets/remove-data-icon.svg similarity index 100% rename from src/plugins/graph/assets/remove-data-icon.svg rename to src/assets/remove-data-icon.svg diff --git a/src/plugins/bar-graph/bar-graph-chart.tsx b/src/plugins/bar-graph/bar-graph-chart.tsx index 78dc9bf9f8..f8851f702c 100644 --- a/src/plugins/bar-graph/bar-graph-chart.tsx +++ b/src/plugins/bar-graph/bar-graph-chart.tsx @@ -12,10 +12,10 @@ import { CategoryPulldown } from "./category-pulldown"; import EditableAxisLabel from "./editable-axis-label"; const margin = { - top: 60, - bottom: 60, - left: 80, - right: 80, + top: 7, + bottom: 70, + left: 70, + right: 10, }; function roundTo5(n: number): number { @@ -39,7 +39,6 @@ interface IBarGraphChartProps { export const BarGraphChart = observer(function BarGraphChart({ width, height }: IBarGraphChartProps) { const model = useBarGraphModelContext(); - model?.cacheSharedDataSet(); // FIXME should be just on initialization const primary = model?.primaryAttribute || "date"; const secondary = model?.secondaryAttribute || "location"; // FIXME @@ -111,13 +110,13 @@ export const BarGraphChart = observer(function BarGraphChart({ width, height }: }), [yMax, maxValue]); - if (xMax <= 0 || yMax <= 0) return Too small; + if (xMax <= 0 || yMax <= 0) return Too small ({width}x{height}); const ticks = Math.min(4, Math.floor(yMax/40)); // leave generous vertical space (>=40 px) between ticks const labelWidth = (xMax/primaryKeys.length)-10; // setting width will wrap lines in labels when needed return ( - + 0 && isSharedDataSet(sharedDataSets[0])) { return sharedDataSets[0]; } else { @@ -49,26 +50,42 @@ export const BarGraphContentModel = TileContentModel }, setSecondaryAttribute(attrId: string) { self.secondaryAttribute = attrId; + }, + setSharedDataSet() { + self.dataSet = self.sharedModelDataSet(getSharedModelManager(self)); } })) .actions(self => ({ - /** - * Sets the volatile self.dataSet property if it hasn't been set yet. - */ - cacheSharedDataSet() { - if (!self.dataSet) { - self.dataSet = self.sharedModelDataSet(); + unlinkDataSet() { + const smm = getSharedModelManager(self); + if (!smm || !smm.isReady) return; + const sharedDataSets = smm.getTileSharedModelsByType(self, SharedDataSet); + for (const sharedDataSet of sharedDataSets) { + smm.removeTileSharedModel(self, sharedDataSet); } + self.dataSet = undefined; + }, + afterAttach() { + // After attached to document & SMM is ready, cache a reference to our dataset. + addDisposer(self, reaction( + () => { + return self.tileEnv?.sharedModelManager?.isReady; + }, + (ready) => { + if (!ready) return; + self.setSharedDataSet(); + }, { fireImmediately: true } + )); }, updateAfterSharedModelChanges(sharedModel?: SharedModelType) { - const dataSet = self.sharedModelDataSet(); + // When new dataset is attached, cache a reference to it and pick a category attribute. + const dataSet = self.sharedModelDataSet(getSharedModelManager(self)); if (self.dataSet !== dataSet) { self.dataSet = dataSet; if (dataSet) { const atts = dataSet.dataSet.attributes; if (atts.length > 0) { self.setPrimaryAttribute(atts[0].id); - console.log('set primary attribute to',atts[0].name); } } } diff --git a/src/plugins/bar-graph/bar-graph-legend.tsx b/src/plugins/bar-graph/bar-graph-legend.tsx new file mode 100644 index 0000000000..5324a5d487 --- /dev/null +++ b/src/plugins/bar-graph/bar-graph-legend.tsx @@ -0,0 +1,40 @@ +import React from 'react'; +import { observer } from 'mobx-react'; +import { useBarGraphModelContext } from './bar-graph-content-context'; + +import RemoveDataIcon from "../../assets/remove-data-icon.svg"; + +export const BarGraphLegend = observer(function BarGraphLegend () { + const model = useBarGraphModelContext(); + + function unlinkDataset() { + if (model) { + model.unlinkDataSet(); + } + } + + + if (!model || !model.dataSet) { + return null; + } + + return ( +
+
+
+ + + +
+
+ Data from: + {model.dataSet.name} +
+
+ +
+ Sort by: +
+
+ ); +}); diff --git a/src/plugins/bar-graph/bar-graph-tile.test.tsx b/src/plugins/bar-graph/bar-graph-tile.test.tsx deleted file mode 100644 index df552f64dd..0000000000 --- a/src/plugins/bar-graph/bar-graph-tile.test.tsx +++ /dev/null @@ -1,68 +0,0 @@ -import React from "react"; -import { render, getByText as globalGetByText } from "@testing-library/react"; - -import { ITileApi } from "../../components/tiles/tile-api"; -import { TileModel } from "../../models/tiles/tile-model"; -import { defaultBarGraphContent } from "./bar-graph-content"; -import { BarGraphComponent } from "./bar-graph-tile"; - -// The tile needs to be registered so the TileModel.create -// knows it is a supported tile type -import "./bar-graph-registration"; - -jest.mock("react-resize-detector", () => ({ - useResizeDetector: jest.fn(() => ({height: 200, width: 200, ref: null})) -})); - -jest.mock("./bar-graph-utils", () => ({ - getBBox: jest.fn(() => ({x: 0, y: 0, width: 500, height: 200})) -})); - - -describe("BarGraphComponent", () => { - const content = defaultBarGraphContent(); - const model = TileModel.create({content}); - - const defaultProps = { - tileElt: null, - context: "", - docId: "", - documentContent: null, - isUserResizable: true, - onResizeRow: (e: React.DragEvent): void => { - throw new Error("Function not implemented."); - }, - onSetCanAcceptDrop: (tileId?: string): void => { - throw new Error("Function not implemented."); - }, - onRequestRowHeight: (tileId: string, height?: number, deltaHeight?: number): void => { - throw new Error("Function not implemented."); - }, - onRegisterTileApi: (tileApi: ITileApi, facet?: string): void => { - throw new Error("Function not implemented."); - }, - onUnregisterTileApi: (facet?: string): void => { - throw new Error("Function not implemented."); - } - }; - - it("renders successfully", () => { - const {getByText, getByTestId} = - render(); - expect(getByText("Tile Title")).toBeInTheDocument(); - expect(getByTestId("bar-graph-content")).toBeInTheDocument(); - expect(globalGetByText(getByTestId("bar-graph-content"), "Counts")).toBeInTheDocument(); - expect(getByText("6/23/24")).toBeInTheDocument(); - }); - - it.skip("updates the text when the model changes", async () => { - const {getByTestId, findByText} = - render(); - expect(globalGetByText(getByTestId("bar-graph-content"), "Counts")).toBeInTheDocument(); - - content.setYAxisLabel("New Text"); - - expect(await findByText( "New Text")).toBeInTheDocument(); - }); - -}); diff --git a/src/plugins/bar-graph/bar-graph-tile.tsx b/src/plugins/bar-graph/bar-graph-tile.tsx index a34f481338..0ab434e069 100644 --- a/src/plugins/bar-graph/bar-graph-tile.tsx +++ b/src/plugins/bar-graph/bar-graph-tile.tsx @@ -9,19 +9,28 @@ import { BarGraphChart } from "./bar-graph-chart"; import { BarGraphModelContext } from "./bar-graph-content-context"; import { isBarGraphModel } from "./bar-graph-content"; import { TileToolbar } from "../../components/toolbar/tile-toolbar"; +import { BarGraphLegend } from "./bar-graph-legend"; import "./bar-graph.scss"; import "./bar-graph-toolbar"; -export const BarGraphComponent: React.FC = observer((props: ITileProps) => { +const legendWidth = 190; +const legendHeight = 190; // FIXME - const {height: resizeHeight, width: resizeWidth, ref} = useResizeDetector(); +export const BarGraphComponent: React.FC = observer((props: ITileProps) => { const { model, readOnly } = props; - const content = isBarGraphModel(model.content) ? model.content : null; + const {height: containerHeight, width: containerWidth, ref} = useResizeDetector(); + let svgWidth = 10, svgHeight = 10; + const legendBelow = containerWidth && containerWidth < 450; + if (containerWidth && containerHeight) { + // Legend is on the right if the width is >= 450px + svgWidth = legendBelow ? containerWidth : containerWidth-legendWidth; + svgHeight = legendBelow ? containerHeight-legendHeight : containerHeight; + } return ( @@ -29,10 +38,11 @@ export const BarGraphComponent: React.FC = observer((props: ITilePro
- + +
); diff --git a/src/plugins/bar-graph/bar-graph.scss b/src/plugins/bar-graph/bar-graph.scss index 20fc30859e..72202ffb37 100644 --- a/src/plugins/bar-graph/bar-graph.scss +++ b/src/plugins/bar-graph/bar-graph.scss @@ -3,15 +3,19 @@ .bar-graph-content { width: 100%; height: 100%; + padding: 44px 0 12px 0; display: flex; flex-direction: row; - flex-wrap: nowrap; align-items: center; overflow: auto; - svg { - width: 100%; - height: 100%; + &.vertical { + flex-direction: column; + } + + svg.bar-graph-svg { + // width: 100%; + // height: 100%; .visx-bar-group .visx-bar { stroke: black; @@ -36,13 +40,20 @@ .button-content { display: flex; flex-direction: row; + flex-wrap: nowrap; justify-content: center; align-items: center; + .button-text { + white-space: nowrap; + } + svg { + width: 100%; + height: 100%; max-height: 1em; max-width: 1em; - margin-left: 1em; + margin-left: .75em; opacity: .35; } } @@ -52,5 +63,48 @@ } + div.bar-graph-legend { + height: 100%; + width: 186px; + border-left: 1.5px solid #0592af; + padding: 0 0 0 5px; + text-align: left; + + .dataset-header { + display: flex; + align-items: top; + + .dataset-icon { + margin-right: 7px; + + &:hover { + background-color: $workspace-teal-light-6; + } + } + + .dataset-label { + .dataset-label-text { + display: block; + } + .dataset-name { + display: block; + font-weight: bold; + } + } + } + + .sort-by { + margin: 10px 0 0 5px; + } + } + + &.vertical div.bar-graph-legend { + width: 100%; + height: 186px; + border-top: 1.5px solid #0592af; + border-left: none; + padding: 8px 0 0 0; + } + } diff --git a/src/plugins/graph/components/legend/layer-legend.tsx b/src/plugins/graph/components/legend/layer-legend.tsx index 257692d54a..1f09b5ea3f 100644 --- a/src/plugins/graph/components/legend/layer-legend.tsx +++ b/src/plugins/graph/components/legend/layer-legend.tsx @@ -18,7 +18,7 @@ import { useTileModelContext } from "../../../../components/tiles/hooks/use-tile import { EditableLabelWithButton } from "../../../../components/utilities/editable-label-with-button"; import { GraphLayerContext, useGraphLayerContext } from "../../hooks/use-graph-layer-context"; -import RemoveDataIcon from "../../assets/remove-data-icon.svg"; +import RemoveDataIcon from "../../../../assets/remove-data-icon.svg"; import XAxisIcon from "../../assets/x-axis-icon.svg"; import YAxisIcon from "../../assets/y-axis-icon.svg"; diff --git a/src/plugins/shared-variables/graph/legend/variable-function-legend.tsx b/src/plugins/shared-variables/graph/legend/variable-function-legend.tsx index 375c3655bb..0137f5ab8e 100644 --- a/src/plugins/shared-variables/graph/legend/variable-function-legend.tsx +++ b/src/plugins/shared-variables/graph/legend/variable-function-legend.tsx @@ -16,8 +16,8 @@ import { } from "../plotted-variables-adornment/plotted-variables-adornment-model"; import { VariableSelection } from "./variable-selection"; +import RemoveDataIcon from "../../../../assets/remove-data-icon.svg"; import AddSeriesIcon from "../../../graph/imports/assets/add-series-icon.svg"; -import RemoveDataIcon from "../../../graph/assets/remove-data-icon.svg"; import XAxisIcon from "../../../graph/assets/x-axis-icon.svg"; import YAxisIcon from "../../../graph/assets/y-axis-icon.svg"; From 1469f42ae476098252000b2587b99967354383fe Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 6 Sep 2024 16:41:43 -0400 Subject: [PATCH 04/36] Sort by menu --- src/plugins/bar-graph/bar-graph-chart.tsx | 64 +++++---------- src/plugins/bar-graph/bar-graph-content.ts | 54 ++++++++++++- src/plugins/bar-graph/bar-graph-legend.tsx | 45 ++++++++++- src/plugins/bar-graph/bar-graph.scss | 90 ++++++++++++++------- src/plugins/bar-graph/category-pulldown.tsx | 6 +- 5 files changed, 178 insertions(+), 81 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-chart.tsx b/src/plugins/bar-graph/bar-graph-chart.tsx index f8851f702c..a8f17b0f3f 100644 --- a/src/plugins/bar-graph/bar-graph-chart.tsx +++ b/src/plugins/bar-graph/bar-graph-chart.tsx @@ -1,12 +1,10 @@ import React, { useMemo } from "react"; import { observer } from "mobx-react"; -import { isNumber } from "lodash"; import { AxisBottom, AxisLeft } from "@visx/axis"; import { GridRows } from "@visx/grid"; import { Group } from "@visx/group"; import { scaleBand, scaleLinear } from "@visx/scale"; import { Bar, BarGroup } from "@visx/shape"; -import { clueDataColorInfo } from "../../utilities/color-utils"; import { useBarGraphModelContext } from "./bar-graph-content-context"; import { CategoryPulldown } from "./category-pulldown"; import EditableAxisLabel from "./editable-axis-label"; @@ -18,12 +16,9 @@ const margin = { right: 10, }; +// Round a number up to the next multiple of 5. function roundTo5(n: number): number { - return Math.ceil(n/5)*5; -} - -function barColor(n: number) { - return clueDataColorInfo[n % clueDataColorInfo.length].color; + return Math.max(5, Math.ceil(n/5)*5); } interface IBarGraphChartProps { @@ -40,51 +35,28 @@ export const BarGraphChart = observer(function BarGraphChart({ width, height }: const model = useBarGraphModelContext(); const primary = model?.primaryAttribute || "date"; - const secondary = model?.secondaryAttribute || "location"; // FIXME const xMax = width - margin.left - margin.right; const yMax = height - margin.top - margin.bottom; - function setDemoCategory(catname: string) { - model?.setPrimaryAttribute(catname); - model?.setSecondaryAttribute(""); + function setPrimaryAttribute(id: string) { + model?.setPrimaryAttribute(id); + model?.setSecondaryAttribute(undefined); + } + + function barColor(key: string) { + if (!model) return "black"; + return model.getColorForSecondaryKey(key); } // Count cases and make the data array - const data = useMemo( - () => { - const dataSet = model?.dataSet; - if (!dataSet || !primary || !model.cases) return []; - return model.cases.reduce((acc, caseID) => { - const cat = dataSet?.dataSet.getStrValue(caseID.__id__, primary); - const subCat = "A"; - const index = acc.findIndex(r => r[primary] === cat); - if (index >= 0) { - const cur = acc[index][subCat]; - acc[index][subCat] = (isNumber(cur) ? cur : 0) + 1; - } else { - const newRow = { [primary]: cat, [subCat]: 1 }; - acc.push(newRow); - } - return acc; - }, [] as { [key: string]: number|string }[]); - }, - [model?.cases, model?.dataSet, primary]); - console.log(data); - - const primaryKeys: string[] - = useMemo(() => data.map(d => d[primary] as string), - [data, primary]); - const secondaryKeys: string[] - = useMemo(() => Array.from(new Set(data.flatMap(d => Object.keys(d)).filter(k => k !== primary))), - [data, primary]); + const data = model?.dataArray || []; + + const primaryKeys = useMemo(() => model?.primaryKeys || [], [model?.primaryKeys]); + const secondaryKeys = useMemo(() => model?.secondaryKeys || [], [model?.secondaryKeys]); // find the maximum data value - const maxValue = data.reduce((acc, row) => { - const rowValues = Object.values(row).slice(1) as (string | number)[]; - const maxInRow = Math.max(...rowValues.map(v => isNumber(v) ? v : 0)); - return Math.max(maxInRow, acc); - }, 0); + const maxValue = model?.maxDataValue || 0; const primaryScale = useMemo( () => @@ -110,7 +82,7 @@ export const BarGraphChart = observer(function BarGraphChart({ width, height }: }), [yMax, maxValue]); - if (xMax <= 0 || yMax <= 0) return Too small ({width}x{height}); + if (xMax <= 0 || yMax <= 0) return Tile too small to show graph ({width}x{height}); const ticks = Math.min(4, Math.floor(yMax/40)); // leave generous vertical space (>=40 px) between ticks const labelWidth = (xMax/primaryKeys.length)-10; // setting width will wrap lines in labels when needed @@ -146,7 +118,7 @@ export const BarGraphChart = observer(function BarGraphChart({ width, height }: /> barColor(i)} + color={barColor} className="bar" keys={secondaryKeys} height={yMax} @@ -183,7 +155,7 @@ export const BarGraphChart = observer(function BarGraphChart({ width, height }: setText={(text) => model?.setYAxisLabel(text)} /> ({ + // TODO what should this do in the case of no secondary attribute? + get dataArray() { + console.log("calculating dataArray"); + const dataSet = self.dataSet?.dataSet; + const primary = self.primaryAttribute; + const secondary = self.secondaryAttribute; + const cases = self.cases; + if (!dataSet || !primary || !cases) return []; + return cases.reduce((acc, caseID) => { + const cat = dataSet.getStrValue(caseID.__id__, primary); + const subCat = secondary ? dataSet.getStrValue(caseID.__id__, secondary) : "default"; // ?? + const index = acc.findIndex(r => r[primary] === cat); + if (index >= 0) { + const cur = acc[index][subCat]; + acc[index][subCat] = (isNumber(cur) ? cur : 0) + 1; + } else { + const newRow = { [primary]: cat, [subCat]: 1 }; + acc.push(newRow); + } + return acc; + }, [] as { [key: string]: number|string }[]); + } + })) + .views(self => ({ + get primaryKeys() { + const primary = self.primaryAttribute; + if (!primary) return []; + return self.dataArray.map(d => d[primary] as string); + }, + get secondaryKeys() { + const primary = self.primaryAttribute; + if (!primary) return []; + return Array.from(new Set(self.dataArray.flatMap(d => Object.keys(d)).filter(k => k !== primary))); + }, + get maxDataValue(): number { + return self.dataArray.reduce((acc, row) => { + const rowValues = Object.values(row).filter(v => isNumber(v)) as number[]; + const maxInRow = Math.max(...rowValues); + return Math.max(maxInRow, acc); + }, 0); + } + })) + .views(self => ({ + // TODO this should track colors in a way that can be edited later + getColorForSecondaryKey(key: string) { + const n = self.secondaryKeys.indexOf(key) || 0; + return clueDataColorInfo[n % clueDataColorInfo.length].color; + } + })) .actions(self => ({ setYAxisLabel(text: string) { self.yAxisLabel = text; @@ -48,7 +100,7 @@ export const BarGraphContentModel = TileContentModel setPrimaryAttribute(attrId: string) { self.primaryAttribute = attrId; }, - setSecondaryAttribute(attrId: string) { + setSecondaryAttribute(attrId: string|undefined) { self.secondaryAttribute = attrId; }, setSharedDataSet() { diff --git a/src/plugins/bar-graph/bar-graph-legend.tsx b/src/plugins/bar-graph/bar-graph-legend.tsx index 5324a5d487..9194c4369b 100644 --- a/src/plugins/bar-graph/bar-graph-legend.tsx +++ b/src/plugins/bar-graph/bar-graph-legend.tsx @@ -1,8 +1,11 @@ import React from 'react'; import { observer } from 'mobx-react'; +import { Menu, MenuButton, MenuItem, MenuList } from '@chakra-ui/react'; import { useBarGraphModelContext } from './bar-graph-content-context'; import RemoveDataIcon from "../../assets/remove-data-icon.svg"; +import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; + export const BarGraphLegend = observer(function BarGraphLegend () { const model = useBarGraphModelContext(); @@ -13,11 +16,24 @@ export const BarGraphLegend = observer(function BarGraphLegend () { } } + function setSecondaryAttribute(attributeId: string|undefined) { + if (model) { + model.setSecondaryAttribute(attributeId); + } + } if (!model || !model.dataSet) { return null; } + const dataSet = model.dataSet.dataSet; + const allAttributes = dataSet?.attributes || []; + const availableAttributes = allAttributes.filter((a) => a.id !== model.primaryAttribute); + const currentSecondary = model.secondaryAttribute ? dataSet?.attrFromID(model.secondaryAttribute) : undefined; + const currentLabel = currentSecondary?.name || "None"; + + const secondaryKeys = model.secondaryKeys; + return (
@@ -33,7 +49,34 @@ export const BarGraphLegend = observer(function BarGraphLegend () {
- Sort by: +
+ Sort by: +
+ + + + {currentLabel} + + + + + setSecondaryAttribute(undefined)}>None + {availableAttributes.map((a) => ( + setSecondaryAttribute(a.id)}>{a.name} + ))} + + +
+ +
+ {secondaryKeys.map((key) => ( +
+
+
+
+
{key}
+
+ ))}
); diff --git a/src/plugins/bar-graph/bar-graph.scss b/src/plugins/bar-graph/bar-graph.scss index 72202ffb37..40c5d0744a 100644 --- a/src/plugins/bar-graph/bar-graph.scss +++ b/src/plugins/bar-graph/bar-graph.scss @@ -31,34 +31,6 @@ padding: 3px; } - button { - font-family: Lato; - border: 1.5px solid #949494; - border-radius: 5px; - padding: 5px; - - .button-content { - display: flex; - flex-direction: row; - flex-wrap: nowrap; - justify-content: center; - align-items: center; - - .button-text { - white-space: nowrap; - } - - svg { - width: 100%; - height: 100%; - max-height: 1em; - max-width: 1em; - margin-left: .75em; - opacity: .35; - } - } - - } } } @@ -67,7 +39,7 @@ height: 100%; width: 186px; border-left: 1.5px solid #0592af; - padding: 0 0 0 5px; + padding: 0 10px 0 5px; text-align: left; .dataset-header { @@ -94,7 +66,36 @@ } .sort-by { - margin: 10px 0 0 5px; + margin: 10px 0 10px 5px; + } + + button.chakra-menu__menu-button { + background-color: $workspace-teal-light-8; + width: 100%; + margin-top: 5px; + } + + .secondaryValue { + display: flex; + margin: 5px 0 5px 5px; + align-items: center; + + .colorButton { + width: 26px; + height: 26px; + margin: 0 7px 0 0; + padding: 5px; + background-color: $workspace-teal-light-8; + border-radius: 5px; + border: solid 1.5px #949494; + + .colorSwatch { + border: 1px solid black; + width: 100%; + height: 100%; + margin: 0; + } + } } } @@ -106,5 +107,34 @@ padding: 8px 0 0 0; } + button.chakra-menu__menu-button { + font-family: Lato; + border: 1.5px solid #949494; + border-radius: 5px; + padding: 5px; + + .button-content { + display: flex; + flex-direction: row; + flex-wrap: nowrap; + justify-content: center; + align-items: center; + + .button-text { + white-space: nowrap; + } + + svg { + width: 100%; + height: 100%; + max-height: 1em; + max-width: 1em; + margin-left: .75em; + opacity: .35; + } + } + + } + } diff --git a/src/plugins/bar-graph/category-pulldown.tsx b/src/plugins/bar-graph/category-pulldown.tsx index 585401f6e2..fe5d52339c 100644 --- a/src/plugins/bar-graph/category-pulldown.tsx +++ b/src/plugins/bar-graph/category-pulldown.tsx @@ -1,9 +1,9 @@ import React from "react"; import { Menu, MenuButton, MenuItem, MenuList, Portal } from "@chakra-ui/react"; import { useReadOnlyContext } from "../../components/document/read-only-context"; +import { useBarGraphModelContext } from "./bar-graph-content-context"; import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; -import { useBarGraphModelContext } from "./bar-graph-content-context"; interface IProps { @@ -22,12 +22,12 @@ export function CategoryPulldown({setCategory, x, y, width, height}: IProps) { const attributes = dataSet?.attributes || []; const current = (dataSet && model.primaryAttribute) ? dataSet.attrFromID(model.primaryAttribute)?.name - : "Choose one"; + : "Categories"; return ( - + {current} From e566501863e33eb8d4c767738f009b62796284a7 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 9 Sep 2024 09:08:03 -0400 Subject: [PATCH 05/36] Reorganize, legend fixes --- src/plugins/bar-graph/bar-graph-content.ts | 7 ++++-- src/plugins/bar-graph/bar-graph-tile.tsx | 8 +++---- src/plugins/bar-graph/bar-graph.scss | 1 + src/plugins/bar-graph/category-pulldown.tsx | 8 +++++-- .../{bar-graph-chart.tsx => chart-area.tsx} | 12 +++++----- src/plugins/bar-graph/editable-axis-label.tsx | 4 ++-- .../{bar-graph-legend.tsx => legend-area.tsx} | 19 ++++++++-------- .../bar-graph/legend-secondary-row.tsx | 22 +++++++++++++++++++ 8 files changed, 56 insertions(+), 25 deletions(-) rename src/plugins/bar-graph/{bar-graph-chart.tsx => chart-area.tsx} (93%) rename src/plugins/bar-graph/{bar-graph-legend.tsx => legend-area.tsx} (82%) create mode 100644 src/plugins/bar-graph/legend-secondary-row.tsx diff --git a/src/plugins/bar-graph/bar-graph-content.ts b/src/plugins/bar-graph/bar-graph-content.ts index 8834cfb44f..4197aaa4f5 100644 --- a/src/plugins/bar-graph/bar-graph-content.ts +++ b/src/plugins/bar-graph/bar-graph-content.ts @@ -89,7 +89,8 @@ export const BarGraphContentModel = TileContentModel .views(self => ({ // TODO this should track colors in a way that can be edited later getColorForSecondaryKey(key: string) { - const n = self.secondaryKeys.indexOf(key) || 0; + let n = self.secondaryKeys.indexOf(key); + if (!n || n<0) n=0; return clueDataColorInfo[n % clueDataColorInfo.length].color; } })) @@ -97,7 +98,7 @@ export const BarGraphContentModel = TileContentModel setYAxisLabel(text: string) { self.yAxisLabel = text; }, - setPrimaryAttribute(attrId: string) { + setPrimaryAttribute(attrId: string|undefined) { self.primaryAttribute = attrId; }, setSecondaryAttribute(attrId: string|undefined) { @@ -133,6 +134,8 @@ export const BarGraphContentModel = TileContentModel // When new dataset is attached, cache a reference to it and pick a category attribute. const dataSet = self.sharedModelDataSet(getSharedModelManager(self)); if (self.dataSet !== dataSet) { + self.setPrimaryAttribute(undefined); + self.setSecondaryAttribute(undefined); self.dataSet = dataSet; if (dataSet) { const atts = dataSet.dataSet.attributes; diff --git a/src/plugins/bar-graph/bar-graph-tile.tsx b/src/plugins/bar-graph/bar-graph-tile.tsx index 0ab434e069..ac095ac6bd 100644 --- a/src/plugins/bar-graph/bar-graph-tile.tsx +++ b/src/plugins/bar-graph/bar-graph-tile.tsx @@ -5,11 +5,11 @@ import { useResizeDetector } from "react-resize-detector"; import { BasicEditableTileTitle } from "../../components/tiles/basic-editable-tile-title"; import { ITileProps } from "../../components/tiles/tile-component"; -import { BarGraphChart } from "./bar-graph-chart"; +import { ChartArea } from "./chart-area"; import { BarGraphModelContext } from "./bar-graph-content-context"; import { isBarGraphModel } from "./bar-graph-content"; import { TileToolbar } from "../../components/toolbar/tile-toolbar"; -import { BarGraphLegend } from "./bar-graph-legend"; +import { LegendArea } from "./legend-area"; import "./bar-graph.scss"; @@ -41,8 +41,8 @@ export const BarGraphComponent: React.FC = observer((props: ITilePro className={classNames("bar-graph-content", legendBelow ? "vertical" : "horizontal", { "read-only": readOnly })} data-testid="bar-graph-content" > - - + +
); diff --git a/src/plugins/bar-graph/bar-graph.scss b/src/plugins/bar-graph/bar-graph.scss index 40c5d0744a..a5d258ce04 100644 --- a/src/plugins/bar-graph/bar-graph.scss +++ b/src/plugins/bar-graph/bar-graph.scss @@ -40,6 +40,7 @@ width: 186px; border-left: 1.5px solid #0592af; padding: 0 10px 0 5px; + overflow: auto; text-align: left; .dataset-header { diff --git a/src/plugins/bar-graph/category-pulldown.tsx b/src/plugins/bar-graph/category-pulldown.tsx index fe5d52339c..d69f87305e 100644 --- a/src/plugins/bar-graph/category-pulldown.tsx +++ b/src/plugins/bar-graph/category-pulldown.tsx @@ -1,4 +1,5 @@ import React from "react"; +import { observer } from "mobx-react"; import { Menu, MenuButton, MenuItem, MenuList, Portal } from "@chakra-ui/react"; import { useReadOnlyContext } from "../../components/document/read-only-context"; import { useBarGraphModelContext } from "./bar-graph-content-context"; @@ -14,7 +15,7 @@ interface IProps { height: number; } -export function CategoryPulldown({setCategory, x, y, width, height}: IProps) { +export const CategoryPulldown = observer(function CategoryPulldown({setCategory, x, y, width, height}: IProps) { const readOnly = useReadOnlyContext(); const model = useBarGraphModelContext(); @@ -43,4 +44,7 @@ export function CategoryPulldown({setCategory, x, y, width, height}: IProps) {
); -} +}); + +export default CategoryPulldown; + diff --git a/src/plugins/bar-graph/bar-graph-chart.tsx b/src/plugins/bar-graph/chart-area.tsx similarity index 93% rename from src/plugins/bar-graph/bar-graph-chart.tsx rename to src/plugins/bar-graph/chart-area.tsx index a8f17b0f3f..1fd893d1e3 100644 --- a/src/plugins/bar-graph/bar-graph-chart.tsx +++ b/src/plugins/bar-graph/chart-area.tsx @@ -21,20 +21,20 @@ function roundTo5(n: number): number { return Math.max(5, Math.ceil(n/5)*5); } -interface IBarGraphChartProps { +interface IProps { width: number; height: number; } -// TODO rotate labels if needed +// Consider: rotating labels if needed // angle: -45, textAnchor: 'end' // https://github.com/airbnb/visx/discussions/1494 -export const BarGraphChart = observer(function BarGraphChart({ width, height }: IBarGraphChartProps) { +export const ChartArea = observer(function BarGraphChart({ width, height }: IProps) { const model = useBarGraphModelContext(); - const primary = model?.primaryAttribute || "date"; + const primary = model?.primaryAttribute || ""; const xMax = width - margin.left - margin.right; const yMax = height - margin.top - margin.bottom; @@ -84,7 +84,9 @@ export const BarGraphChart = observer(function BarGraphChart({ width, height }: if (xMax <= 0 || yMax <= 0) return Tile too small to show graph ({width}x{height}); - const ticks = Math.min(4, Math.floor(yMax/40)); // leave generous vertical space (>=40 px) between ticks + const ticks = data.length > 0 + ? Math.min(4, Math.floor(yMax/40)) // leave generous vertical space (>=40 px) between ticks + : 0; const labelWidth = (xMax/primaryKeys.length)-10; // setting width will wrap lines in labels when needed return ( diff --git a/src/plugins/bar-graph/editable-axis-label.tsx b/src/plugins/bar-graph/editable-axis-label.tsx index 5b625079d1..f1d1b428ac 100644 --- a/src/plugins/bar-graph/editable-axis-label.tsx +++ b/src/plugins/bar-graph/editable-axis-label.tsx @@ -5,14 +5,14 @@ import { useReadOnlyContext } from '../../components/document/read-only-context' const paddingX = 5, paddingY = 10; -interface Props { +interface IProps { x: number; y: number; text?: string; setText: (text: string) => void; } -const EditableAxisLabel: React.FC = ({text, x, y, setText}) => { +const EditableAxisLabel: React.FC = ({text, x, y, setText}) => { const readOnly = useReadOnlyContext(); const textRef = React.useRef(null); diff --git a/src/plugins/bar-graph/bar-graph-legend.tsx b/src/plugins/bar-graph/legend-area.tsx similarity index 82% rename from src/plugins/bar-graph/bar-graph-legend.tsx rename to src/plugins/bar-graph/legend-area.tsx index 9194c4369b..47ce7bd793 100644 --- a/src/plugins/bar-graph/bar-graph-legend.tsx +++ b/src/plugins/bar-graph/legend-area.tsx @@ -2,12 +2,13 @@ import React from 'react'; import { observer } from 'mobx-react'; import { Menu, MenuButton, MenuItem, MenuList } from '@chakra-ui/react'; import { useBarGraphModelContext } from './bar-graph-content-context'; +import { LegendSecondaryRow } from './legend-secondary-row'; import RemoveDataIcon from "../../assets/remove-data-icon.svg"; import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; -export const BarGraphLegend = observer(function BarGraphLegend () { +export const LegendArea = observer(function LegendArea () { const model = useBarGraphModelContext(); function unlinkDataset() { @@ -22,13 +23,14 @@ export const BarGraphLegend = observer(function BarGraphLegend () { } } - if (!model || !model.dataSet) { + if (!model || !model.dataSet || !model.primaryAttribute) { return null; } const dataSet = model.dataSet.dataSet; const allAttributes = dataSet?.attributes || []; const availableAttributes = allAttributes.filter((a) => a.id !== model.primaryAttribute); + const currentPrimary = dataSet?.attrFromID(model.primaryAttribute); const currentSecondary = model.secondaryAttribute ? dataSet?.attrFromID(model.secondaryAttribute) : undefined; const currentLabel = currentSecondary?.name || "None"; @@ -69,15 +71,12 @@ export const BarGraphLegend = observer(function BarGraphLegend () {
- {secondaryKeys.map((key) => ( -
-
-
-
-
{key}
-
- ))} + {currentSecondary + ? secondaryKeys.map((key) => ) + : }
); }); + +export default LegendArea; diff --git a/src/plugins/bar-graph/legend-secondary-row.tsx b/src/plugins/bar-graph/legend-secondary-row.tsx new file mode 100644 index 0000000000..6da35aba16 --- /dev/null +++ b/src/plugins/bar-graph/legend-secondary-row.tsx @@ -0,0 +1,22 @@ +import React from 'react'; +import { useBarGraphModelContext } from './bar-graph-content-context'; + +interface IProps { + attrValue: string; +} + +export function LegendSecondaryRow({attrValue}: IProps) { + const model = useBarGraphModelContext(); + if (!model) return null; + + return ( +
+
+
+
+
{attrValue}
+
+ ); +} + +export default LegendSecondaryRow; From 3a3f78943915ac5b3ee2c2b761bd30ded00230f8 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 9 Sep 2024 11:28:19 -0400 Subject: [PATCH 06/36] Design fidelity; tests --- .../bar-graph/bar-graph-content.test.ts | 106 ++++++++++++++++- src/plugins/bar-graph/bar-graph-content.ts | 64 +++++++--- src/plugins/bar-graph/bar-graph-utils.ts | 5 + src/plugins/bar-graph/category-pulldown.tsx | 1 - src/plugins/bar-graph/chart-area.tsx | 110 +++++++++++------- 5 files changed, 228 insertions(+), 58 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-content.test.ts b/src/plugins/bar-graph/bar-graph-content.test.ts index 1b5a7029c4..9299df3cb8 100644 --- a/src/plugins/bar-graph/bar-graph-content.test.ts +++ b/src/plugins/bar-graph/bar-graph-content.test.ts @@ -1,14 +1,51 @@ +import { getSnapshot } from "mobx-state-tree"; +import { addAttributeToDataSet, addCasesToDataSet, DataSet } from "../../models/data/data-set"; +import { ICaseCreation } from "../../models/data/data-set-types"; +import { SharedDataSet, SharedDataSetType } from "../../models/shared/shared-data-set"; import { defaultBarGraphContent, BarGraphContentModel } from "./bar-graph-content"; +const mockCases: ICaseCreation[] = [ + { species: "cat", location: "yard" }, + { species: "cat", location: "yard" }, + { species: "owl", location: "yard" }, + { species: "owl", location: "forest" } +]; + +const emptyDataSet = DataSet.create(); +addAttributeToDataSet(emptyDataSet, { id: "att-s", name: "species" }); +addAttributeToDataSet(emptyDataSet, { id: "att-l", name: "location" }); +const sharedEmptyDataSet = SharedDataSet.create({ dataSet: emptyDataSet }); + +const sampleDataSet = DataSet.create(); +addAttributeToDataSet(sampleDataSet, { id: "att-s", name: "species" }); +addAttributeToDataSet(sampleDataSet, { id: "att-l", name: "location" }); +addCasesToDataSet(sampleDataSet, mockCases); +const sharedSampleDataSet = SharedDataSet.create({ dataSet: sampleDataSet }); + +const TestableBarGraphContentModel = BarGraphContentModel + .actions(self => ({ + setDataSet(ds: SharedDataSetType) { + self.dataSet = ds; + } + })); + describe("Bar Graph Content", () => { it("is a Bar Graph model", () => { const content = BarGraphContentModel.create(); expect(content.type).toBe("BarGraph"); }); - it("yAxisLabel has default content of 'Counts'", () => { + it("yAxisLabel has expected default content", () => { const content = defaultBarGraphContent(); expect(content.yAxisLabel).toBe("Counts"); + expect(getSnapshot(content)).toMatchInlineSnapshot(` +Object { + "primaryAttribute": undefined, + "secondaryAttribute": undefined, + "type": "BarGraph", + "yAxisLabel": "Counts", +} +`); }); it("is always user resizable", () => { @@ -36,4 +73,71 @@ describe("Bar Graph Content", () => { expect(content.secondaryAttribute).toBe("attrId"); }); + it("returns empty data array when there are no cases", () => { + const content = TestableBarGraphContentModel.create({ }); + content.setDataSet(sharedEmptyDataSet); + expect(content.dataArray).toEqual([]); + }); + + it("returns empty data array when there is no primary attribute", () => { + const content = TestableBarGraphContentModel.create({ }); + content.setDataSet(sharedSampleDataSet); + expect(content.dataArray).toEqual([]); + }); + + it("returns expected data array with primary attribute", () => { + const content = TestableBarGraphContentModel.create({ }); + content.setDataSet(sharedSampleDataSet); + content.setPrimaryAttribute("att-s"); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "value": 2 }, + { "att-s": "owl","value": 2} + ]); + + content.setPrimaryAttribute("att-l"); + expect(content.dataArray).toEqual([ + { "att-l": "yard", "value": 3 }, + { "att-l": "forest", "value": 1 } + ]); + }); + + it("returns expected data array with primary and secondary attributes", () => { + const content = TestableBarGraphContentModel.create({ }); + content.setDataSet(sharedSampleDataSet); + content.setPrimaryAttribute("att-s"); + content.setSecondaryAttribute("att-l"); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "yard": 2 }, + { "att-s": "owl", "yard": 1, "forest": 1 } + ]); + }); + + it("extracts primary keys", () => { + const content = TestableBarGraphContentModel.create({ }); + content.setDataSet(sharedSampleDataSet); + content.setPrimaryAttribute("att-s"); + expect(content.primaryKeys).toEqual(["cat", "owl"]); + }); + + it("extracts secondary keys", () => { + const content = TestableBarGraphContentModel.create({ }); + content.setDataSet(sharedSampleDataSet); + content.setPrimaryAttribute("att-s"); + content.setSecondaryAttribute("att-l"); + expect(content.secondaryKeys).toEqual(["yard", "forest"]); + }); + + it("calculates the maximum data value", () => { + const content = TestableBarGraphContentModel.create({ }); + content.setDataSet(sharedSampleDataSet); + content.setPrimaryAttribute("att-s"); + expect(content.maxDataValue).toBe(2); + + content.setPrimaryAttribute("att-l"); + expect(content.maxDataValue).toBe(3); + + content.setSecondaryAttribute("att-s"); + expect(content.maxDataValue).toBe(2); + }); + }); diff --git a/src/plugins/bar-graph/bar-graph-content.ts b/src/plugins/bar-graph/bar-graph-content.ts index 4197aaa4f5..447fc242fb 100644 --- a/src/plugins/bar-graph/bar-graph-content.ts +++ b/src/plugins/bar-graph/bar-graph-content.ts @@ -44,27 +44,61 @@ export const BarGraphContentModel = TileContentModel } })) .views(self => ({ - // TODO what should this do in the case of no secondary attribute? + /** + * Returns the dataset data in a format suitable for plotting. + * + * With a primary attribute "species" and no secondary attribute, this will be something like: + * ```json + * [ + * { species: "cat", value: 7 }, + * { species: "owl", value: 3 } + * ] + * ``` + * + * If there is a secondary attribute "location", this will be like: + * ```json + * [ + * { species: "cat", backyard: 5, street: 2, forest: 0 }, + * { species: "owl", backyard: 1, street: 0, forest: 2 } + * ] + * ``` + */ get dataArray() { - console.log("calculating dataArray"); const dataSet = self.dataSet?.dataSet; const primary = self.primaryAttribute; const secondary = self.secondaryAttribute; const cases = self.cases; if (!dataSet || !primary || !cases) return []; - return cases.reduce((acc, caseID) => { - const cat = dataSet.getStrValue(caseID.__id__, primary); - const subCat = secondary ? dataSet.getStrValue(caseID.__id__, secondary) : "default"; // ?? - const index = acc.findIndex(r => r[primary] === cat); - if (index >= 0) { - const cur = acc[index][subCat]; - acc[index][subCat] = (isNumber(cur) ? cur : 0) + 1; - } else { - const newRow = { [primary]: cat, [subCat]: 1 }; - acc.push(newRow); - } - return acc; - }, [] as { [key: string]: number|string }[]); + if (secondary) { + // Two-dimensionsal data + return cases.reduce((acc, caseID) => { + const cat = dataSet.getStrValue(caseID.__id__, primary); + const subCat = dataSet.getStrValue(caseID.__id__, secondary); + const index = acc.findIndex(r => r[primary] === cat); + if (index >= 0) { + const cur = acc[index][subCat]; + acc[index][subCat] = (isNumber(cur) ? cur : 0) + 1; + } else { + const newRow = { [primary]: cat, [subCat]: 1 }; + acc.push(newRow); + } + return acc; + }, [] as { [key: string]: number | string }[]); + } else { + // One-dimensional data + return cases.reduce((acc, caseID) => { + const cat = dataSet.getStrValue(caseID.__id__, primary); + const index = acc.findIndex(r => r[primary] === cat); + if (index >= 0) { + const cur = acc[index].value; + acc[index].value = isNumber(cur) ? cur + 1 : 1; + } else { + const newRow = { [primary]: cat, value: 1 }; + acc.push(newRow); + } + return acc; + }, [] as { [key: string]: number | string }[]); + } } })) .views(self => ({ diff --git a/src/plugins/bar-graph/bar-graph-utils.ts b/src/plugins/bar-graph/bar-graph-utils.ts index 991175e7b3..65c920e04e 100644 --- a/src/plugins/bar-graph/bar-graph-utils.ts +++ b/src/plugins/bar-graph/bar-graph-utils.ts @@ -3,3 +3,8 @@ export function getBBox(element: SVGGraphicsElement): DOMRect { return element.getBBox(); } + +// Round a number up to the next multiple of 5. +export function roundTo5(n: number): number { + return Math.max(5, Math.ceil(n/5)*5); +} diff --git a/src/plugins/bar-graph/category-pulldown.tsx b/src/plugins/bar-graph/category-pulldown.tsx index d69f87305e..df6c2548a9 100644 --- a/src/plugins/bar-graph/category-pulldown.tsx +++ b/src/plugins/bar-graph/category-pulldown.tsx @@ -6,7 +6,6 @@ import { useBarGraphModelContext } from "./bar-graph-content-context"; import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; - interface IProps { setCategory: (category: string) => void; x: number; diff --git a/src/plugins/bar-graph/chart-area.tsx b/src/plugins/bar-graph/chart-area.tsx index 1fd893d1e3..28d04b28c4 100644 --- a/src/plugins/bar-graph/chart-area.tsx +++ b/src/plugins/bar-graph/chart-area.tsx @@ -8,6 +8,7 @@ import { Bar, BarGroup } from "@visx/shape"; import { useBarGraphModelContext } from "./bar-graph-content-context"; import { CategoryPulldown } from "./category-pulldown"; import EditableAxisLabel from "./editable-axis-label"; +import { roundTo5 } from "./bar-graph-utils"; const margin = { top: 7, @@ -16,11 +17,6 @@ const margin = { right: 10, }; -// Round a number up to the next multiple of 5. -function roundTo5(n: number): number { - return Math.max(5, Math.ceil(n/5)*5); -} - interface IProps { width: number; height: number; @@ -35,6 +31,7 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro const model = useBarGraphModelContext(); const primary = model?.primaryAttribute || ""; + const secondary = model?.secondaryAttribute; const xMax = width - margin.left - margin.right; const yMax = height - margin.top - margin.bottom; @@ -62,15 +59,16 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro () => scaleBand({ domain: primaryKeys, - padding: 0.2, + paddingInner: (secondary ? 0.2 : .66), + paddingOuter: (secondary ? 0.2 : .33), range: [0, xMax]}), - [xMax, primaryKeys]); + [secondary, xMax, primaryKeys]); const secondaryScale = useMemo( () => scaleBand({ domain: secondaryKeys, - padding: 0.2, + padding: 0.4, range: [0, primaryScale.bandwidth()]}), [primaryScale, secondaryKeys]); @@ -86,8 +84,68 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro const ticks = data.length > 0 ? Math.min(4, Math.floor(yMax/40)) // leave generous vertical space (>=40 px) between ticks - : 0; - const labelWidth = (xMax/primaryKeys.length)-10; // setting width will wrap lines in labels when needed + : 0; // no ticks or grid for empty graph + + const labelWidth = (xMax/primaryKeys.length)-10; // setting width will wrap lines when needed + + function simpleBars() { + const color = barColor(primary); + return ( + + {data.map((d) => { + const key = d[primary] as string; + const val = d.value as number; + return ( + + ); + })} + + ); + } + + function groupedBars() { + return ( + d[primary] as string} + x0Scale={primaryScale} + x1Scale={secondaryScale} + yScale={countScale} + > + {(barGroups) => + + {barGroups.map((barGroup) => ( + + {barGroup.bars.map((bar) => { + if (!bar.value) return null; + return ; + })} + + ))} + + } + + ); + } return ( @@ -118,37 +176,7 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro tickLabelProps={{ dx: -5, fontSize: 14, fontFamily: 'Lato', fill: '#3f3f3f' }} tickFormat={(value) => Number(value).toFixed(0)} /> - d[primary] as string} - x0Scale={primaryScale} - x1Scale={secondaryScale} - yScale={countScale} - > - {(barGroups) => - - {barGroups.map((barGroup) => ( - - {barGroup.bars.map((bar) => { - if(!bar.value) return null; - return ; - })} - - ))} - - } - + { secondary ? groupedBars() : simpleBars() } Date: Mon, 9 Sep 2024 16:27:05 -0400 Subject: [PATCH 07/36] Extend Cypress tests, style improvements. --- .../tile_tests/bar_graph_tile_spec.js | 126 +++++++++++++++--- .../tile_tests/xy_plot_tool_spec.js | 30 +---- cypress/support/elements/common/cCanvas.js | 5 + cypress/support/elements/tile/BarGraphTile.js | 48 ++++++- .../support/elements/tile/TableToolTile.js | 20 +++ src/plugins/bar-graph/bar-graph.scss | 6 +- src/plugins/bar-graph/chart-area.tsx | 2 - src/plugins/bar-graph/legend-area.tsx | 18 +-- .../bar-graph/legend-secondary-row.tsx | 8 +- 9 files changed, 198 insertions(+), 65 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index 37348aa578..c3cc8ee312 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -1,24 +1,30 @@ import ClueCanvas from '../../../support/elements/common/cCanvas'; import Canvas from '../../../support/elements/common/Canvas'; import BarGraphTile from '../../../support/elements/tile/BarGraphTile'; - +import TableToolTile + from '../../../support/elements/tile/TableToolTile'; let clueCanvas = new ClueCanvas, - barGraph = new BarGraphTile; + barGraph = new BarGraphTile, + tableTile = new TableToolTile; // eslint-disable-next-line unused-imports/no-unused-vars const canvas = new Canvas; +function textMatchesList(selector, expected) { + selector.should('have.length', expected.length); + selector.each(($el, index) => { + cy.wrap($el).invoke('text').then(text => cy.wrap(text).should('eq', expected[index])); + }); +} + function beforeTest() { - const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); - cy.visit(queryParams); - cy.waitForLoad(); - cy.showOnlyDocumentWorkspace(); + const url = "/editor/?appMode=qa&unit=./demo/units/qa/content.json"; + cy.visit(url); } context('Bar Graph Tile', function () { - it('Can create tile', function () { + it('Basic tile operations, ', function () { beforeTest(); clueCanvas.addTile('bargraph'); @@ -30,27 +36,111 @@ context('Bar Graph Tile', function () { barGraph.getTileTitle().should("be.visible").and('have.text', 'Bar Graph 1'); barGraph.getYAxisLabel().should('have.text', 'Counts'); - barGraph.getXAxisPulldownButton(0).should('have.text', 'date'); - }); + barGraph.getXAxisPulldownButton(0).should('have.text', 'Categories'); - it('Can edit Y axis label', function () { - beforeTest(); - clueCanvas.addTile('bargraph'); - barGraph.getYAxisLabel().should('have.text', 'Counts'); + cy.log('Change Y axis label'); barGraph.getYAxisLabelEditor().should('not.exist'); barGraph.getYAxisLabelButton().click(); barGraph.getYAxisLabelEditor().should('be.visible').type(' of something{enter}'); barGraph.getYAxisLabelEditor().should('not.exist'); barGraph.getYAxisLabel().should('have.text', 'Counts of something'); + + cy.log('Duplicate tile'); + clueCanvas.getDuplicateTool().click(); + barGraph.getTiles().should('have.length', 2); + barGraph.getTile(0) + .should('be.visible') + .and('have.class', 'bar-graph-tile') + .and('not.have.class', 'read-only'); + barGraph.getTileTitle(0).should("be.visible").and('have.text', 'Bar Graph 1'); + barGraph.getYAxisLabel(0).should('have.text', 'Counts of something'); + barGraph.getXAxisPulldownButton(0).should('have.text', 'Categories'); + + barGraph.getTile(1) + .should('be.visible') + .and('have.class', 'bar-graph-tile') + .and('not.have.class', 'read-only'); + barGraph.getTileTitle(1).should("be.visible").and('have.text', 'Bar Graph 2'); + barGraph.getYAxisLabel(1).should('have.text', 'Counts of something'); + barGraph.getXAxisPulldownButton(1).should('have.text', 'Categories'); + + cy.log('Delete tile'); + clueCanvas.deleteTile('bar-graph'); + clueCanvas.deleteTile('bar-graph'); + barGraph.getTiles().should('have.length', 0); }); - it('Can change primary category', function () { + it('Can link data ', function () { beforeTest(); + + // Table dataset for testing: + // 4 instances of X / Y / Z + // 2 instances of XX / Y / Z + // 1 instance of X / YY / Z + clueCanvas.addTile('table'); + tableTile.fillTable(tableTile.getTableTile(), [ + ['X', 'Y', 'Z'], + ['XX', 'Y', 'Z'], + ['X', 'YY', 'Z'], + ['X', 'Y', 'Z'], + ['XX', 'Y', 'Z'], + ['X', 'Y', 'Z'], + ['X', 'Y', 'Z'], + ]); + clueCanvas.addTile('bargraph'); - barGraph.getXAxisPulldown().should('have.text', 'date'); + barGraph.getTiles().click(); + barGraph.getYAxisLabel().should('have.text', 'Counts'); + barGraph.getXAxisPulldown().should('have.text', 'Categories'); + barGraph.getYAxisTickLabel().should('not.exist'); + barGraph.getXAxisTickLabel().should('not.exist'); + barGraph.getLegendArea().should('not.exist'); + barGraph.getBar().should('not.exist'); + + cy.log('Link bar graph'); + clueCanvas.clickToolbarButton('bargraph', 'link-tile'); + cy.get('select').select('Table Data 1'); + cy.get('.modal-button').contains("Graph It!").click(); + + barGraph.getXAxisPulldown().should('have.text', 'x'); + + textMatchesList(barGraph.getXAxisTickLabel(), ['X', 'XX']); + textMatchesList(barGraph.getYAxisTickLabel(), ['0', '1', '2', '3', '4', '5']); + barGraph.getBar().should('have.length', 2); + barGraph.getDatasetLabel().should('have.text', 'Table Data 1'); + barGraph.getSortByMenuButton().should('have.text', 'None'); + barGraph.getSecondaryValueName().should('have.length', 1).and('have.text', 'x'); + + cy.log('Change Sort By'); + barGraph.getSortByMenuButton().click(); + barGraph.getChakraMenuItem().should('have.length', 3); + barGraph.getChakraMenuItem().eq(1).should('have.text', 'y').click(); + textMatchesList(barGraph.getXAxisTickLabel(), ['X', 'XX']); + textMatchesList(barGraph.getYAxisTickLabel(), ['0', '1', '2', '3', '4', '5']); + barGraph.getBar().should('have.length', 3); + barGraph.getDatasetLabel().should('have.text', 'Table Data 1'); + barGraph.getSortByMenuButton().should('have.text', 'y'); + textMatchesList(barGraph.getSecondaryValueName(), ['Y', 'YY']); + + cy.log('Change Category'); barGraph.getXAxisPulldownButton().click(); - barGraph.getXAxisPulldownMenuItem().eq(1).click(); - barGraph.getXAxisPulldown().should('have.text', 'location'); + barGraph.getChakraMenuItem().should('have.length', 3); + barGraph.getChakraMenuItem().eq(1).should('have.text', 'y').click(); + barGraph.getXAxisPulldown().should('have.text', 'y'); + textMatchesList(barGraph.getXAxisTickLabel(), ['Y', 'YY']); + textMatchesList(barGraph.getYAxisTickLabel(), ['0', '2', '4', '6', '8', '10']); // there are 6 Ys in this view so scale expands. + barGraph.getBar().should('have.length', 2); + barGraph.getDatasetLabel().should('have.text', 'Table Data 1'); + barGraph.getSortByMenuButton().should('have.text', 'None'); + barGraph.getSecondaryValueName().should('have.length', 1).and('have.text', 'y'); + + cy.log('Unlink data'); + barGraph.getDatasetUnlinkButton().click(); + barGraph.getXAxisPulldown().should('have.text', 'Categories'); + barGraph.getYAxisTickLabel().should('not.exist'); + barGraph.getXAxisTickLabel().should('not.exist'); + barGraph.getLegendArea().should('not.exist'); + barGraph.getBar().should('not.exist'); }); }); 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 1e8d472bbe..828e7af508 100644 --- a/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js @@ -19,27 +19,6 @@ const queryParamsPlotVariables = `${Cypress.config("qaNoGroupShareUnitStudent5") const problemDoc = '1.1 Unit Toolbar Configuration'; -// Construct and fill in a table tile with the given data (a list of lists) -function buildTable(data) { - // at least two cols, or as many as the longest row in the data array - const cols = Math.max(2, ...data.map(row => row.length)); - clueCanvas.addTile('table'); - tableToolTile.getTableTile().last().should('be.visible'); - tableToolTile.getTableTile().last().within((tile) => { - // tile will start with two columns; make more if desired - for (let i=2; i row.length)); + $tile.within((tile) => { + // tile will start with two columns; make more if desired + for (let i=2; i d[primary] as string} diff --git a/src/plugins/bar-graph/legend-area.tsx b/src/plugins/bar-graph/legend-area.tsx index 47ce7bd793..6feb327200 100644 --- a/src/plugins/bar-graph/legend-area.tsx +++ b/src/plugins/bar-graph/legend-area.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { observer } from 'mobx-react'; -import { Menu, MenuButton, MenuItem, MenuList } from '@chakra-ui/react'; +import { Menu, MenuButton, MenuItem, MenuList, Portal } from '@chakra-ui/react'; import { useBarGraphModelContext } from './bar-graph-content-context'; import { LegendSecondaryRow } from './legend-secondary-row'; @@ -61,16 +61,18 @@ export const LegendArea = observer(function LegendArea () { - - setSecondaryAttribute(undefined)}>None - {availableAttributes.map((a) => ( - setSecondaryAttribute(a.id)}>{a.name} - ))} - + + + setSecondaryAttribute(undefined)}>None + {availableAttributes.map((a) => ( + setSecondaryAttribute(a.id)}>{a.name} + ))} + +
-
+
{currentSecondary ? secondaryKeys.map((key) => ) : } diff --git a/src/plugins/bar-graph/legend-secondary-row.tsx b/src/plugins/bar-graph/legend-secondary-row.tsx index 6da35aba16..e8c1083e64 100644 --- a/src/plugins/bar-graph/legend-secondary-row.tsx +++ b/src/plugins/bar-graph/legend-secondary-row.tsx @@ -10,11 +10,11 @@ export function LegendSecondaryRow({attrValue}: IProps) { if (!model) return null; return ( -
-
-
+
+
+
-
{attrValue}
+
{attrValue}
); } From 2465800ff103da451098976a8d7972c641e82eda Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 9 Sep 2024 16:50:04 -0400 Subject: [PATCH 08/36] Handle missing data --- src/plugins/bar-graph/bar-graph-content.ts | 8 +++++--- src/plugins/bar-graph/bar-graph-utils.ts | 13 ++++++++++++- src/plugins/bar-graph/bar-graph.scss | 4 ++++ src/plugins/bar-graph/legend-secondary-row.tsx | 8 +++++++- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-content.ts b/src/plugins/bar-graph/bar-graph-content.ts index 447fc242fb..51c08ce153 100644 --- a/src/plugins/bar-graph/bar-graph-content.ts +++ b/src/plugins/bar-graph/bar-graph-content.ts @@ -8,6 +8,7 @@ import { getSharedModelManager } from "../../models/tiles/tile-environment"; import { isSharedDataSet, SharedDataSet, SharedDataSetType } from "../../models/shared/shared-data-set"; import { ISharedModelManager } from "../../models/shared/shared-model-manager"; import { clueDataColorInfo } from "../../utilities/color-utils"; +import { displayValue } from "./bar-graph-utils"; export function defaultBarGraphContent(): BarGraphContentModelType { return BarGraphContentModel.create({yAxisLabel: "Counts"}); @@ -62,6 +63,7 @@ export const BarGraphContentModel = TileContentModel * { species: "owl", backyard: 1, street: 0, forest: 2 } * ] * ``` + * Any empty values of attributes are replaced with "(no value)". */ get dataArray() { const dataSet = self.dataSet?.dataSet; @@ -72,8 +74,8 @@ export const BarGraphContentModel = TileContentModel if (secondary) { // Two-dimensionsal data return cases.reduce((acc, caseID) => { - const cat = dataSet.getStrValue(caseID.__id__, primary); - const subCat = dataSet.getStrValue(caseID.__id__, secondary); + const cat = displayValue(dataSet.getStrValue(caseID.__id__, primary)); + const subCat = displayValue(dataSet.getStrValue(caseID.__id__, secondary)); const index = acc.findIndex(r => r[primary] === cat); if (index >= 0) { const cur = acc[index][subCat]; @@ -87,7 +89,7 @@ export const BarGraphContentModel = TileContentModel } else { // One-dimensional data return cases.reduce((acc, caseID) => { - const cat = dataSet.getStrValue(caseID.__id__, primary); + const cat = displayValue(dataSet.getStrValue(caseID.__id__, primary)); const index = acc.findIndex(r => r[primary] === cat); if (index >= 0) { const cur = acc[index].value; diff --git a/src/plugins/bar-graph/bar-graph-utils.ts b/src/plugins/bar-graph/bar-graph-utils.ts index 65c920e04e..bd7616abe3 100644 --- a/src/plugins/bar-graph/bar-graph-utils.ts +++ b/src/plugins/bar-graph/bar-graph-utils.ts @@ -1,5 +1,16 @@ +const kMissingValueString = "(no value)"; -// Just wraps the native getBBox method to make it mockable in tests +// Substitute "(no value)" for missing data +export function displayValue(attrValue: string | undefined): string { + return attrValue ? attrValue : kMissingValueString; +} + +// true if the string matches the pattern that we use for missing data +export function isMissingData(display: string): boolean { + return display === kMissingValueString; +} + +// Wraps the native getBBox method to make it mockable in tests export function getBBox(element: SVGGraphicsElement): DOMRect { return element.getBBox(); } diff --git a/src/plugins/bar-graph/bar-graph.scss b/src/plugins/bar-graph/bar-graph.scss index d37c080aa1..3dd8597421 100644 --- a/src/plugins/bar-graph/bar-graph.scss +++ b/src/plugins/bar-graph/bar-graph.scss @@ -97,6 +97,10 @@ margin: 0; } } + + .secondary-value-name.missing { + font-style: italic; + } } } diff --git a/src/plugins/bar-graph/legend-secondary-row.tsx b/src/plugins/bar-graph/legend-secondary-row.tsx index e8c1083e64..35f3344802 100644 --- a/src/plugins/bar-graph/legend-secondary-row.tsx +++ b/src/plugins/bar-graph/legend-secondary-row.tsx @@ -1,5 +1,7 @@ import React from 'react'; +import classNames from 'classnames'; import { useBarGraphModelContext } from './bar-graph-content-context'; +import { isMissingData } from './bar-graph-utils'; interface IProps { attrValue: string; @@ -9,12 +11,16 @@ export function LegendSecondaryRow({attrValue}: IProps) { const model = useBarGraphModelContext(); if (!model) return null; + const missingData = isMissingData(attrValue); + return (
-
{attrValue}
+
+ {attrValue} +
); } From edcf1f085aa1689eb16ca4c11b081db8d771f750 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 9 Sep 2024 16:53:55 -0400 Subject: [PATCH 09/36] More consistent naming --- cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js | 4 ++-- cypress/support/elements/common/cCanvas.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index c3cc8ee312..3ebe3f6fe4 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -65,8 +65,8 @@ context('Bar Graph Tile', function () { barGraph.getXAxisPulldownButton(1).should('have.text', 'Categories'); cy.log('Delete tile'); - clueCanvas.deleteTile('bar-graph'); - clueCanvas.deleteTile('bar-graph'); + clueCanvas.deleteTile('bargraph'); + clueCanvas.deleteTile('bargraph'); barGraph.getTiles().should('have.length', 0); }); diff --git a/cypress/support/elements/common/cCanvas.js b/cypress/support/elements/common/cCanvas.js index e2ba334124..7d6fea227b 100644 --- a/cypress/support/elements/common/cCanvas.js +++ b/cypress/support/elements/common/cCanvas.js @@ -297,7 +297,7 @@ class ClueCanvas { case 'xyplot': tileElement = xyPlotToolTile.getTile().last().click({ force: true }); break; - case 'bar-graph': + case 'bargraph': tileElement = barGraphTile.getTile().last().click({ force: true }); break; } From 40f2198beb56ee7fd126d8f3838b710e012fd04c Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 9 Sep 2024 16:59:28 -0400 Subject: [PATCH 10/36] Typo --- cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index 3ebe3f6fe4..329a3b9a4f 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -24,7 +24,7 @@ function beforeTest() { context('Bar Graph Tile', function () { - it('Basic tile operations, ', function () { + it('Basic tile operations', function () { beforeTest(); clueCanvas.addTile('bargraph'); From 4620d4015c81ba57f9ba9911c42eb5eb94ddde7c Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 9 Sep 2024 17:21:47 -0400 Subject: [PATCH 11/36] Test for (no value). Update documentation --- docs/unit-configuration.md | 6 ++ .../bar-graph/bar-graph-content.test.ts | 63 +++++++++++++------ 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/docs/unit-configuration.md b/docs/unit-configuration.md index ab465935d2..c5acb68790 100644 --- a/docs/unit-configuration.md +++ b/docs/unit-configuration.md @@ -120,6 +120,12 @@ Under 'dataset', there is one option: - `cellsSelectCases`: boolean +#### Bar Graph + +Common toolbar framework. One default button: + +- `link-tile`: bring up the linking dialog to connect/disconnect a dataset + #### Data Deck Not updated to common toolbar framework. However, supports toolbar configuration in a similar manner. Default buttons: diff --git a/src/plugins/bar-graph/bar-graph-content.test.ts b/src/plugins/bar-graph/bar-graph-content.test.ts index 9299df3cb8..3765a8c430 100644 --- a/src/plugins/bar-graph/bar-graph-content.test.ts +++ b/src/plugins/bar-graph/bar-graph-content.test.ts @@ -11,16 +11,20 @@ const mockCases: ICaseCreation[] = [ { species: "owl", location: "forest" } ]; -const emptyDataSet = DataSet.create(); -addAttributeToDataSet(emptyDataSet, { id: "att-s", name: "species" }); -addAttributeToDataSet(emptyDataSet, { id: "att-l", name: "location" }); -const sharedEmptyDataSet = SharedDataSet.create({ dataSet: emptyDataSet }); +function sharedEmptyDataSet() { + const emptyDataSet = DataSet.create(); + addAttributeToDataSet(emptyDataSet, { id: "att-s", name: "species" }); + addAttributeToDataSet(emptyDataSet, { id: "att-l", name: "location" }); + return SharedDataSet.create({ dataSet: emptyDataSet }); +} -const sampleDataSet = DataSet.create(); -addAttributeToDataSet(sampleDataSet, { id: "att-s", name: "species" }); -addAttributeToDataSet(sampleDataSet, { id: "att-l", name: "location" }); -addCasesToDataSet(sampleDataSet, mockCases); -const sharedSampleDataSet = SharedDataSet.create({ dataSet: sampleDataSet }); +function sharedSampleDataSet() { + const sampleDataSet = DataSet.create(); + addAttributeToDataSet(sampleDataSet, { id: "att-s", name: "species" }); + addAttributeToDataSet(sampleDataSet, { id: "att-l", name: "location" }); + addCasesToDataSet(sampleDataSet, mockCases); + return SharedDataSet.create({ dataSet: sampleDataSet }); +} const TestableBarGraphContentModel = BarGraphContentModel .actions(self => ({ @@ -75,19 +79,19 @@ Object { it("returns empty data array when there are no cases", () => { const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedEmptyDataSet); + content.setDataSet(sharedEmptyDataSet()); expect(content.dataArray).toEqual([]); }); it("returns empty data array when there is no primary attribute", () => { const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet); + content.setDataSet(sharedSampleDataSet()); expect(content.dataArray).toEqual([]); }); it("returns expected data array with primary attribute", () => { const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet); + content.setDataSet(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); expect(content.dataArray).toEqual([ { "att-s": "cat", "value": 2 }, @@ -103,25 +107,46 @@ Object { it("returns expected data array with primary and secondary attributes", () => { const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet); + content.setDataSet(sharedSampleDataSet()); + content.setPrimaryAttribute("att-s"); + content.setSecondaryAttribute("att-l"); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "yard": 2 }, + { "att-s": "owl", "yard": 1, "forest": 1 } + ]); + }); + + it("fills in missing values with (no value)", () => { + const content = TestableBarGraphContentModel.create({ }); + const dataSet = sharedSampleDataSet(); + dataSet.dataSet?.attributes[1].setValue(3, undefined); // hide forest owl's location + content.setDataSet(dataSet); content.setPrimaryAttribute("att-s"); content.setSecondaryAttribute("att-l"); expect(content.dataArray).toEqual([ - { "att-s": "cat", "yard": 2 }, - { "att-s": "owl", "yard": 1, "forest": 1 } - ]); + { "att-s": "cat", "yard": 2 }, + { "att-s": "owl", "yard": 1, "(no value)": 1 } + ]); + + dataSet.dataSet?.attributes[0].setValue(3, undefined); // hide that owl entirely + expect(content.dataArray).toEqual([ + { "att-s": "cat", "yard": 2 }, + { "att-s": "owl", "yard": 1 }, + { "att-s": "(no value)", "(no value)": 1 } + ]); + }); it("extracts primary keys", () => { const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet); + content.setDataSet(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); expect(content.primaryKeys).toEqual(["cat", "owl"]); }); it("extracts secondary keys", () => { const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet); + content.setDataSet(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); content.setSecondaryAttribute("att-l"); expect(content.secondaryKeys).toEqual(["yard", "forest"]); @@ -129,7 +154,7 @@ Object { it("calculates the maximum data value", () => { const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet); + content.setDataSet(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); expect(content.maxDataValue).toBe(2); From 6865b74cf1ca78ec158798876b48021d06517c91 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Tue, 10 Sep 2024 16:28:11 -0400 Subject: [PATCH 12/36] Adjust tile size when legend changes. Restyle 'vertical' layout --- src/plugins/bar-graph/bar-graph-tile.tsx | 56 ++++++++++++++---- src/plugins/bar-graph/bar-graph.scss | 52 ++++++++++++---- src/plugins/bar-graph/legend-area.tsx | 75 +++++++++++++----------- src/plugins/graph/graph-registration.ts | 2 +- 4 files changed, 125 insertions(+), 60 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-tile.tsx b/src/plugins/bar-graph/bar-graph-tile.tsx index ac095ac6bd..05ceed2236 100644 --- a/src/plugins/bar-graph/bar-graph-tile.tsx +++ b/src/plugins/bar-graph/bar-graph-tile.tsx @@ -1,35 +1,67 @@ -import React from "react"; +import React, { useRef } from "react"; import classNames from "classnames"; import { observer } from "mobx-react"; import { useResizeDetector } from "react-resize-detector"; +import { ChartArea } from "./chart-area"; +import { LegendArea } from "./legend-area"; import { BasicEditableTileTitle } from "../../components/tiles/basic-editable-tile-title"; import { ITileProps } from "../../components/tiles/tile-component"; -import { ChartArea } from "./chart-area"; import { BarGraphModelContext } from "./bar-graph-content-context"; import { isBarGraphModel } from "./bar-graph-content"; import { TileToolbar } from "../../components/toolbar/tile-toolbar"; -import { LegendArea } from "./legend-area"; import "./bar-graph.scss"; import "./bar-graph-toolbar"; const legendWidth = 190; -const legendHeight = 190; // FIXME export const BarGraphComponent: React.FC = observer((props: ITileProps) => { - - const { model, readOnly } = props; + const { model, readOnly, onRequestRowHeight } = props; const content = isBarGraphModel(model.content) ? model.content : null; - const {height: containerHeight, width: containerWidth, ref} = useResizeDetector(); + const requestedHeight = useRef(undefined); + + const onResize = (width: number|undefined, height: number|undefined) => { + let desiredTileHeight; + if (height) { + if (legendBelow) { + const desiredLegendHeight = height; + desiredTileHeight = 300 + desiredLegendHeight; + } else { + const desiredLegendHeight = Math.max(height, 260); // Leave room for at least 5 rows per spec + desiredTileHeight = desiredLegendHeight + 66; + } + if (requestedHeight.current !== desiredTileHeight) { + requestedHeight.current = desiredTileHeight; + onRequestRowHeight(model.id, desiredTileHeight); + } + } + }; + + // We use two resize detectors to track the size of the container and the size of the legend area + const { height: containerHeight, width: containerWidth, ref: containerRef } = useResizeDetector(); + + const { height: legendHeight, ref: legendRef } = useResizeDetector({ + refreshMode: 'debounce', + refreshRate: 500, + skipOnMount: false, + onResize + }); + let svgWidth = 10, svgHeight = 10; + // Legend is on the right if the width is >= 450px, otherwise below const legendBelow = containerWidth && containerWidth < 450; if (containerWidth && containerHeight) { - // Legend is on the right if the width is >= 450px - svgWidth = legendBelow ? containerWidth : containerWidth-legendWidth; - svgHeight = legendBelow ? containerHeight-legendHeight : containerHeight; + if (legendBelow) { + const vertPadding = 18; + svgWidth = containerWidth; + svgHeight = containerHeight - vertPadding - (legendHeight || 0); + } else { + svgWidth = containerWidth - legendWidth; + svgHeight = containerHeight; + } } return ( @@ -37,12 +69,12 @@ export const BarGraphComponent: React.FC = observer((props: ITilePro
- +
); diff --git a/src/plugins/bar-graph/bar-graph.scss b/src/plugins/bar-graph/bar-graph.scss index 3dd8597421..862b76a1c5 100644 --- a/src/plugins/bar-graph/bar-graph.scss +++ b/src/plugins/bar-graph/bar-graph.scss @@ -9,13 +9,7 @@ align-items: center; overflow: auto; - &.vertical { - flex-direction: column; - } - svg.bar-graph-svg { - // width: 100%; - // height: 100%; .visx-bar-group .visx-bar { stroke: black; @@ -104,12 +98,46 @@ } } - &.vertical div.bar-graph-legend { - width: 100%; - height: 186px; - border-top: 1.5px solid #0592af; - border-left: none; - padding: 8px 0 0 0; + // Overrides for vertical (legend underneath) layout + &.vertical { + flex-direction: column; + + div.bar-graph-legend { + width: 100%; + height: auto; + border-top: 1.5px solid #0592af; + border-left: none; + padding: 8px 0 0 0; + + .dataset-header { + margin-left: 5px; + align-items: center; + + .dataset-label-text, .dataset-name { + display: inline-block; + margin-right: 5px; + } + } + + .sort-by { + margin: 0 0 10px 20px; + + button.chakra-menu__menu-button { + width: auto; + margin: 5px 0 0 5px; + } + } + + .secondary-values { + display: flex; + flex-wrap: wrap; + margin-left: 8px; + + .color-button { + margin: 0 7px 0 7px; + } + } + } } button.chakra-menu__menu-button { diff --git a/src/plugins/bar-graph/legend-area.tsx b/src/plugins/bar-graph/legend-area.tsx index 6feb327200..bd68d9eaf2 100644 --- a/src/plugins/bar-graph/legend-area.tsx +++ b/src/plugins/bar-graph/legend-area.tsx @@ -7,8 +7,11 @@ import { LegendSecondaryRow } from './legend-secondary-row'; import RemoveDataIcon from "../../assets/remove-data-icon.svg"; import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; +interface IProps { + legendRef: React.RefObject; +} -export const LegendArea = observer(function LegendArea () { +export const LegendArea = observer(function LegendArea ({legendRef}: IProps) { const model = useBarGraphModelContext(); function unlinkDataset() { @@ -38,44 +41,46 @@ export const LegendArea = observer(function LegendArea () { return (
-
-
- - - +
+
+
+ + + +
+
+ Data from: + {model.dataSet.name} +
-
- Data from: - {model.dataSet.name} -
-
-
-
- Sort by: +
+ + Sort by: + + + + + {currentLabel} + + + + + + setSecondaryAttribute(undefined)}>None + {availableAttributes.map((a) => ( + setSecondaryAttribute(a.id)}>{a.name} + ))} + + +
- - - - {currentLabel} - - - - - - setSecondaryAttribute(undefined)}>None - {availableAttributes.map((a) => ( - setSecondaryAttribute(a.id)}>{a.name} - ))} - - - -
-
- {currentSecondary - ? secondaryKeys.map((key) => ) - : } +
+ {currentSecondary + ? secondaryKeys.map((key) => ) + : } +
); diff --git a/src/plugins/graph/graph-registration.ts b/src/plugins/graph/graph-registration.ts index 7d050940b6..b69a54df04 100644 --- a/src/plugins/graph/graph-registration.ts +++ b/src/plugins/graph/graph-registration.ts @@ -5,10 +5,10 @@ import { GraphWrapperComponent } from "./components/graph-wrapper-component"; import { createGraphModel, GraphModel } from "./models/graph-model"; import { updateGraphContentWithNewSharedModelIds, updateGraphObjectWithNewSharedModelIds } from "./utilities/graph-utils"; + import { AppConfigModelType } from "../../models/stores/app-config-model"; import Icon from "./assets/graph-icon.svg"; import HeaderIcon from "./assets/graph-tile-id.svg"; -import { AppConfigModelType } from "../../models/stores/app-config-model"; function graphAllowsMultipleDataSets(appConfig: AppConfigModelType) { return !!appConfig.getSetting("defaultSeriesLegend", "graph"); From d8acf33d02f022a5b1b47e29a6c112d8585aadd1 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Wed, 11 Sep 2024 08:29:54 -0400 Subject: [PATCH 13/36] Add test for vertical layout --- .../tile_tests/bar_graph_tile_spec.js | 29 +++++++++++++------ cypress/support/elements/tile/BarGraphTile.js | 4 +++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index 329a3b9a4f..829558e4ae 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -73,6 +73,15 @@ context('Bar Graph Tile', function () { it('Can link data ', function () { beforeTest(); + clueCanvas.addTile('bargraph'); + barGraph.getTiles().click(); + barGraph.getYAxisLabel().should('have.text', 'Counts'); + barGraph.getXAxisPulldown().should('have.text', 'Categories'); + barGraph.getYAxisTickLabel().should('not.exist'); + barGraph.getXAxisTickLabel().should('not.exist'); + barGraph.getLegendArea().should('not.exist'); + barGraph.getBar().should('not.exist'); + // Table dataset for testing: // 4 instances of X / Y / Z // 2 instances of XX / Y / Z @@ -88,16 +97,8 @@ context('Bar Graph Tile', function () { ['X', 'Y', 'Z'], ]); - clueCanvas.addTile('bargraph'); - barGraph.getTiles().click(); - barGraph.getYAxisLabel().should('have.text', 'Counts'); - barGraph.getXAxisPulldown().should('have.text', 'Categories'); - barGraph.getYAxisTickLabel().should('not.exist'); - barGraph.getXAxisTickLabel().should('not.exist'); - barGraph.getLegendArea().should('not.exist'); - barGraph.getBar().should('not.exist'); - cy.log('Link bar graph'); + barGraph.getTile().click(); clueCanvas.clickToolbarButton('bargraph', 'link-tile'); cy.get('select').select('Table Data 1'); cy.get('.modal-button').contains("Graph It!").click(); @@ -111,6 +112,16 @@ context('Bar Graph Tile', function () { barGraph.getSortByMenuButton().should('have.text', 'None'); barGraph.getSecondaryValueName().should('have.length', 1).and('have.text', 'x'); + cy.log('Legend should move to bottom when tile is narrow'); + barGraph.getTileContent().should('have.class', 'horizontal').and('not.have.class', 'vertical'); + clueCanvas.addTileByDrag('table', 'right'); + clueCanvas.addTileByDrag('table', 'right'); + barGraph.getTileContent().should('have.class', 'vertical').and('not.have.class', 'horizontal'); + clueCanvas.getUndoTool().click(); // undo add table + clueCanvas.getUndoTool().click(); // undo add table + tableTile.getTableTile().should('have.length', 1); + barGraph.getTileContent().should('have.class', 'horizontal').and('not.have.class', 'vertical'); + cy.log('Change Sort By'); barGraph.getSortByMenuButton().click(); barGraph.getChakraMenuItem().should('have.length', 3); diff --git a/cypress/support/elements/tile/BarGraphTile.js b/cypress/support/elements/tile/BarGraphTile.js index 6eeea90682..5fe641923f 100644 --- a/cypress/support/elements/tile/BarGraphTile.js +++ b/cypress/support/elements/tile/BarGraphTile.js @@ -12,6 +12,10 @@ class BarGraphTile { return this.getTile(tileIndex, workspaceClass).find(`.editable-tile-title-text`); } + getTileContent(tileIndex = 0, workspaceClass) { + return this.getTile(tileIndex, workspaceClass).find(`[data-testid="bar-graph-content"]`); + } + getChakraMenuItem(tileIndex = 0, workspaceClass) { return cy.get(`body .chakra-portal button`).filter(':visible'); } From 5ce60bd18985b86fea0e8491f9105914af436e93 Mon Sep 17 00:00:00 2001 From: nstclair-cc <20171905+nstclair-cc@users.noreply.github.com> Date: Wed, 11 Sep 2024 10:22:46 -0700 Subject: [PATCH 14/36] added close to statement to allow tolerance for values of selected labels --- .../tile_tests/geometry_tool_spec.js | 98 ++++++++++++------- 1 file changed, 65 insertions(+), 33 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js index e231e1650d..c095696640 100644 --- a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js @@ -123,7 +123,7 @@ context('Geometry Tool', function () { geometryToolTile.getGraphAxisTickLabels().last().should("have.text", "15"); }); - it('works in all four modes', () => { + it.only('works in all four modes', () => { beforeTest(); clueCanvas.addTile('geometry'); geometryToolTile.getGraph().should("exist"); @@ -417,47 +417,79 @@ context('Geometry Tool', function () { clueCanvas.clickToolbarButton('geometry', 'label'); geometryToolTile.getModalTitle().should('contain.text', 'Length'); geometryToolTile.chooseLabelOption('length'); - geometryToolTile.clickGraphPosition(0, 0); // deselect + geometryToolTile.clickGraphPosition(20, 20); // deselect - // Verify that the two line segments were created - geometryToolTile.getGraphPointLabel().contains('10.0').should('exist'); - geometryToolTile.getGraphPointLabel().contains('5.0').should('exist'); +// Store initial values for the line segments +let originalValue1 = 10.0; +let originalValue2 = 5.0; - // Move the point - clueCanvas.clickToolbarButton('geometry', 'select'); - geometryToolTile.clickGraphPosition(15, 5); // shared point - geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 39 }); // simulate right arrow key press - geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 38 }); // simulate up arrow key press +// Verify that the first point label is close to 10.0 +geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(originalValue1, 0.1); // Allow tolerance for value close to 10.0 +}); - // Verify that the point values changed - geometryToolTile.getSelectedGraphPoint().then(($point) => { - const newPx = parseFloat($point.attr('cx')); // 'px' for point x-coordinate - const newPy = parseFloat($point.attr('cy')); // 'py' for point y-coordinate +// Verify that the second point label is close to 5.0 +geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(originalValue2, 0.1); // Allow tolerance for value close to 5.0 +}); - expect(newPx).to.be.greaterThan(originalCx); - expect(newPy).to.be.lessThan(originalCy); - }); +// Move the point +clueCanvas.clickToolbarButton('geometry', 'select'); +geometryToolTile.clickGraphPosition(15, 5); // shared point +geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 39 }); // simulate right arrow key press +geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 38 }); // simulate up arrow key press - // Verify that the two line segments have changed - geometryToolTile.getGraphPointLabel().contains('10.1').should('exist'); - geometryToolTile.getGraphPointLabel().contains('4.9').should('exist'); +// Updated expected values after the point move +let updatedValue1 = originalValue1 + 0.1; // 10.1 +let updatedValue2 = originalValue2 - 0.1; // 4.9 - // Move the point back to the original position - geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 37 }); // simulate left arrow key press - geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 40 }); // simulate down arrow key press +// Verify that the point values changed +geometryToolTile.getSelectedGraphPoint().then(($point) => { + const newPx = parseFloat($point.attr('cx')); + const newPy = parseFloat($point.attr('cy')); - // Verify that the point has returned to its original coordinates - geometryToolTile.getSelectedGraphPoint().then(($point) => { - const resetPx = parseFloat($point.attr('cx')); - const resetPy = parseFloat($point.attr('cy')); + expect(newPx).to.be.greaterThan(originalCx); + expect(newPy).to.be.lessThan(originalCy); +}); - expect(resetPx).to.equal(originalCx); - expect(resetPy).to.equal(originalCy); - }); +// Verify that the first line segment has changed to a value close to 10.1 +geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(updatedValue1, 0.1); // Tolerance for value close to 10.1 +}); + +// Verify that the second line segment has changed to a value close to 4.9 +geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(updatedValue2, 0.1); // Tolerance for value close to 4.9 +}); + +// Move the point back to the original position +geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 37 }); // simulate left arrow key press +geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 40 }); // simulate down arrow key press - // Verify that the two line segments returned to their original values - geometryToolTile.getGraphPointLabel().contains('10.0').should('exist'); - geometryToolTile.getGraphPointLabel().contains('5.0').should('exist'); +// Verify that the point has returned to its original coordinates +geometryToolTile.getSelectedGraphPoint().then(($point) => { + const resetPx = parseFloat($point.attr('cx')); + const resetPy = parseFloat($point.attr('cy')); + + expect(resetPx).to.equal(originalCx); + expect(resetPy).to.equal(originalCy); +}); + +// Verify that the first line segment has returned to a value close to 10.0 +geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(originalValue1, 0.1); // Tolerance for value close to 10.0 +}); + +// Verify that the second line segment has returned to a value close to 5.0 +geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(originalValue2, 0.1); // Tolerance for value close to 5.0 +}); // Verify the point is still shared geometryToolTile.getGraphPoint().should("have.length", 6); // New point added From 166945b41ea402847c958b071ea4fecb7c598c72 Mon Sep 17 00:00:00 2001 From: nstclair-cc <20171905+nstclair-cc@users.noreply.github.com> Date: Wed, 11 Sep 2024 10:23:39 -0700 Subject: [PATCH 15/36] fixed indenting issues --- .../tile_tests/geometry_tool_spec.js | 118 +++++++++--------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js index c095696640..6194c6a6f9 100644 --- a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js @@ -419,77 +419,77 @@ context('Geometry Tool', function () { geometryToolTile.chooseLabelOption('length'); geometryToolTile.clickGraphPosition(20, 20); // deselect -// Store initial values for the line segments -let originalValue1 = 10.0; -let originalValue2 = 5.0; - -// Verify that the first point label is close to 10.0 -geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(originalValue1, 0.1); // Allow tolerance for value close to 10.0 -}); + // Store initial values for the line segments + let originalValue1 = 10.0; + let originalValue2 = 5.0; + + // Verify that the first point label is close to 10.0 + geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(originalValue1, 0.1); // Allow tolerance for value close to 10.0 + }); -// Verify that the second point label is close to 5.0 -geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(originalValue2, 0.1); // Allow tolerance for value close to 5.0 -}); + // Verify that the second point label is close to 5.0 + geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(originalValue2, 0.1); // Allow tolerance for value close to 5.0 + }); -// Move the point -clueCanvas.clickToolbarButton('geometry', 'select'); -geometryToolTile.clickGraphPosition(15, 5); // shared point -geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 39 }); // simulate right arrow key press -geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 38 }); // simulate up arrow key press + // Move the point + clueCanvas.clickToolbarButton('geometry', 'select'); + geometryToolTile.clickGraphPosition(15, 5); // shared point + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 39 }); // simulate right arrow key press + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 38 }); // simulate up arrow key press -// Updated expected values after the point move -let updatedValue1 = originalValue1 + 0.1; // 10.1 -let updatedValue2 = originalValue2 - 0.1; // 4.9 + // Updated expected values after the point move + let updatedValue1 = originalValue1 + 0.1; // 10.1 + let updatedValue2 = originalValue2 - 0.1; // 4.9 -// Verify that the point values changed -geometryToolTile.getSelectedGraphPoint().then(($point) => { - const newPx = parseFloat($point.attr('cx')); - const newPy = parseFloat($point.attr('cy')); + // Verify that the point values changed + geometryToolTile.getSelectedGraphPoint().then(($point) => { + const newPx = parseFloat($point.attr('cx')); + const newPy = parseFloat($point.attr('cy')); - expect(newPx).to.be.greaterThan(originalCx); - expect(newPy).to.be.lessThan(originalCy); -}); + expect(newPx).to.be.greaterThan(originalCx); + expect(newPy).to.be.lessThan(originalCy); + }); -// Verify that the first line segment has changed to a value close to 10.1 -geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(updatedValue1, 0.1); // Tolerance for value close to 10.1 -}); + // Verify that the first line segment has changed to a value close to 10.1 + geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(updatedValue1, 0.1); // Tolerance for value close to 10.1 + }); -// Verify that the second line segment has changed to a value close to 4.9 -geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(updatedValue2, 0.1); // Tolerance for value close to 4.9 -}); + // Verify that the second line segment has changed to a value close to 4.9 + geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(updatedValue2, 0.1); // Tolerance for value close to 4.9 + }); -// Move the point back to the original position -geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 37 }); // simulate left arrow key press -geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 40 }); // simulate down arrow key press + // Move the point back to the original position + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 37 }); // simulate left arrow key press + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 40 }); // simulate down arrow key press -// Verify that the point has returned to its original coordinates -geometryToolTile.getSelectedGraphPoint().then(($point) => { - const resetPx = parseFloat($point.attr('cx')); - const resetPy = parseFloat($point.attr('cy')); + // Verify that the point has returned to its original coordinates + geometryToolTile.getSelectedGraphPoint().then(($point) => { + const resetPx = parseFloat($point.attr('cx')); + const resetPy = parseFloat($point.attr('cy')); - expect(resetPx).to.equal(originalCx); - expect(resetPy).to.equal(originalCy); -}); + expect(resetPx).to.equal(originalCx); + expect(resetPy).to.equal(originalCy); + }); -// Verify that the first line segment has returned to a value close to 10.0 -geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(originalValue1, 0.1); // Tolerance for value close to 10.0 -}); + // Verify that the first line segment has returned to a value close to 10.0 + geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(originalValue1, 0.1); // Tolerance for value close to 10.0 + }); -// Verify that the second line segment has returned to a value close to 5.0 -geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(originalValue2, 0.1); // Tolerance for value close to 5.0 -}); + // Verify that the second line segment has returned to a value close to 5.0 + geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(originalValue2, 0.1); // Tolerance for value close to 5.0 + }); // Verify the point is still shared geometryToolTile.getGraphPoint().should("have.length", 6); // New point added From aeead05f2064dd56788a62c8269be231193ff166 Mon Sep 17 00:00:00 2001 From: nstclair-cc <20171905+nstclair-cc@users.noreply.github.com> Date: Wed, 11 Sep 2024 13:23:27 -0700 Subject: [PATCH 16/36] Added tolerance statements for automation of polygon points --- .../tile_tests/geometry_tool_spec.js | 58 ++++++++++++------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js index 6194c6a6f9..32b9e8e0c3 100644 --- a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js @@ -123,7 +123,7 @@ context('Geometry Tool', function () { geometryToolTile.getGraphAxisTickLabels().last().should("have.text", "15"); }); - it.only('works in all four modes', () => { + it('works in all four modes', () => { beforeTest(); clueCanvas.addTile('geometry'); geometryToolTile.getGraph().should("exist"); @@ -419,31 +419,38 @@ context('Geometry Tool', function () { geometryToolTile.chooseLabelOption('length'); geometryToolTile.clickGraphPosition(20, 20); // deselect - // Store initial values for the line segments - let originalValue1 = 10.0; - let originalValue2 = 5.0; + let origPointLabel1 = 10.0; + let origPointLabel2 = 5.0; // Verify that the first point label is close to 10.0 geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { const value = parseFloat(text); - expect(value).to.be.closeTo(originalValue1, 0.1); // Allow tolerance for value close to 10.0 + expect(value).to.be.closeTo(origPointLabel1, 0.1); // Allow tolerance for value close to 10.0 }); // Verify that the second point label is close to 5.0 geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { const value = parseFloat(text); - expect(value).to.be.closeTo(originalValue2, 0.1); // Allow tolerance for value close to 5.0 + expect(value).to.be.closeTo(origPointLabel2, 0.1); // Allow tolerance for value close to 5.0 }); // Move the point clueCanvas.clickToolbarButton('geometry', 'select'); geometryToolTile.clickGraphPosition(15, 5); // shared point - geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 39 }); // simulate right arrow key press - geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 38 }); // simulate up arrow key press + + // Simulate 4 right arrow key presses + for (let i = 0; i < 4; i++) { + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 39 }); + } + + // Simulate 4 up arrow key presses + for (let i = 0; i < 4; i++) { + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 38 }); + } // Updated expected values after the point move - let updatedValue1 = originalValue1 + 0.1; // 10.1 - let updatedValue2 = originalValue2 - 0.1; // 4.9 + let updatedValue1 = origPointLabel1 + 0.4; // 10.4 + let updatedValue2 = origPointLabel2 - 0.4; // 4.6 // Verify that the point values changed geometryToolTile.getSelectedGraphPoint().then(($point) => { @@ -454,21 +461,28 @@ context('Geometry Tool', function () { expect(newPy).to.be.lessThan(originalCy); }); - // Verify that the first line segment has changed to a value close to 10.1 + // Verify that the first line segment has changed to a value close to 10.4 geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(updatedValue1, 0.1); // Tolerance for value close to 10.1 + const lineSegment1 = parseFloat(text); + expect(lineSegment1).to.be.closeTo(updatedValue1, 0.1); // Tolerance for value close to 10.4 }); - // Verify that the second line segment has changed to a value close to 4.9 + // Verify that the second line segment has changed to a value close to 4.6 geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(updatedValue2, 0.1); // Tolerance for value close to 4.9 + const lineSegment2 = parseFloat(text); + expect(lineSegment2).to.be.closeTo(updatedValue2, 0.1); // Tolerance for value close to 4.6 }); // Move the point back to the original position - geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 37 }); // simulate left arrow key press - geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 40 }); // simulate down arrow key press + // Simulate 4 left arrow key presses + for (let i = 0; i < 4; i++) { + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 37 }); + } + + // Simulate 4 down arrow key presses + for (let i = 0; i < 4; i++) { + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 40 }); + } // Verify that the point has returned to its original coordinates geometryToolTile.getSelectedGraphPoint().then(($point) => { @@ -481,14 +495,14 @@ context('Geometry Tool', function () { // Verify that the first line segment has returned to a value close to 10.0 geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(originalValue1, 0.1); // Tolerance for value close to 10.0 + const lineSegment1 = parseFloat(text); + expect(lineSegment1).to.be.closeTo(origPointLabel1, 0.1); // Tolerance for value close to 10.0 }); // Verify that the second line segment has returned to a value close to 5.0 geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { - const value = parseFloat(text); - expect(value).to.be.closeTo(originalValue2, 0.1); // Tolerance for value close to 5.0 + const lineSegment2 = parseFloat(text); + expect(lineSegment2).to.be.closeTo(origPointLabel2, 0.1); // Tolerance for value close to 5.0 }); // Verify the point is still shared From 8aa238e60194d4fdc6074ba14aed584176f92116 Mon Sep 17 00:00:00 2001 From: nstclair-cc <20171905+nstclair-cc@users.noreply.github.com> Date: Wed, 11 Sep 2024 16:04:44 -0700 Subject: [PATCH 17/36] fix rebase issues --- cypress/e2e/functional/tile_tests/geometry_tool_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js index 32b9e8e0c3..0b74277a14 100644 --- a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js @@ -301,7 +301,7 @@ context('Geometry Tool', function () { // Also check that the angle label has changed from its original value geometryToolTile.getAngleAdornment().should(($label) => { const angleText = $label.text(); - expect(angleText).not.to.equal('90'); // 90° was the original value + expect(angleText).not.contains('90°'); // 90° was the original value }); // Move the point back to the original position @@ -385,7 +385,7 @@ context('Geometry Tool', function () { geometryToolTile.getGraphPolygon().should("have.length", 1); // Create a second polygon that shares the same points as the first - cy.log('Create a second polygon that shares the same points as the first'); + cy.log('Create a second polygon that shares a point with the first'); clueCanvas.clickToolbarButton('geometry', 'polygon'); geometryToolTile.clickGraphPosition(15, 10); // new point geometryToolTile.clickGraphPosition(15, 5); // shared point From f002638ad88130ab0bf977fa3e0fb7fdbdfc797b Mon Sep 17 00:00:00 2001 From: nstclair-cc <20171905+nstclair-cc@users.noreply.github.com> Date: Wed, 11 Sep 2024 16:35:19 -0700 Subject: [PATCH 18/36] fix 90 statement --- cypress/e2e/functional/tile_tests/geometry_tool_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js index 0b74277a14..37aee96f83 100644 --- a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js @@ -301,7 +301,7 @@ context('Geometry Tool', function () { // Also check that the angle label has changed from its original value geometryToolTile.getAngleAdornment().should(($label) => { const angleText = $label.text(); - expect(angleText).not.contains('90°'); // 90° was the original value + expect(angleText).not.to.equal('90'); // 90° was the original value }); // Move the point back to the original position From 8911681d391832628791f408fd3772d63a365652 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Thu, 12 Sep 2024 10:23:47 -0400 Subject: [PATCH 19/36] use local storage for the dev root id - needed to relax the security rules for the dev root - update user icon roll over so devs know the root - update firebase and firestore to share the same logic for computing the root id based on the appMode --- firestore.rules | 7 ++-- src/clue/components/clue-app-header.tsx | 18 +++++++-- src/lib/firebase.test.ts | 54 +++++++++++++++++++++++++ src/lib/firebase.ts | 13 ++---- src/lib/firestore.test.ts | 34 ++++++++++++++++ src/lib/firestore.ts | 22 ++-------- src/lib/root-id.ts | 46 +++++++++++++++++++++ 7 files changed, 157 insertions(+), 37 deletions(-) create mode 100644 src/lib/firebase.test.ts create mode 100644 src/lib/root-id.ts diff --git a/firestore.rules b/firestore.rules index 7f01a54011..72a79b3526 100644 --- a/firestore.rules +++ b/firestore.rules @@ -426,10 +426,9 @@ service cloud.firestore { allow read, write: if isAuthed(); } - match /dev/{userId}/{restOfPath=**} { - allow read: if isAuthed(); - // users can only write to their own folders - allow write: if matchFirebaseUserId(userId); + // Developers get a random unique key that their data is stored under + match /dev/{devId}/{restOfPath=**} { + allow read, write: if isAuthed(); } match /qa/{userId}/{restOfPath=**} { diff --git a/src/clue/components/clue-app-header.tsx b/src/clue/components/clue-app-header.tsx index 33dc5dc88e..56a09e76ab 100644 --- a/src/clue/components/clue-app-header.tsx +++ b/src/clue/components/clue-app-header.tsx @@ -9,6 +9,7 @@ import { ToggleGroup } from "@concord-consortium/react-components"; import { GroupModelType, GroupUserModelType } from "../../models/stores/groups"; import { CustomSelect } from "./custom-select"; import { useStores } from "../../hooks/use-stores"; +import { getDevId } from "../../lib/root-id"; // cf. https://mattferderer.com/use-sass-variables-in-typescript-and-javascript import styles from "./toggle-buttons.scss"; @@ -26,8 +27,17 @@ export const ClueAppHeaderComponent: React.FC = observer(function ClueAp const { showGroup } = props; const { appConfig, appMode, appVersion, db, user, problem, groups, investigation, ui, unit } = useStores(); const myGroup = showGroup ? groups.getGroupById(user.currentGroupId) : undefined; - const userTitle = appMode !== "authed" && appMode !== "demo" - ? `Firebase UID: ${db.firebase.userId}` : undefined; + const getUserTitle = () => { + switch(appMode){ + case "dev": + return `Firebase Root: ${getDevId()}`; + case "qa": + case "test": + return `Firebase UID: ${db.firebase.userId}`; + default: + return undefined; + } + }; const renderPanelButtons = () => { const { panels, onPanelChange, current} = props; @@ -150,7 +160,7 @@ export const ClueAppHeaderComponent: React.FC = observer(function ClueAp
Version {appVersion}
-
+
@@ -191,7 +201,7 @@ export const ClueAppHeaderComponent: React.FC = observer(function ClueAp
Version {appVersion}
{myGroup ? renderGroup(myGroup) : null} -
+
{user.name}
{user.className}
diff --git a/src/lib/firebase.test.ts b/src/lib/firebase.test.ts new file mode 100644 index 0000000000..f1d31a6f0a --- /dev/null +++ b/src/lib/firebase.test.ts @@ -0,0 +1,54 @@ +import { DB } from "./db"; +import { Firebase } from "./firebase"; +import { kClueDevIDKey } from "./root-id"; + +const mockStores = { + appMode: "authed", + demo: { name: "demo" }, + user: { portal: "test-portal" } +}; +const mockDB = { + stores: mockStores +} as DB; + +describe("Firebase class", () => { + describe("initialization", () => { + it("should create a valid Firebase object", () => { + const firebase = new Firebase(mockDB); + expect(firebase).toBeDefined(); + }); + }); + describe("getRootFolder", () => { + it("should handle authed mode", () => { + const firebase = new Firebase(mockDB); + expect(firebase.getRootFolder()).toBe("/authed/test-portal/portals/test-portal/"); + }); + it("should handle the dev appMode", () => { + window.localStorage.setItem(kClueDevIDKey, "random-id"); + const stores = {...mockStores, appMode: "dev"}; + const firestore = new Firebase({stores} as DB); + expect(firestore.getRootFolder()).toBe("/dev/random-id/portals/test-portal/"); + }); + describe("should handle the demo appMode", () => { + it("handles basic demo name", () => { + const stores = {...mockStores, + appMode: "demo", demo: { name: "test-demo" }}; + const firestore = new Firebase({stores} as DB); + expect(firestore.getRootFolder()).toBe("/demo/test-demo/portals/test-portal/"); + }); + it("handles empty demo name", () => { + const stores = {...mockStores, + appMode: "demo", demo: { name: "" }}; + const firestore = new Firebase({stores} as DB); + expect(firestore.getRootFolder()).toBe("/demo/test-portal/portals/test-portal/"); + }); + it("handles empty demo name and empty portal", () => { + const stores = {...mockStores, + appMode: "demo", demo: { name: "" }, user: { portal: ""}}; + const firestore = new Firebase({stores} as DB); + // FIXME + expect(firestore.getRootFolder()).toBe("/demo/demo/portals//"); + }); + }); + }); +}); diff --git a/src/lib/firebase.ts b/src/lib/firebase.ts index ef2f77393c..24e784b841 100644 --- a/src/lib/firebase.ts +++ b/src/lib/firebase.ts @@ -8,6 +8,7 @@ import { DB } from "./db"; import { escapeKey } from "./fire-utils"; import { urlParams } from "../utilities/url-params"; import { DocumentModelType } from "src/models/document/document"; +import { getRootId } from "./root-id"; // Set this during database testing in combination with the urlParam testMigration=true to // override the top-level Firebase key regardless of mode. For example, setting this to "authed-copy" @@ -65,22 +66,14 @@ export class Firebase { // dev: /dev//portals/localhost // qa: /qa//portals/qa // test: /test//portals/ - const { appMode, demo: { name: demoName }, user } = this.db.stores; + const { appMode, user } = this.db.stores; const parts = []; if (urlParams.testMigration === "true" && FIREBASE_ROOT_OVERRIDE) { parts.push(FIREBASE_ROOT_OVERRIDE); } else { parts.push(`${appMode}`); - if ((appMode === "dev") || (appMode === "test") || (appMode === "qa")) { - parts.push(this.userId); - } - else if (appMode === "demo") { - const slug = demoName && demoName.length > 0 ? escapeKey(demoName) : ""; - if (slug.length > 0) { - parts.push(slug); - } - } + parts.push(getRootId(this.db.stores, this.userId)); } parts.push("portals"); parts.push(escapeKey(user.portal)); diff --git a/src/lib/firestore.test.ts b/src/lib/firestore.test.ts index 2233c49331..78fe1f6c28 100644 --- a/src/lib/firestore.test.ts +++ b/src/lib/firestore.test.ts @@ -1,5 +1,6 @@ import { DB } from "./db"; import { Firestore } from "./firestore"; +import { kClueDevIDKey } from "./root-id"; const mockStores = { appMode: "authed", @@ -56,8 +57,41 @@ describe("Firestore class", () => { it("should create a valid Firestore object", () => { const firestore = new Firestore(mockDB); expect(firestore).toBeDefined(); + }); + }); + + describe("getRootFolder", () => { + beforeEach(() => resetMocks()); + it("should handle the authed appMode", () => { + const firestore = new Firestore(mockDB); expect(firestore.getRootFolder()).toBe("/authed/test-portal/"); }); + it("should handle the dev appMode", () => { + window.localStorage.setItem(kClueDevIDKey, "random-id"); + const stores = {...mockStores, appMode: "dev"}; + const firestore = new Firestore({stores} as DB); + expect(firestore.getRootFolder()).toBe("/dev/random-id/"); + }); + describe("should handle the demo appMode", () => { + it("handles basic demo name", () => { + const stores = {...mockStores, + appMode: "demo", demo: { name: "test-demo" }}; + const firestore = new Firestore({stores} as DB); + expect(firestore.getRootFolder()).toBe("/demo/test-demo/"); + }); + it("handles empty demo name", () => { + const stores = {...mockStores, + appMode: "demo", demo: { name: "" }}; + const firestore = new Firestore({stores} as DB); + expect(firestore.getRootFolder()).toBe("/demo/test-portal/"); + }); + it("handles empty demo name and empty portal", () => { + const stores = {...mockStores, + appMode: "demo", demo: { name: "" }, user: { portal: ""}}; + const firestore = new Firestore({stores} as DB); + expect(firestore.getRootFolder()).toBe("/demo/demo/"); + }); + }); }); describe("setFirebaseUser", () => { diff --git a/src/lib/firestore.ts b/src/lib/firestore.ts index cb867bbd8c..4575cd9611 100644 --- a/src/lib/firestore.ts +++ b/src/lib/firestore.ts @@ -1,6 +1,7 @@ import firebase from "firebase/app"; import "firebase/firestore"; import { DB } from "./db"; +import { getRootId } from "./root-id"; import { escapeKey } from "./fire-utils"; import { UserDocument } from "./firestore-schema"; @@ -40,25 +41,8 @@ export class Firestore { // public getRootFolder() { - const { appMode, demo: { name: demoName }, user: { portal } } = this.db.stores; - let rootDocId: string; - const escapedPortal = portal ? escapeKey(portal) : portal; - - // `authed/${escapedPortalDomain}` - if (appMode === "authed") { - rootDocId = escapedPortal; - } - // `demo/${escapedDemoName}` - else if ((appMode === "demo") && (demoName?.length > 0)) { - const escapedDemoName = demoName ? escapeKey(demoName) : demoName; - rootDocId = escapedDemoName || escapedPortal || "demo"; - } - // `${appMode}/${userId}` - else { // (appMode === "dev") || (appMode === "test") || (appMode === "qa") - rootDocId = this.userId; - } - - return `/${appMode}/${rootDocId}/`; + const { appMode } = this.db.stores; + return `/${appMode}/${getRootId(this.db.stores, this.userId)}/`; } public getFullPath(path = "") { diff --git a/src/lib/root-id.ts b/src/lib/root-id.ts new file mode 100644 index 0000000000..c75e476aca --- /dev/null +++ b/src/lib/root-id.ts @@ -0,0 +1,46 @@ +import { nanoid } from "nanoid"; +import { IStores } from "../models/stores/stores"; +import { escapeKey } from "./fire-utils"; + +export const kClueDevIDKey = "clue-dev-id"; + +export function getDevId() { + let devId = window.localStorage.getItem(kClueDevIDKey); + if (!devId) { + const newDevId = nanoid(); + window.localStorage.setItem(kClueDevIDKey, newDevId); + devId = newDevId; + } + return devId; +} + +type IRootDocIdStores = Pick; + +export function getRootId(stores: IRootDocIdStores, firebaseUserId: string) { + const { appMode, demo: { name: demoName }, user: { portal } } = stores; + const escapedPortal = portal ? escapeKey(portal) : portal; + + switch (appMode) { + case "authed": { + return escapedPortal; + } + case "demo": { + // Legacy Note: Previously if the demoName was "", the root id in the realtime database + // and Firestore would be different. The paths would end up being: + // database: /demo/portals/demo/ (root id is skipped) + // firestore: /demo/{firebaseUserId}/ + // Now the root id will be the default "demo" in this case so the paths will be: + // database: /demo/demo/portals/demo/ + // firestore: /demo/demo/ + const escapedDemoName = demoName ? escapeKey(demoName) : demoName; + return escapedDemoName || escapedPortal || "demo"; + } + case "dev": { + return getDevId(); + } + // "test" and "qa" + default: { + return firebaseUserId; + } + } +} From e02c6c80655c6c1b4a7366393057bd1e6138db64 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Thu, 12 Sep 2024 12:10:40 -0400 Subject: [PATCH 20/36] firebase authed root just has the portal --- src/lib/firebase.test.ts | 2 +- src/lib/firebase.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib/firebase.test.ts b/src/lib/firebase.test.ts index f1d31a6f0a..9db8b9f986 100644 --- a/src/lib/firebase.test.ts +++ b/src/lib/firebase.test.ts @@ -21,7 +21,7 @@ describe("Firebase class", () => { describe("getRootFolder", () => { it("should handle authed mode", () => { const firebase = new Firebase(mockDB); - expect(firebase.getRootFolder()).toBe("/authed/test-portal/portals/test-portal/"); + expect(firebase.getRootFolder()).toBe("/authed/portals/test-portal/"); }); it("should handle the dev appMode", () => { window.localStorage.setItem(kClueDevIDKey, "random-id"); diff --git a/src/lib/firebase.ts b/src/lib/firebase.ts index 24e784b841..378c81d4ae 100644 --- a/src/lib/firebase.ts +++ b/src/lib/firebase.ts @@ -73,7 +73,9 @@ export class Firebase { parts.push(FIREBASE_ROOT_OVERRIDE); } else { parts.push(`${appMode}`); - parts.push(getRootId(this.db.stores, this.userId)); + if (appMode !== "authed") { + parts.push(getRootId(this.db.stores, this.userId)); + } } parts.push("portals"); parts.push(escapeKey(user.portal)); From 3dc759594789a2026c2936d35d3416e077275e8e Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 12 Sep 2024 15:57:19 -0400 Subject: [PATCH 21/36] Track shared model ID in persistent state to avoid sync issues. Add ID update function to fix tile copy issue. --- .../bar-graph/bar-graph-content.test.ts | 2 +- src/plugins/bar-graph/bar-graph-content.ts | 73 +++++++------------ .../bar-graph/bar-graph-registration.ts | 4 +- src/plugins/bar-graph/bar-graph-utils.ts | 13 ++++ src/plugins/bar-graph/category-pulldown.tsx | 2 +- src/plugins/bar-graph/legend-area.tsx | 11 +-- 6 files changed, 50 insertions(+), 55 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-content.test.ts b/src/plugins/bar-graph/bar-graph-content.test.ts index 3765a8c430..71d3548cda 100644 --- a/src/plugins/bar-graph/bar-graph-content.test.ts +++ b/src/plugins/bar-graph/bar-graph-content.test.ts @@ -29,7 +29,7 @@ function sharedSampleDataSet() { const TestableBarGraphContentModel = BarGraphContentModel .actions(self => ({ setDataSet(ds: SharedDataSetType) { - self.dataSet = ds; + // self.dataSet = ds; } })); diff --git a/src/plugins/bar-graph/bar-graph-content.ts b/src/plugins/bar-graph/bar-graph-content.ts index 51c08ce153..14d6f4b208 100644 --- a/src/plugins/bar-graph/bar-graph-content.ts +++ b/src/plugins/bar-graph/bar-graph-content.ts @@ -1,14 +1,12 @@ -import { reaction } from "mobx"; -import { types, Instance, addDisposer } from "mobx-state-tree"; +import { types, Instance } from "mobx-state-tree"; import { isNumber } from "lodash"; -import { SharedModelType } from "../../models/shared/shared-model"; import { ITileContentModel, TileContentModel } from "../../models/tiles/tile-content"; import { kBarGraphTileType, kBarGraphContentType } from "./bar-graph-types"; import { getSharedModelManager } from "../../models/tiles/tile-environment"; -import { isSharedDataSet, SharedDataSet, SharedDataSetType } from "../../models/shared/shared-data-set"; -import { ISharedModelManager } from "../../models/shared/shared-model-manager"; +import { SharedDataSet, SharedDataSetType } from "../../models/shared/shared-data-set"; import { clueDataColorInfo } from "../../utilities/color-utils"; import { displayValue } from "./bar-graph-utils"; +import { SharedModelType } from "../../models/shared/shared-model"; export function defaultBarGraphContent(): BarGraphContentModelType { return BarGraphContentModel.create({yAxisLabel: "Counts"}); @@ -19,29 +17,26 @@ export const BarGraphContentModel = TileContentModel .props({ type: types.optional(types.literal(kBarGraphTileType), kBarGraphTileType), yAxisLabel: "", + // Generally we fetch the dataset from SharedModelManager, but we track the ID here + // in persisted state as well so that remote read-only tiles will notice when it changes. + dataSetId: types.maybe(types.string), primaryAttribute: types.maybe(types.string), secondaryAttribute: types.maybe(types.string) }) - .volatile(self => ({ - dataSet: undefined as SharedDataSetType|undefined - })) .views(self => ({ + get sharedModel() { + const sharedModelManager = self.tileEnv?.sharedModelManager; + const firstSharedModel = sharedModelManager?.getTileSharedModelsByType(self, SharedDataSet)?.[0]; + if (!firstSharedModel) return undefined; + return firstSharedModel as SharedDataSetType; + }, get isUserResizable() { return true; - }, + } + })) + .views(self => ({ get cases() { - return self.dataSet?.dataSet.cases; - }, - // Query the SharedModelManager to find a connected DataSet - // An argument is passed in to avoid the return value being cached. - sharedModelDataSet(smm: ISharedModelManager|undefined) { - if (!smm || !smm.isReady) return; - const sharedDataSets = smm.getTileSharedModelsByType(self, SharedDataSet); - if (sharedDataSets.length > 0 && isSharedDataSet(sharedDataSets[0])) { - return sharedDataSets[0]; - } else { - return undefined; - } + return self.sharedModel?.dataSet.cases; } })) .views(self => ({ @@ -66,7 +61,7 @@ export const BarGraphContentModel = TileContentModel * Any empty values of attributes are replaced with "(no value)". */ get dataArray() { - const dataSet = self.dataSet?.dataSet; + const dataSet = self.sharedModel?.dataSet; const primary = self.primaryAttribute; const secondary = self.secondaryAttribute; const cases = self.cases; @@ -139,9 +134,6 @@ export const BarGraphContentModel = TileContentModel }, setSecondaryAttribute(attrId: string|undefined) { self.secondaryAttribute = attrId; - }, - setSharedDataSet() { - self.dataSet = self.sharedModelDataSet(getSharedModelManager(self)); } })) .actions(self => ({ @@ -152,40 +144,27 @@ export const BarGraphContentModel = TileContentModel for (const sharedDataSet of sharedDataSets) { smm.removeTileSharedModel(self, sharedDataSet); } - self.dataSet = undefined; - }, - afterAttach() { - // After attached to document & SMM is ready, cache a reference to our dataset. - addDisposer(self, reaction( - () => { - return self.tileEnv?.sharedModelManager?.isReady; - }, - (ready) => { - if (!ready) return; - self.setSharedDataSet(); - }, { fireImmediately: true } - )); }, + updateAfterSharedModelChanges(sharedModel?: SharedModelType) { - // When new dataset is attached, cache a reference to it and pick a category attribute. - const dataSet = self.sharedModelDataSet(getSharedModelManager(self)); - if (self.dataSet !== dataSet) { + // When new dataset is attached, store its ID and pick a primary attribute to display. + const dataSetId = self.sharedModel?.dataSet?.id; + if (self.dataSetId !== dataSetId) { + self.dataSetId = dataSetId; self.setPrimaryAttribute(undefined); self.setSecondaryAttribute(undefined); - self.dataSet = dataSet; - if (dataSet) { - const atts = dataSet.dataSet.attributes; + if (dataSetId) { + const atts = self.sharedModel.dataSet.attributes; if (atts.length > 0) { self.setPrimaryAttribute(atts[0].id); } } - } } - })); + } +})); export interface BarGraphContentModelType extends Instance {} - export function isBarGraphModel(model?: ITileContentModel): model is BarGraphContentModelType { return model?.type === kBarGraphTileType; } diff --git a/src/plugins/bar-graph/bar-graph-registration.ts b/src/plugins/bar-graph/bar-graph-registration.ts index 3de8812d01..0ba5e2384b 100644 --- a/src/plugins/bar-graph/bar-graph-registration.ts +++ b/src/plugins/bar-graph/bar-graph-registration.ts @@ -3,6 +3,7 @@ import { registerTileContentInfo } from "../../models/tiles/tile-content-info"; import { kBarGraphTileType, kBarGraphDefaultHeight } from "./bar-graph-types"; import { BarGraphComponent } from "./bar-graph-tile"; import { defaultBarGraphContent, BarGraphContentModel } from "./bar-graph-content"; +import { updateBarGraphContentWithNewSharedModelIds } from "./bar-graph-utils"; import Icon from "./assets/bar-graph-icon.svg"; @@ -11,7 +12,8 @@ registerTileContentInfo({ displayName: "Bar Graph", modelClass: BarGraphContentModel, defaultContent: defaultBarGraphContent, - defaultHeight: kBarGraphDefaultHeight + defaultHeight: kBarGraphDefaultHeight, + updateContentWithNewSharedModelIds: updateBarGraphContentWithNewSharedModelIds }); registerTileComponentInfo({ diff --git a/src/plugins/bar-graph/bar-graph-utils.ts b/src/plugins/bar-graph/bar-graph-utils.ts index bd7616abe3..e0a5c02e18 100644 --- a/src/plugins/bar-graph/bar-graph-utils.ts +++ b/src/plugins/bar-graph/bar-graph-utils.ts @@ -1,3 +1,8 @@ +import { SnapshotOut } from "mobx-state-tree"; +import { SharedModelEntrySnapshotType } from "../../models/document/shared-model-entry"; +import { replaceJsonStringsWithUpdatedIds, UpdatedSharedDataSetIds } from "../../models/shared/shared-data-set"; +import { BarGraphContentModel } from "./bar-graph-content"; + const kMissingValueString = "(no value)"; // Substitute "(no value)" for missing data @@ -19,3 +24,11 @@ export function getBBox(element: SVGGraphicsElement): DOMRect { export function roundTo5(n: number): number { return Math.max(5, Math.ceil(n/5)*5); } + +export function updateBarGraphContentWithNewSharedModelIds( + content: SnapshotOut, + sharedDataSetEntries: SharedModelEntrySnapshotType[], + updatedSharedModelMap: Record +) { + return replaceJsonStringsWithUpdatedIds(content, '"', ...Object.values(updatedSharedModelMap)); +} diff --git a/src/plugins/bar-graph/category-pulldown.tsx b/src/plugins/bar-graph/category-pulldown.tsx index df6c2548a9..67c5d1a045 100644 --- a/src/plugins/bar-graph/category-pulldown.tsx +++ b/src/plugins/bar-graph/category-pulldown.tsx @@ -18,7 +18,7 @@ export const CategoryPulldown = observer(function CategoryPulldown({setCategory, const readOnly = useReadOnlyContext(); const model = useBarGraphModelContext(); - const dataSet = model?.dataSet?.dataSet; + const dataSet = model?.sharedModel?.dataSet; const attributes = dataSet?.attributes || []; const current = (dataSet && model.primaryAttribute) ? dataSet.attrFromID(model.primaryAttribute)?.name diff --git a/src/plugins/bar-graph/legend-area.tsx b/src/plugins/bar-graph/legend-area.tsx index bd68d9eaf2..ccb9654d77 100644 --- a/src/plugins/bar-graph/legend-area.tsx +++ b/src/plugins/bar-graph/legend-area.tsx @@ -26,11 +26,12 @@ export const LegendArea = observer(function LegendArea ({legendRef}: IProps) { } } - if (!model || !model.dataSet || !model.primaryAttribute) { + if (!model || !model.sharedModel || !model.primaryAttribute) { return null; } - const dataSet = model.dataSet.dataSet; + const dataSet = model.sharedModel.dataSet; + const dataSetName = model.sharedModel.name; const allAttributes = dataSet?.attributes || []; const availableAttributes = allAttributes.filter((a) => a.id !== model.primaryAttribute); const currentPrimary = dataSet?.attrFromID(model.primaryAttribute); @@ -44,13 +45,13 @@ export const LegendArea = observer(function LegendArea ({legendRef}: IProps) {
Data from: - {model.dataSet.name} + {dataSetName}
@@ -79,7 +80,7 @@ export const LegendArea = observer(function LegendArea ({legendRef}: IProps) {
{currentSecondary ? secondaryKeys.map((key) => ) - : } + : }
From c3f02fc94619083e1346ee1183c7ade9d5bf120f Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Thu, 12 Sep 2024 16:08:10 -0400 Subject: [PATCH 22/36] Update Jest tests --- .../bar-graph/bar-graph-content.test.ts | 60 +++++++++++++------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-content.test.ts b/src/plugins/bar-graph/bar-graph-content.test.ts index 71d3548cda..565a7f56fc 100644 --- a/src/plugins/bar-graph/bar-graph-content.test.ts +++ b/src/plugins/bar-graph/bar-graph-content.test.ts @@ -26,10 +26,21 @@ function sharedSampleDataSet() { return SharedDataSet.create({ dataSet: sampleDataSet }); } -const TestableBarGraphContentModel = BarGraphContentModel +// This is a testable version of the BarGraphContentModel that doesn't rely on the shared model manager +// It just lets you set a SharedModel and returns that. +const TestingBarGraphContentModel = BarGraphContentModel + .volatile(() => ({ + storedSharedModel: undefined as SharedDataSetType | undefined + })) .actions(self => ({ - setDataSet(ds: SharedDataSetType) { - // self.dataSet = ds; + setSharedModel(sharedModel: SharedDataSetType) { + self.storedSharedModel = sharedModel; + self.updateAfterSharedModelChanges(sharedModel); + } + })) + .views(self => ({ + get sharedModel() { + return self.storedSharedModel; } })); @@ -44,6 +55,7 @@ describe("Bar Graph Content", () => { expect(content.yAxisLabel).toBe("Counts"); expect(getSnapshot(content)).toMatchInlineSnapshot(` Object { + "dataSetId": undefined, "primaryAttribute": undefined, "secondaryAttribute": undefined, "type": "BarGraph", @@ -78,20 +90,21 @@ Object { }); it("returns empty data array when there are no cases", () => { - const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedEmptyDataSet()); + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedEmptyDataSet()); expect(content.dataArray).toEqual([]); }); it("returns empty data array when there is no primary attribute", () => { - const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet()); + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); + content.setPrimaryAttribute(undefined); expect(content.dataArray).toEqual([]); }); it("returns expected data array with primary attribute", () => { - const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet()); + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); expect(content.dataArray).toEqual([ { "att-s": "cat", "value": 2 }, @@ -105,9 +118,18 @@ Object { ]); }); + it("sets first dataset attribute as the primary attribute by default", () => { + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "value": 2 }, + { "att-s": "owl","value": 2} + ]); + }); + it("returns expected data array with primary and secondary attributes", () => { - const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet()); + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); content.setSecondaryAttribute("att-l"); expect(content.dataArray).toEqual([ @@ -117,10 +139,10 @@ Object { }); it("fills in missing values with (no value)", () => { - const content = TestableBarGraphContentModel.create({ }); + const content = TestingBarGraphContentModel.create({ }); const dataSet = sharedSampleDataSet(); dataSet.dataSet?.attributes[1].setValue(3, undefined); // hide forest owl's location - content.setDataSet(dataSet); + content.setSharedModel(dataSet); content.setPrimaryAttribute("att-s"); content.setSecondaryAttribute("att-l"); expect(content.dataArray).toEqual([ @@ -138,23 +160,23 @@ Object { }); it("extracts primary keys", () => { - const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet()); + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); expect(content.primaryKeys).toEqual(["cat", "owl"]); }); it("extracts secondary keys", () => { - const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet()); + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); content.setSecondaryAttribute("att-l"); expect(content.secondaryKeys).toEqual(["yard", "forest"]); }); it("calculates the maximum data value", () => { - const content = TestableBarGraphContentModel.create({ }); - content.setDataSet(sharedSampleDataSet()); + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); expect(content.maxDataValue).toBe(2); From 5f09a104fc1c2ade4d9ebe01fb6a48081a956b0e Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 13 Sep 2024 11:56:36 -0400 Subject: [PATCH 23/36] Read only & sync & test improvements. Prevent changes in read-only mode. When changing primary attribute, reset secondary as part of the same action to support undo. Make EditableAxisLabel handle things more locally to fix sync bugs. Test read-only views Test undo and redo of each operation. Add classes in DocEditorApp to support testing. --- .../tile_tests/bar_graph_tile_spec.js | 240 +++++++++++++----- cypress/support/elements/tile/BarGraphTile.js | 72 +++--- .../support/elements/tile/TableToolTile.js | 8 +- src/components/doc-editor/doc-editor-app.tsx | 8 +- src/plugins/bar-graph/bar-graph-content.ts | 6 +- src/plugins/bar-graph/bar-graph-tile.tsx | 2 +- src/plugins/bar-graph/chart-area.tsx | 3 - src/plugins/bar-graph/editable-axis-label.tsx | 27 +- src/plugins/bar-graph/legend-area.tsx | 10 +- 9 files changed, 253 insertions(+), 123 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index 829558e4ae..19711e72cd 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -10,6 +10,8 @@ let clueCanvas = new ClueCanvas, // eslint-disable-next-line unused-imports/no-unused-vars const canvas = new Canvas; +const workspaces = ['.primary-workspace', '.read-only-local-workspace', '.read-only-remote-workspace']; + function textMatchesList(selector, expected) { selector.should('have.length', expected.length); selector.each(($el, index) => { @@ -28,59 +30,111 @@ context('Bar Graph Tile', function () { beforeTest(); clueCanvas.addTile('bargraph'); - barGraph.getTiles().should('have.length', 1); - barGraph.getTile() - .should('be.visible') - .and('have.class', 'bar-graph-tile') - .and('not.have.class', 'read-only'); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 1); + barGraph.getTile(workspace, 0) + .should('be.visible') + .and('have.class', 'bar-graph-tile'); + barGraph.getTileTitle(workspace).should("be.visible").and('have.text', 'Bar Graph 1'); + barGraph.getYAxisLabel(workspace).should('have.text', 'Counts'); + barGraph.getXAxisPulldownButton(workspace).should('have.text', 'Categories'); + } + barGraph.getTile(workspaces[0]).should('not.have.class', 'readonly'); + barGraph.getTile(workspaces[1]).should('have.class', 'readonly'); + barGraph.getTile(workspaces[2]).should('have.class', 'readonly'); - barGraph.getTileTitle().should("be.visible").and('have.text', 'Bar Graph 1'); - barGraph.getYAxisLabel().should('have.text', 'Counts'); - barGraph.getXAxisPulldownButton(0).should('have.text', 'Categories'); + // Undo/redo tile creation + clueCanvas.getUndoTool().click(); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 0); + } + clueCanvas.getRedoTool().click(); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 1); + } cy.log('Change Y axis label'); barGraph.getYAxisLabelEditor().should('not.exist'); barGraph.getYAxisLabelButton().click(); barGraph.getYAxisLabelEditor().should('be.visible').type(' of something{enter}'); barGraph.getYAxisLabelEditor().should('not.exist'); + for (const workspace of workspaces) { + barGraph.getYAxisLabel(workspace).should('have.text', 'Counts of something'); + } + + // Undo/redo label change + clueCanvas.getUndoTool().click(); + barGraph.getYAxisLabel().should('have.text', 'Counts'); + clueCanvas.getRedoTool().click(); barGraph.getYAxisLabel().should('have.text', 'Counts of something'); cy.log('Duplicate tile'); clueCanvas.getDuplicateTool().click(); - barGraph.getTiles().should('have.length', 2); - barGraph.getTile(0) - .should('be.visible') - .and('have.class', 'bar-graph-tile') - .and('not.have.class', 'read-only'); - barGraph.getTileTitle(0).should("be.visible").and('have.text', 'Bar Graph 1'); - barGraph.getYAxisLabel(0).should('have.text', 'Counts of something'); - barGraph.getXAxisPulldownButton(0).should('have.text', 'Categories'); - - barGraph.getTile(1) - .should('be.visible') - .and('have.class', 'bar-graph-tile') - .and('not.have.class', 'read-only'); - barGraph.getTileTitle(1).should("be.visible").and('have.text', 'Bar Graph 2'); - barGraph.getYAxisLabel(1).should('have.text', 'Counts of something'); - barGraph.getXAxisPulldownButton(1).should('have.text', 'Categories'); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 2); + barGraph.getTile(workspace, 0) + .should('be.visible') + .and('have.class', 'bar-graph-tile'); + barGraph.getTileTitle(workspace, 0).should("be.visible").and('have.text', 'Bar Graph 1'); + barGraph.getYAxisLabel(workspace, 0).should('have.text', 'Counts of something'); + barGraph.getXAxisPulldownButton(workspace, 0).should('have.text', 'Categories'); + + barGraph.getTile(workspace, 1) + .should('be.visible') + .and('have.class', 'bar-graph-tile'); + barGraph.getTileTitle(workspace, 1).should("be.visible").and('have.text', 'Bar Graph 2'); + barGraph.getYAxisLabel(workspace, 1).should('have.text', 'Counts of something'); + barGraph.getXAxisPulldownButton(workspace, 1).should('have.text', 'Categories'); + } + + // Undo/redo tile duplication + clueCanvas.getUndoTool().click(); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 1); + } + clueCanvas.getRedoTool().click(); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 2); + } cy.log('Delete tile'); clueCanvas.deleteTile('bargraph'); clueCanvas.deleteTile('bargraph'); - barGraph.getTiles().should('have.length', 0); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 0); + } + + // Undo/redo tile deletion + clueCanvas.getUndoTool().click(); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 1); + } + clueCanvas.getUndoTool().click(); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 2); + } + clueCanvas.getRedoTool().click(); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 1); + } + clueCanvas.getRedoTool().click(); + for (const workspace of workspaces) { + barGraph.getTiles(workspace).should('have.length', 0); + } }); it('Can link data ', function () { beforeTest(); clueCanvas.addTile('bargraph'); - barGraph.getTiles().click(); - barGraph.getYAxisLabel().should('have.text', 'Counts'); - barGraph.getXAxisPulldown().should('have.text', 'Categories'); - barGraph.getYAxisTickLabel().should('not.exist'); - barGraph.getXAxisTickLabel().should('not.exist'); - barGraph.getLegendArea().should('not.exist'); - barGraph.getBar().should('not.exist'); + for (const workspace of workspaces) { + barGraph.getYAxisLabel(workspace).should('have.text', 'Counts'); + barGraph.getXAxisPulldown(workspace).should('have.text', 'Categories'); + barGraph.getYAxisTickLabel(workspace).should('not.exist'); + barGraph.getXAxisTickLabel(workspace).should('not.exist'); + barGraph.getLegendArea(workspace).should('not.exist'); + barGraph.getBar(workspace).should('not.exist'); + } // Table dataset for testing: // 4 instances of X / Y / Z @@ -103,55 +157,121 @@ context('Bar Graph Tile', function () { cy.get('select').select('Table Data 1'); cy.get('.modal-button').contains("Graph It!").click(); - barGraph.getXAxisPulldown().should('have.text', 'x'); + for (const workspace of workspaces) { + barGraph.getXAxisPulldown(workspace).should('have.text', 'x'); + textMatchesList(barGraph.getXAxisTickLabel(workspace), ['X', 'XX']); + textMatchesList(barGraph.getYAxisTickLabel(workspace), ['0', '1', '2', '3', '4', '5']); + barGraph.getBar(workspace).should('have.length', 2); + barGraph.getDatasetLabel(workspace).should('have.text', 'Table Data 1'); + barGraph.getSortByMenuButton(workspace).should('have.text', 'None'); + barGraph.getSecondaryValueName(workspace).should('have.length', 1).and('have.text', 'x'); + } - textMatchesList(barGraph.getXAxisTickLabel(), ['X', 'XX']); - textMatchesList(barGraph.getYAxisTickLabel(), ['0', '1', '2', '3', '4', '5']); - barGraph.getBar().should('have.length', 2); - barGraph.getDatasetLabel().should('have.text', 'Table Data 1'); - barGraph.getSortByMenuButton().should('have.text', 'None'); - barGraph.getSecondaryValueName().should('have.length', 1).and('have.text', 'x'); + // Undo/redo linking + clueCanvas.getUndoTool().click(); + for (const workspace of workspaces) { + barGraph.getXAxisPulldown(workspace).should('have.text', 'Categories'); + barGraph.getLegendArea(workspace).should('not.exist'); + barGraph.getBar(workspace).should('not.exist'); + } + + clueCanvas.getRedoTool().click(); + for (const workspace of workspaces) { + barGraph.getDatasetLabel(workspace).should('have.text', 'Table Data 1'); + barGraph.getXAxisPulldown(workspace).should('have.text', 'x'); + barGraph.getBar(workspace).should('have.length', 2); + } cy.log('Legend should move to bottom when tile is narrow'); barGraph.getTileContent().should('have.class', 'horizontal').and('not.have.class', 'vertical'); clueCanvas.addTileByDrag('table', 'right'); clueCanvas.addTileByDrag('table', 'right'); - barGraph.getTileContent().should('have.class', 'vertical').and('not.have.class', 'horizontal'); + for (const workspace of workspaces) { + barGraph.getTileContent(workspace).should('have.class', 'vertical').and('not.have.class', 'horizontal'); + } clueCanvas.getUndoTool().click(); // undo add table clueCanvas.getUndoTool().click(); // undo add table - tableTile.getTableTile().should('have.length', 1); - barGraph.getTileContent().should('have.class', 'horizontal').and('not.have.class', 'vertical'); + for (const workspace of workspaces) { + tableTile.getTableTile(workspace).should('have.length', 1); + barGraph.getTileContent(workspace).should('have.class', 'horizontal').and('not.have.class', 'vertical'); + } cy.log('Change Sort By'); + barGraph.getSortByMenuButton().should('have.text', 'None'); barGraph.getSortByMenuButton().click(); barGraph.getChakraMenuItem().should('have.length', 3); barGraph.getChakraMenuItem().eq(1).should('have.text', 'y').click(); - textMatchesList(barGraph.getXAxisTickLabel(), ['X', 'XX']); - textMatchesList(barGraph.getYAxisTickLabel(), ['0', '1', '2', '3', '4', '5']); - barGraph.getBar().should('have.length', 3); - barGraph.getDatasetLabel().should('have.text', 'Table Data 1'); - barGraph.getSortByMenuButton().should('have.text', 'y'); - textMatchesList(barGraph.getSecondaryValueName(), ['Y', 'YY']); + for (const workspace of workspaces) { + textMatchesList(barGraph.getXAxisTickLabel(workspace), ['X', 'XX']); + textMatchesList(barGraph.getYAxisTickLabel(workspace), ['0', '1', '2', '3', '4', '5']); + barGraph.getBar(workspace).should('have.length', 3); + barGraph.getDatasetLabel(workspace).should('have.text', 'Table Data 1'); + barGraph.getSortByMenuButton(workspace).should('have.text', 'y'); + textMatchesList(barGraph.getSecondaryValueName(workspace), ['Y', 'YY']); + } + + // Undo-redo sort by + clueCanvas.getUndoTool().click(); + for (const workspace of workspaces) { + barGraph.getSortByMenuButton(workspace).should('have.text', 'None'); + barGraph.getBar(workspace).should('have.length', 2); + barGraph.getSecondaryValueName(workspace).should('have.text', 'x'); + } + clueCanvas.getRedoTool().click(); + for (const workspace of workspaces) { + barGraph.getSortByMenuButton(workspace).should('have.text', 'y'); + textMatchesList(barGraph.getSecondaryValueName(workspace), ['Y', 'YY']); + barGraph.getBar(workspace).should('have.length', 3); + } cy.log('Change Category'); barGraph.getXAxisPulldownButton().click(); barGraph.getChakraMenuItem().should('have.length', 3); barGraph.getChakraMenuItem().eq(1).should('have.text', 'y').click(); - barGraph.getXAxisPulldown().should('have.text', 'y'); - textMatchesList(barGraph.getXAxisTickLabel(), ['Y', 'YY']); - textMatchesList(barGraph.getYAxisTickLabel(), ['0', '2', '4', '6', '8', '10']); // there are 6 Ys in this view so scale expands. - barGraph.getBar().should('have.length', 2); - barGraph.getDatasetLabel().should('have.text', 'Table Data 1'); - barGraph.getSortByMenuButton().should('have.text', 'None'); - barGraph.getSecondaryValueName().should('have.length', 1).and('have.text', 'y'); + for (const workspace of workspaces) { + barGraph.getXAxisPulldown(workspace).should('have.text', 'y'); + textMatchesList(barGraph.getXAxisTickLabel(workspace), ['Y', 'YY']); + textMatchesList(barGraph.getYAxisTickLabel(workspace), ['0', '2', '4', '6', '8', '10']); // there are 6 Ys in this view so scale expands. + barGraph.getBar(workspace).should('have.length', 2); + barGraph.getDatasetLabel(workspace).should('have.text', 'Table Data 1'); + barGraph.getSortByMenuButton(workspace).should('have.text', 'None'); + barGraph.getSecondaryValueName(workspace).should('have.length', 1).and('have.text', 'y'); + } + + // Undo-redo category change + clueCanvas.getUndoTool().click(); + for (const workspace of workspaces) { + barGraph.getXAxisPulldown(workspace).should('have.text', 'x'); + textMatchesList(barGraph.getXAxisTickLabel(workspace), ['X', 'XX']); + } + clueCanvas.getRedoTool().click(); + for (const workspace of workspaces) { + barGraph.getXAxisPulldown(workspace).should('have.text', 'y'); + textMatchesList(barGraph.getXAxisTickLabel(workspace), ['Y', 'YY']); + } cy.log('Unlink data'); barGraph.getDatasetUnlinkButton().click(); - barGraph.getXAxisPulldown().should('have.text', 'Categories'); - barGraph.getYAxisTickLabel().should('not.exist'); - barGraph.getXAxisTickLabel().should('not.exist'); - barGraph.getLegendArea().should('not.exist'); - barGraph.getBar().should('not.exist'); + for (const workspace of workspaces) { + barGraph.getXAxisPulldown(workspace).should('have.text', 'Categories'); + barGraph.getYAxisTickLabel(workspace).should('not.exist'); + barGraph.getXAxisTickLabel(workspace).should('not.exist'); + barGraph.getLegendArea(workspace).should('not.exist'); + barGraph.getBar(workspace).should('not.exist'); + } + + // Undo-redo unlink + clueCanvas.getUndoTool().click(); + for (const workspace of workspaces) { + barGraph.getXAxisPulldown().should('have.text', 'y'); + textMatchesList(barGraph.getXAxisTickLabel(workspace), ['Y', 'YY']); + barGraph.getBar(workspace).should('have.length', 2); + } + clueCanvas.getRedoTool().click(); + for (const workspace of workspaces) { + barGraph.getXAxisPulldown(workspace).should('have.text', 'Categories'); + barGraph.getBar(workspace).should('not.exist'); + } }); }); diff --git a/cypress/support/elements/tile/BarGraphTile.js b/cypress/support/elements/tile/BarGraphTile.js index 5fe641923f..d19a3b1d9e 100644 --- a/cypress/support/elements/tile/BarGraphTile.js +++ b/cypress/support/elements/tile/BarGraphTile.js @@ -1,79 +1,79 @@ class BarGraphTile { getTiles(workspaceClass) { - return cy.get(`${workspaceClass || ".primary-workspace"} .canvas-area .bar-graph-tile`); + return cy.get(`${workspaceClass || ".primary-workspace"} .canvas .bar-graph-tile`); } - getTile(tileIndex = 0, workspaceClass) { - return this.getTiles().eq(tileIndex); + getTile(workspaceClass, tileIndex = 0) { + return this.getTiles(workspaceClass).eq(tileIndex); } - getTileTitle(tileIndex = 0, workspaceClass) { - return this.getTile(tileIndex, workspaceClass).find(`.editable-tile-title-text`); + getTileTitle(workspaceClass, tileIndex = 0) { + return this.getTile(workspaceClass, tileIndex).find(`.editable-tile-title-text`); } - getTileContent(tileIndex = 0, workspaceClass) { - return this.getTile(tileIndex, workspaceClass).find(`[data-testid="bar-graph-content"]`); + getTileContent(workspaceClass, tileIndex = 0) { + return this.getTile(workspaceClass, tileIndex).find(`[data-testid="bar-graph-content"]`); } - getChakraMenuItem(tileIndex = 0, workspaceClass) { + getChakraMenuItem(workspaceClass, tileIndex = 0) { return cy.get(`body .chakra-portal button`).filter(':visible'); } - getChartArea(tileIndex = 0, workspaceClass) { - return this.getTile(tileIndex, workspaceClass).find(`svg.bar-graph-svg`); + getChartArea(workspaceClass, tileIndex = 0) { + return this.getTile(workspaceClass, tileIndex).find(`svg.bar-graph-svg`); } - getYAxisLabel(tileIndex = 0, workspaceClass) { - return this.getChartArea(tileIndex, workspaceClass).find(`.editable-axis-label`); + getYAxisLabel(workspaceClass, tileIndex = 0) { + return this.getChartArea(workspaceClass, tileIndex).find(`.editable-axis-label`); } - getYAxisLabelButton(tileIndex = 0, workspaceClass) { - return this.getChartArea(tileIndex, workspaceClass).find(`[data-testid="axis-label-button"]`); + getYAxisLabelButton(workspaceClass, tileIndex = 0) { + return this.getChartArea(workspaceClass, tileIndex).find(`[data-testid="axis-label-button"]`); } - getYAxisLabelEditor(tileIndex = 0, workspaceClass) { - return this.getChartArea(tileIndex, workspaceClass).find(`[data-testid="axis-label-editor"] input`); + getYAxisLabelEditor(workspaceClass, tileIndex = 0) { + return this.getChartArea(workspaceClass, tileIndex).find(`[data-testid="axis-label-editor"] input`); } - getXAxisPulldown(tileIndex = 0, workspaceClass) { - return this.getChartArea(tileIndex, workspaceClass).find(`[data-testid="category-pulldown"]`); + getXAxisPulldown(workspaceClass, tileIndex = 0) { + return this.getChartArea(workspaceClass, tileIndex).find(`[data-testid="category-pulldown"]`); } - getXAxisPulldownButton(tileIndex = 0, workspaceClass) { - return this.getXAxisPulldown(tileIndex, workspaceClass).find(`button`); + getXAxisPulldownButton(workspaceClass, tileIndex = 0) { + return this.getXAxisPulldown(workspaceClass, tileIndex).find(`button`); } - getYAxisTickLabel(tileIndex = 0, workspaceClass) { - return this.getChartArea(tileIndex, workspaceClass).find(`.visx-axis-left text`); + getYAxisTickLabel(workspaceClass, tileIndex = 0) { + return this.getChartArea(workspaceClass, tileIndex).find(`.visx-axis-left text`); } - getXAxisTickLabel(tileIndex = 0, workspaceClass) { - return this.getChartArea(tileIndex, workspaceClass).find(`.visx-axis-bottom text`); + getXAxisTickLabel(workspaceClass, tileIndex = 0) { + return this.getChartArea(workspaceClass, tileIndex).find(`.visx-axis-bottom text`); } - getBar(tileIndex = 0, workspaceClass) { - return this.getChartArea(tileIndex, workspaceClass).find(`.visx-bar`); + getBar(workspaceClass, tileIndex = 0) { + return this.getChartArea(workspaceClass, tileIndex).find(`.visx-bar`); } - getLegendArea(tileIndex = 0, workspaceClass) { - return this.getTile(tileIndex, workspaceClass).find(`.bar-graph-legend`); + getLegendArea(workspaceClass, tileIndex = 0) { + return this.getTile(workspaceClass, tileIndex).find(`.bar-graph-legend`); } - getDatasetLabel(tileIndex = 0, workspaceClass) { - return this.getLegendArea(tileIndex, workspaceClass).find(`.dataset-header .dataset-name`); + getDatasetLabel(workspaceClass, tileIndex = 0) { + return this.getLegendArea(workspaceClass, tileIndex).find(`.dataset-header .dataset-name`); } - getDatasetUnlinkButton(tileIndex = 0, workspaceClass) { - return this.getLegendArea(tileIndex, workspaceClass).find(`.dataset-header .dataset-icon a`); + getDatasetUnlinkButton(workspaceClass, tileIndex = 0) { + return this.getLegendArea(workspaceClass, tileIndex).find(`.dataset-header .dataset-icon a`); } - getSortByMenuButton(tileIndex = 0, workspaceClass) { - return this.getLegendArea(tileIndex, workspaceClass).find(`.sort-by button.chakra-menu__menu-button`); + getSortByMenuButton(workspaceClass, tileIndex = 0) { + return this.getLegendArea(workspaceClass, tileIndex).find(`.sort-by button.chakra-menu__menu-button`); } - getSecondaryValueName(tileIndex = 0, workspaceClass) { - return this.getLegendArea(tileIndex, workspaceClass).find(`.secondary-values .secondary-value-name`); + getSecondaryValueName(workspaceClass, tileIndex = 0) { + return this.getLegendArea(workspaceClass, tileIndex).find(`.secondary-values .secondary-value-name`); } } diff --git a/cypress/support/elements/tile/TableToolTile.js b/cypress/support/elements/tile/TableToolTile.js index 5fa38207e9..737f146044 100644 --- a/cypress/support/elements/tile/TableToolTile.js +++ b/cypress/support/elements/tile/TableToolTile.js @@ -4,10 +4,10 @@ function wsclass(workspaceClass) { class TableToolTile{ getTableTile(workspaceClass) { - return cy.get(`${wsclass(workspaceClass)} .canvas-area .table-tool`); + return cy.get(`${wsclass(workspaceClass)} .canvas .table-tool`); } getTableTitle(workspaceClass){ - return cy.get(`${wsclass(workspaceClass)} .canvas-area .table-title`); + return cy.get(`${wsclass(workspaceClass)} .canvas .table-title`); } getAddColumnButton(){ return cy.get('.add-column-button'); @@ -39,7 +39,7 @@ class TableToolTile{ this.getRemoveRowButton().click(); } getTableRow(){ - return cy.get('.canvas-area .rdg-row'); + return cy.get('.canvas .rdg-row'); } getColumnHeaderText(i){ return cy.get('.column-header-cell .editable-header-cell .header-name').text(); @@ -89,7 +89,7 @@ class TableToolTile{ this.getTableCell().eq(cell).type(num+'{enter}'); } getTableIndexColumnCell(){ - return cy.get('.canvas-area .rdg-cell.index-column'); + return cy.get('.canvas .rdg-cell.index-column'); } // Fill in a table tile with the given data (a list of lists) // Table tile should in the default state (2 columns, no rows) diff --git a/src/components/doc-editor/doc-editor-app.tsx b/src/components/doc-editor/doc-editor-app.tsx index 2edc3c5f6f..0d47d2509a 100644 --- a/src/components/doc-editor/doc-editor-app.tsx +++ b/src/components/doc-editor/doc-editor-app.tsx @@ -228,13 +228,13 @@ export const DocEditorApp = observer(function DocEditorApp() { { showLocalReadOnly && <>
Read Only Local
- + } { showRemoteReadOnly && <>
Read Only Remote (emulated)
- + }
@@ -243,7 +243,7 @@ export const DocEditorApp = observer(function DocEditorApp() { ); }); -const ReadonlyCanvas = ({document}:{document: DocumentModelType}) => { +const ReadonlyCanvas = ({document, className}:{document: DocumentModelType, className: string}) => { const readOnlyScale = 0.5; const scaledStyle = { position: "absolute", @@ -254,7 +254,7 @@ const ReadonlyCanvas = ({document}:{document: DocumentModelType}) => { } as const; return ( -
+
= observer((props: ITilePro
diff --git a/src/plugins/bar-graph/chart-area.tsx b/src/plugins/bar-graph/chart-area.tsx index 5b20193d4e..e0b37d3b1e 100644 --- a/src/plugins/bar-graph/chart-area.tsx +++ b/src/plugins/bar-graph/chart-area.tsx @@ -38,7 +38,6 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro function setPrimaryAttribute(id: string) { model?.setPrimaryAttribute(id); - model?.setSecondaryAttribute(undefined); } function barColor(key: string) { @@ -179,8 +178,6 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro model?.setYAxisLabel(text)} /> void; } -const EditableAxisLabel: React.FC = ({text, x, y, setText}) => { - +const EditableAxisLabel: React.FC = observer(function EditableAxisLabel({x, y}) { + const model = useBarGraphModelContext(); const readOnly = useReadOnlyContext(); const textRef = React.useRef(null); const [boundingBox, setBoundingBox] = React.useState(null); const [editing, setEditing] = React.useState(false); - const [displayText, setDisplayText] = React.useState(text || "Y axis"); - const [editText, setEditText] = React.useState(text || "Y axis"); + const [editText, setEditText] = React.useState(""); + + const displayText = model?.yAxisLabel || ""; useEffect(() => { if (textRef.current) { @@ -28,12 +29,18 @@ const EditableAxisLabel: React.FC = ({text, x, y, setText}) => { } }, [x, y, displayText, textRef]); + const handleStartEdit = () => { + if (!readOnly) { + setEditText(displayText); + setEditing(true); + } + }; + const handleClose = (accept: boolean) => { setEditing(false); if (accept && editText) { const trimmed = editText.trim(); - setDisplayText(trimmed); - setText(trimmed); + model?.setYAxisLabel(trimmed); } }; @@ -81,7 +88,7 @@ const EditableAxisLabel: React.FC = ({text, x, y, setText}) => { strokeWidth={1.5} fill="none" pointerEvents={editing ? "none" : "all"} - onClick={() => { if (!readOnly) setEditing(true); }} + onClick={handleStartEdit} />} = ({text, x, y, setText}) => { ); -}; +}); export default EditableAxisLabel; diff --git a/src/plugins/bar-graph/legend-area.tsx b/src/plugins/bar-graph/legend-area.tsx index ccb9654d77..f0e32dab5e 100644 --- a/src/plugins/bar-graph/legend-area.tsx +++ b/src/plugins/bar-graph/legend-area.tsx @@ -6,6 +6,7 @@ import { LegendSecondaryRow } from './legend-secondary-row'; import RemoveDataIcon from "../../assets/remove-data-icon.svg"; import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; +import { useReadOnlyContext } from '../../components/document/read-only-context'; interface IProps { legendRef: React.RefObject; @@ -13,9 +14,10 @@ interface IProps { export const LegendArea = observer(function LegendArea ({legendRef}: IProps) { const model = useBarGraphModelContext(); + const readOnly = useReadOnlyContext(); function unlinkDataset() { - if (model) { + if (!readOnly && model) { model.unlinkDataSet(); } } @@ -68,9 +70,11 @@ export const LegendArea = observer(function LegendArea ({legendRef}: IProps) { - setSecondaryAttribute(undefined)}>None + setSecondaryAttribute(undefined)}>None {availableAttributes.map((a) => ( - setSecondaryAttribute(a.id)}>{a.name} + setSecondaryAttribute(a.id)}> + {a.name} + ))} From 9ec6bc0d0875ec664e1bdefc0dafd34a8d0098cc Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 13 Sep 2024 14:50:17 -0400 Subject: [PATCH 24/36] Logging and logging tests --- .../tile_tests/bar_graph_tile_spec.js | 31 +++++++++++++++++++ src/lib/logger-types.ts | 1 + src/plugins/bar-graph/bar-graph-utils.ts | 23 +++++++++++++- src/plugins/bar-graph/chart-area.tsx | 7 +++-- src/plugins/bar-graph/editable-axis-label.tsx | 15 ++++----- src/plugins/bar-graph/legend-area.tsx | 14 ++++++++- 6 files changed, 80 insertions(+), 11 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index 19711e72cd..6fe0db9393 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -3,6 +3,9 @@ import Canvas from '../../../support/elements/common/Canvas'; import BarGraphTile from '../../../support/elements/tile/BarGraphTile'; import TableToolTile from '../../../support/elements/tile/TableToolTile'; +import { LogEventName } from "../../../../src/lib/logger-types"; + + let clueCanvas = new ClueCanvas, barGraph = new BarGraphTile, tableTile = new TableToolTile; @@ -22,6 +25,9 @@ function textMatchesList(selector, expected) { function beforeTest() { const url = "/editor/?appMode=qa&unit=./demo/units/qa/content.json"; cy.visit(url); + cy.window().then(win => { + cy.stub(win.ccLogger, "log").as("log"); + }); } context('Bar Graph Tile', function () { @@ -43,6 +49,10 @@ context('Bar Graph Tile', function () { barGraph.getTile(workspaces[1]).should('have.class', 'readonly'); barGraph.getTile(workspaces[2]).should('have.class', 'readonly'); + cy.get("@log") + .should("have.been.been.calledWith", LogEventName.CREATE_TILE, Cypress.sinon.match.object) + .its("firstCall.args.1").should("deep.include", { objectType: "BarGraph" }); + // Undo/redo tile creation clueCanvas.getUndoTool().click(); for (const workspace of workspaces) { @@ -62,6 +72,9 @@ context('Bar Graph Tile', function () { barGraph.getYAxisLabel(workspace).should('have.text', 'Counts of something'); } + cy.get("@log").its("lastCall.args.0").should("equal", LogEventName.BARGRAPH_TOOL_CHANGE); + cy.get("@log").its("lastCall.args.1").should("deep.include", { operation: "setYAxisLabel", text: "Counts of something" }); + // Undo/redo label change clueCanvas.getUndoTool().click(); barGraph.getYAxisLabel().should('have.text', 'Counts'); @@ -87,6 +100,9 @@ context('Bar Graph Tile', function () { barGraph.getXAxisPulldownButton(workspace, 1).should('have.text', 'Categories'); } + cy.get("@log").its("lastCall.args.0").should("equal", LogEventName.COPY_TILE); + cy.get("@log").its("lastCall.args.1").should("deep.include", { objectType: "BarGraph" }); + // Undo/redo tile duplication clueCanvas.getUndoTool().click(); for (const workspace of workspaces) { @@ -104,6 +120,9 @@ context('Bar Graph Tile', function () { barGraph.getTiles(workspace).should('have.length', 0); } + cy.get("@log").its("lastCall.args.0").should("equal", LogEventName.DELETE_TILE); + cy.get("@log").its("lastCall.args.1").should("deep.include", { objectType: "BarGraph" }); + // Undo/redo tile deletion clueCanvas.getUndoTool().click(); for (const workspace of workspaces) { @@ -167,6 +186,9 @@ context('Bar Graph Tile', function () { barGraph.getSecondaryValueName(workspace).should('have.length', 1).and('have.text', 'x'); } + cy.get("@log").its("lastCall.args.0").should("equal", LogEventName.TILE_LINK); + cy.get("@log").its("lastCall.args.1").should("nested.include", { "sourceTile.type": "BarGraph", "sharedModel.type": "SharedDataSet" }); + // Undo/redo linking clueCanvas.getUndoTool().click(); for (const workspace of workspaces) { @@ -210,6 +232,9 @@ context('Bar Graph Tile', function () { textMatchesList(barGraph.getSecondaryValueName(workspace), ['Y', 'YY']); } + cy.get("@log").its("lastCall.args.0").should("equal", LogEventName.BARGRAPH_TOOL_CHANGE); + cy.get("@log").its("lastCall.args.1").should("deep.include", { operation: "setSecondaryAttribute" }); + // Undo-redo sort by clueCanvas.getUndoTool().click(); for (const workspace of workspaces) { @@ -238,6 +263,9 @@ context('Bar Graph Tile', function () { barGraph.getSecondaryValueName(workspace).should('have.length', 1).and('have.text', 'y'); } + cy.get("@log").its("lastCall.args.0").should("equal", LogEventName.BARGRAPH_TOOL_CHANGE); + cy.get("@log").its("lastCall.args.1").should("deep.include", { operation: "setPrimaryAttribute" }); + // Undo-redo category change clueCanvas.getUndoTool().click(); for (const workspace of workspaces) { @@ -260,6 +288,9 @@ context('Bar Graph Tile', function () { barGraph.getBar(workspace).should('not.exist'); } + cy.get("@log").its("lastCall.args.0").should("equal", LogEventName.TILE_UNLINK); + cy.get("@log").its("lastCall.args.1").should("nested.include", { "sourceTile.type": "BarGraph", "sharedModel.type": "SharedDataSet" }); + // Undo-redo unlink clueCanvas.getUndoTool().click(); for (const workspace of workspaces) { diff --git a/src/lib/logger-types.ts b/src/lib/logger-types.ts index a566aa6b7c..a0d77d3a06 100644 --- a/src/lib/logger-types.ts +++ b/src/lib/logger-types.ts @@ -32,6 +32,7 @@ export enum LogEventName { HIDE_SOLUTIONS, SHOW_SOLUTIONS, + BARGRAPH_TOOL_CHANGE, GEOMETRY_TOOL_CHANGE, DRAWING_TOOL_CHANGE, TABLE_TOOL_CHANGE, diff --git a/src/plugins/bar-graph/bar-graph-utils.ts b/src/plugins/bar-graph/bar-graph-utils.ts index e0a5c02e18..e5a1d86810 100644 --- a/src/plugins/bar-graph/bar-graph-utils.ts +++ b/src/plugins/bar-graph/bar-graph-utils.ts @@ -1,7 +1,10 @@ import { SnapshotOut } from "mobx-state-tree"; +import { LogEventName } from "../../lib/logger-types"; import { SharedModelEntrySnapshotType } from "../../models/document/shared-model-entry"; import { replaceJsonStringsWithUpdatedIds, UpdatedSharedDataSetIds } from "../../models/shared/shared-data-set"; -import { BarGraphContentModel } from "./bar-graph-content"; +import { logTileChangeEvent } from "../../models/tiles/log/log-tile-change-event"; +import { getTileIdFromContent } from "../../models/tiles/tile-model"; +import { BarGraphContentModel, BarGraphContentModelType } from "./bar-graph-content"; const kMissingValueString = "(no value)"; @@ -32,3 +35,21 @@ export function updateBarGraphContentWithNewSharedModelIds( ) { return replaceJsonStringsWithUpdatedIds(content, '"', ...Object.values(updatedSharedModelMap)); } + +// Define types here to document all possible values that this tile logs +type LoggableOperation = "setPrimaryAttribute" | "setSecondaryAttribute" | "setYAxisLabel"; +type LoggableChange = { + attributeId?: string; + text?: string; +}; + +export function logBarGraphEvent( + model: BarGraphContentModelType, operation: LoggableOperation, change: LoggableChange) { + const tileId = getTileIdFromContent(model) || ""; + + logTileChangeEvent(LogEventName.BARGRAPH_TOOL_CHANGE, { + tileId, + operation, + change + }); +} diff --git a/src/plugins/bar-graph/chart-area.tsx b/src/plugins/bar-graph/chart-area.tsx index e0b37d3b1e..f250997188 100644 --- a/src/plugins/bar-graph/chart-area.tsx +++ b/src/plugins/bar-graph/chart-area.tsx @@ -8,7 +8,7 @@ import { Bar, BarGroup } from "@visx/shape"; import { useBarGraphModelContext } from "./bar-graph-content-context"; import { CategoryPulldown } from "./category-pulldown"; import EditableAxisLabel from "./editable-axis-label"; -import { roundTo5 } from "./bar-graph-utils"; +import { logBarGraphEvent, roundTo5 } from "./bar-graph-utils"; const margin = { top: 7, @@ -37,7 +37,10 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro const yMax = height - margin.top - margin.bottom; function setPrimaryAttribute(id: string) { - model?.setPrimaryAttribute(id); + if (model) { + model.setPrimaryAttribute(id); + logBarGraphEvent(model, "setPrimaryAttribute", { attributeId: id }); + } } function barColor(key: string) { diff --git a/src/plugins/bar-graph/editable-axis-label.tsx b/src/plugins/bar-graph/editable-axis-label.tsx index fe6d62a0e1..6af8758e31 100644 --- a/src/plugins/bar-graph/editable-axis-label.tsx +++ b/src/plugins/bar-graph/editable-axis-label.tsx @@ -1,7 +1,7 @@ import React, { useEffect } from 'react'; import { observer } from 'mobx-react'; import { Text } from '@visx/text'; -import { getBBox } from './bar-graph-utils'; +import { getBBox, logBarGraphEvent } from './bar-graph-utils'; import { useReadOnlyContext } from '../../components/document/read-only-context'; import { useBarGraphModelContext } from './bar-graph-content-context'; @@ -36,11 +36,12 @@ const EditableAxisLabel: React.FC = observer(function EditableAxisLabel( } }; - const handleClose = (accept: boolean) => { + const handleEndEdit = (accept: boolean) => { setEditing(false); - if (accept && editText) { + if (model && accept && editText) { const trimmed = editText.trim(); - model?.setYAxisLabel(trimmed); + model.setYAxisLabel(trimmed); + logBarGraphEvent(model, "setYAxisLabel", { text: trimmed }); } }; @@ -48,11 +49,11 @@ const EditableAxisLabel: React.FC = observer(function EditableAxisLabel( const { key } = e; switch (key) { case "Escape": - handleClose(false); + handleEndEdit(false); break; case "Enter": case "Tab": - handleClose(true); + handleEndEdit(true); break; } }; @@ -66,7 +67,7 @@ const EditableAxisLabel: React.FC = observer(function EditableAxisLabel( value={editText} size={editText.length + 5} onKeyDown={handleKeyDown} - onBlur={() => handleClose(true)} + onBlur={() => handleEndEdit(true)} onChange={(e) => setEditText(e.target.value)} /> diff --git a/src/plugins/bar-graph/legend-area.tsx b/src/plugins/bar-graph/legend-area.tsx index f0e32dab5e..7625c98e92 100644 --- a/src/plugins/bar-graph/legend-area.tsx +++ b/src/plugins/bar-graph/legend-area.tsx @@ -7,24 +7,36 @@ import { LegendSecondaryRow } from './legend-secondary-row'; import RemoveDataIcon from "../../assets/remove-data-icon.svg"; import DropdownCaretIcon from "../../assets/dropdown-caret.svg"; import { useReadOnlyContext } from '../../components/document/read-only-context'; +import { logBarGraphEvent } from './bar-graph-utils'; +import { logSharedModelDocEvent } from '../../models/document/log-shared-model-document-event'; +import { LogEventName } from '../../lib/logger-types'; +import { useTileModelContext } from '../../components/tiles/hooks/use-tile-model-context'; +import { getSharedModelManager } from '../../models/tiles/tile-environment'; interface IProps { legendRef: React.RefObject; } export const LegendArea = observer(function LegendArea ({legendRef}: IProps) { + const { tile } = useTileModelContext(); const model = useBarGraphModelContext(); const readOnly = useReadOnlyContext(); function unlinkDataset() { - if (!readOnly && model) { + const sharedModel = model?.sharedModel; + if (!readOnly && sharedModel) { model.unlinkDataSet(); + if (tile) { + const sharedTiles = getSharedModelManager()?.getSharedModelProviders(sharedModel) || []; + logSharedModelDocEvent(LogEventName.TILE_UNLINK, tile, sharedTiles, sharedModel); + } } } function setSecondaryAttribute(attributeId: string|undefined) { if (model) { model.setSecondaryAttribute(attributeId); + logBarGraphEvent(model, "setSecondaryAttribute", { attributeId }); } } From 9c22dc60c3b00eb6093744886cb1a04a3d4b9479 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 13 Sep 2024 14:50:30 -0400 Subject: [PATCH 25/36] Manage deletion of attributes --- src/plugins/bar-graph/bar-graph-content.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-content.ts b/src/plugins/bar-graph/bar-graph-content.ts index f69e757769..60a1571984 100644 --- a/src/plugins/bar-graph/bar-graph-content.ts +++ b/src/plugins/bar-graph/bar-graph-content.ts @@ -161,9 +161,16 @@ export const BarGraphContentModel = TileContentModel self.setPrimaryAttribute(atts[0].id); } } + } + // Check if primary or secondary attribute has been deleted + if (self.primaryAttribute && !self.sharedModel?.dataSet.attrFromID(self.primaryAttribute)) { + self.setPrimaryAttribute(undefined); // this will also unset secondaryAttribute + } + if (self.secondaryAttribute && !self.sharedModel?.dataSet.attrFromID(self.secondaryAttribute)) { + self.setSecondaryAttribute(undefined); + } } - } -})); + })); export interface BarGraphContentModelType extends Instance {} From 858954bdb61388a94e66f62cc52da539db4ce439 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Fri, 13 Sep 2024 15:21:47 -0400 Subject: [PATCH 26/36] Fix table test --- cypress/support/elements/tile/TableToolTile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/support/elements/tile/TableToolTile.js b/cypress/support/elements/tile/TableToolTile.js index 737f146044..68fb922ff9 100644 --- a/cypress/support/elements/tile/TableToolTile.js +++ b/cypress/support/elements/tile/TableToolTile.js @@ -89,7 +89,7 @@ class TableToolTile{ this.getTableCell().eq(cell).type(num+'{enter}'); } getTableIndexColumnCell(){ - return cy.get('.canvas .rdg-cell.index-column'); + return this.getTableTile().find('.rdg-cell.index-column'); } // Fill in a table tile with the given data (a list of lists) // Table tile should in the default state (2 columns, no rows) From f32d8144a2f8b9a93ee84c78aae3f286f40adc92 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Mon, 16 Sep 2024 11:02:13 -0400 Subject: [PATCH 27/36] More tests --- .../tile_tests/bar_graph_tile_spec.js | 30 +++++++++++++++++++ src/plugins/graph/graph-registration.ts | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index 6fe0db9393..57a15d2fe2 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -81,6 +81,18 @@ context('Bar Graph Tile', function () { clueCanvas.getRedoTool().click(); barGraph.getYAxisLabel().should('have.text', 'Counts of something'); + // ESC key should cancel the edit + barGraph.getYAxisLabelButton().click(); + barGraph.getYAxisLabelEditor().should('be.visible').type(' abandon this{esc}'); + barGraph.getYAxisLabelEditor().should('not.exist'); + barGraph.getYAxisLabel().should('have.text', 'Counts of something'); + + // Should not be able to change Y axis label in read-only views + barGraph.getYAxisLabelButton(workspaces[1]).click(); + barGraph.getYAxisLabelEditor(workspaces[1]).should('not.exist'); + barGraph.getYAxisLabelButton(workspaces[2]).click(); + barGraph.getYAxisLabelEditor(workspaces[2]).should('not.exist'); + cy.log('Duplicate tile'); clueCanvas.getDuplicateTool().click(); for (const workspace of workspaces) { @@ -220,6 +232,16 @@ context('Bar Graph Tile', function () { cy.log('Change Sort By'); barGraph.getSortByMenuButton().should('have.text', 'None'); + + // Cannot change sort by in read-only views + for (const workspace of workspaces.slice(1)) { + barGraph.getSortByMenuButton(workspace).click(); + barGraph.getChakraMenuItem(workspace).should('have.length', 3); + barGraph.getChakraMenuItem(workspace).eq(1).should('have.text', 'y'); // menu exists + barGraph.getChakraMenuItem(workspace).should('be.disabled'); // all options disabled + barGraph.getSortByMenuButton(workspace).click(); // close menu + } + barGraph.getSortByMenuButton().click(); barGraph.getChakraMenuItem().should('have.length', 3); barGraph.getChakraMenuItem().eq(1).should('have.text', 'y').click(); @@ -250,6 +272,14 @@ context('Bar Graph Tile', function () { } cy.log('Change Category'); + + // Cannot change category in read-only views + for (const workspace of workspaces.slice(1)) { + barGraph.getXAxisPulldownButton(workspace).click(); + barGraph.getChakraMenuItem(workspace).should('have.length', 3).and('be.disabled'); + barGraph.getXAxisPulldownButton(workspace).click(); // close menu + } + barGraph.getXAxisPulldownButton().click(); barGraph.getChakraMenuItem().should('have.length', 3); barGraph.getChakraMenuItem().eq(1).should('have.text', 'y').click(); diff --git a/src/plugins/graph/graph-registration.ts b/src/plugins/graph/graph-registration.ts index b69a54df04..b09eb95294 100644 --- a/src/plugins/graph/graph-registration.ts +++ b/src/plugins/graph/graph-registration.ts @@ -5,7 +5,7 @@ import { GraphWrapperComponent } from "./components/graph-wrapper-component"; import { createGraphModel, GraphModel } from "./models/graph-model"; import { updateGraphContentWithNewSharedModelIds, updateGraphObjectWithNewSharedModelIds } from "./utilities/graph-utils"; - import { AppConfigModelType } from "../../models/stores/app-config-model"; +import { AppConfigModelType } from "../../models/stores/app-config-model"; import Icon from "./assets/graph-icon.svg"; import HeaderIcon from "./assets/graph-tile-id.svg"; From 42d79e8def54234e353d3f50f47cc9f0ff50e408 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Fri, 13 Sep 2024 09:23:38 -0400 Subject: [PATCH 28/36] reenable cypress test and fix some sort work issues Changes to the test: - need to comment on exemplar docs twice to make doc show up in the correct strategy sort group - deleting strategies is not supported, the test confirms the wrong behavior so we remember to fix it when we fix the behavior Other changes: - the requesting of the investigation by ordinal was wrong and was causing console warnings, fixing this also has fixed part of the issue with titles PT-188227517 - removed the firestoreTagDocumentMap, it was not actually being populated so was not really used in getTagsWithDocs. - handle the case where users comment on exemplar documents. Previously this was causing duplicate items in the sort work groups. --- .../teacher_sort_work_view_spec.js | 23 +++++++--- .../thumbnail/simple-document-item.tsx | 5 +-- src/hooks/document-comment-hooks.ts | 5 +++ src/models/stores/document-group.ts | 3 +- src/models/stores/sorted-documents.ts | 45 ++++++++++++------- src/utilities/sort-document-utils.ts | 19 +++----- 6 files changed, 58 insertions(+), 42 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 f2c6146c2f..1637bf6e01 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 @@ -175,8 +175,7 @@ describe('SortWorkView Tests', () => { }); - // TODO: Reinstate the tests below when all metadata documents have the new fields and are being updated in real time. - it.skip("should open Sort Work tab and test sorting by group", () => { + it("should open Sort Work tab and test sorting by group", () => { const students = ["student:1", "student:2", "student:3", "student:4"]; const studentProblemDocs = [ @@ -300,15 +299,20 @@ describe('SortWorkView Tests', () => { sortWork.getSortWorkItem().contains(exemplarDocs[0]).click(); chatPanel.getChatPanelToggle().click(); chatPanel.addCommentTagAndVerify("Diverging Designs"); + // FIXME: at the moment it is necessary to comment the document twice. + // Search for "exemplar" in document-comment-hooks.ts for an explanation. + cy.wait(100); + 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.getPrimarySortByMenu().click(); - sortWork.getPrimarySortByNameOption().click(); - sortWork.getPrimarySortByMenu().click(); - sortWork.getPrimarySortByTagOption().click(); + sortWork.getShowForMenu().click(); + sortWork.getShowForInvestigationOption().click(); + sortWork.getShowForMenu().click(); + sortWork.getShowForProblemOption().click(); + cy.get('.section-header-arrow').click({multiple: true}); // Open the sections sortWork.checkDocumentInGroup("Diverging Designs", exemplarDocs[0]); @@ -326,7 +330,12 @@ describe('SortWorkView Tests', () => { sortWork.getPrimarySortByTagOption().click(); cy.get('.section-header-arrow').click({multiple: true}); // Open the sections sortWork.checkDocumentInGroup("Unit Rate", exemplarDocs[0]); - sortWork.checkGroupIsEmpty("Diverging Designs"); + + // FIXME: We haven't implemented support for deleting comments + // what should be true: + // sortWork.checkGroupIsEmpty("Diverging Designs"); + // what currently happens + sortWork.checkDocumentInGroup("Diverging Designs", exemplarDocs[0]); cy.log("run CLUE as a student:1 and join group 6"); runClueAsStudent(students[0], 6); diff --git a/src/components/thumbnail/simple-document-item.tsx b/src/components/thumbnail/simple-document-item.tsx index ba48cbaa6a..fe0820ea40 100644 --- a/src/components/thumbnail/simple-document-item.tsx +++ b/src/components/thumbnail/simple-document-item.tsx @@ -16,12 +16,11 @@ export const SimpleDocumentItem = ({ document, investigationOrdinal, onSelectDoc const { documents, class: classStore, unit, user } = useStores(); const { uid } = document; const userName = classStore.getUserById(uid)?.displayName; - const investigations = unit.investigations; // TODO: Make it so we don't have to convert investigationOrdinal and problemOrdinal to numbers here? We do so // because the values originate as strings. Changing their types to numbers in the model would make this unnecessary, // but doing that causes errors elsewhere when trying to load documents that aren't associated with a problem. - const investigation = investigations[Number(investigationOrdinal)]; - const problem = investigation?.problems[Number(problemOrdinal) - 1]; + const investigation = unit.getInvestigation(Number(investigationOrdinal)); + const problem = investigation?.getProblem(Number(problemOrdinal)); const title = document.title ? `${userName}: ${document.title}` : `${userName}: ${problem?.title ?? "unknown title"}`; const isPrivate = !isDocumentAccessibleToUser(document, user, documents); diff --git a/src/hooks/document-comment-hooks.ts b/src/hooks/document-comment-hooks.ts index 41b4706035..5de83972fe 100644 --- a/src/hooks/document-comment-hooks.ts +++ b/src/hooks/document-comment-hooks.ts @@ -139,6 +139,11 @@ export const usePostDocumentComment = (options?: PostDocumentCommentUseMutationO // FIXME: provide access to remoteContext here so we can update strategies on remote // documents. Alternatively move this into a firebase function instead of doing this // in the client. + // FIXME: in the case of exemplar documents, the metadata document won't exist + // until this mutation happens. That probably means metadataQuery.get() below + // will run before the document has been created so it will return an empty array of + // docs. This is another reason the firebase function approach to keep the strategies + // updated is a better way to go const metadataQuery = firestore.collection("documents") .where("key", "==", documentKey) .where("context_id", "==", context.classHash); diff --git a/src/models/stores/document-group.ts b/src/models/stores/document-group.ts index 24db168fdd..8bb92658a7 100644 --- a/src/models/stores/document-group.ts +++ b/src/models/stores/document-group.ts @@ -45,7 +45,6 @@ export class DocumentGroup { stores: ISortedDocumentsStores; label: string; documents: IDocumentMetadata[]; - firestoreTagDocumentMap = new Map>(); icon?: FC>; constructor(props: IDocumentGroup) { @@ -98,7 +97,7 @@ export class DocumentGroup { get byStrategy(): DocumentGroup[] { const commentTags = this.stores.appConfig.commentTags; - const tagsWithDocs = getTagsWithDocs(this.documents, commentTags, this.firestoreTagDocumentMap); + const tagsWithDocs = getTagsWithDocs(this.documents, commentTags); const sortedDocsArr: DocumentGroup[] = []; Object.entries(tagsWithDocs).forEach((tagKeyAndValObj) => { diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index 1318b1b8d0..6b7cbefd40 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -43,7 +43,6 @@ export interface ISortedDocumentsStores { export class SortedDocuments { stores: ISortedDocumentsStores; - firestoreTagDocumentMap = new Map>(); firestoreMetadataDocs: IObservableArray = observable.array([]); constructor(stores: ISortedDocumentsStores) { @@ -113,7 +112,7 @@ export class SortedDocuments { get byStrategy(): DocumentGroup[] { const commentTags = this.commentTags; - const tagsWithDocs = getTagsWithDocs(this.firestoreMetadataDocs, commentTags, this.firestoreTagDocumentMap); + const tagsWithDocs = getTagsWithDocs(this.firestoreMetadataDocs, commentTags); const sortedDocsArr: DocumentGroup[] = []; Object.entries(tagsWithDocs).forEach((tagKeyAndValObj) => { @@ -208,20 +207,34 @@ export class SortedDocuments { tools.push("Sparrow"); } - const metadata: IDocumentMetadata = { - uid: doc.uid, - type: doc.type, - key: doc.key, - createdAt: doc.createdAt, - title: doc.title, - properties: undefined, - tools, - strategies: exemplarStrategy ? [exemplarStrategy] : [], - investigation: doc.investigation, - problem: doc.problem, - unit: doc.unit - }; - docsArray.push(metadata); + const authoredStrategies = exemplarStrategy ? [exemplarStrategy] : []; + + const existingMetadataDoc = docsArray.find(metadataDoc => doc.key === metadataDoc.key); + if (existingMetadataDoc) { + // This will happen if a user comments on a exemplar + // That will create a metadata document in Firestore. + // So in this case we want to update this existing metadata document so we don't + // create a duplicate one + const userStrategies = existingMetadataDoc.strategies || []; + existingMetadataDoc.tools = tools; + existingMetadataDoc.strategies = [...new Set([...authoredStrategies, ...userStrategies])]; + } else { + const metadata: IDocumentMetadata = { + uid: doc.uid, + type: doc.type, + key: doc.key, + createdAt: doc.createdAt, + title: doc.title, + properties: undefined, + tools, + strategies: exemplarStrategy ? [exemplarStrategy] : [], + investigation: doc.investigation, + problem: doc.problem, + unit: doc.unit + }; + docsArray.push(metadata); + } + }); runInAction(() => { diff --git a/src/utilities/sort-document-utils.ts b/src/utilities/sort-document-utils.ts index 60175323b0..c742a23ec9 100644 --- a/src/utilities/sort-document-utils.ts +++ b/src/utilities/sort-document-utils.ts @@ -71,8 +71,10 @@ export const sortNameSectionLabels = (docMapKeys: string[]) => { }); }; -export const getTagsWithDocs = (documents: IDocumentMetadata[], commentTags: Record|undefined, - firestoreTagDocumentMap: Map>) => { +export const getTagsWithDocs = ( + documents: IDocumentMetadata[], + commentTags: Record|undefined, +) => { const tagsWithDocs: Record = {}; if (commentTags) { for (const key of Object.keys(commentTags)) { @@ -93,18 +95,7 @@ export const getTagsWithDocs = (documents: IDocumentMetadata[], commentTags: Rec // in store to find "Documents with no comments" then place those doc keys to "Not Tagged" const uniqueDocKeysWithTags = new Set(); - // grouping documents based on firestore comment tags - 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 + // Sort documents into their groups documents.forEach(doc => { doc.strategies?.forEach(strategy => { if (tagsWithDocs[strategy]) { From 073a763ca9276e0b7d0d0a7331eb902978623b1b Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Fri, 13 Sep 2024 09:59:28 -0400 Subject: [PATCH 29/36] decrease Cypress timeout when running locally --- cypress/config/cypress.dev.json | 4 ---- cypress/config/cypress.local.json | 3 ++- cypress/run_cypress_test.sh | 19 ------------------- package.json | 1 - 4 files changed, 2 insertions(+), 25 deletions(-) delete mode 100644 cypress/config/cypress.dev.json delete mode 100755 cypress/run_cypress_test.sh diff --git a/cypress/config/cypress.dev.json b/cypress/config/cypress.dev.json deleted file mode 100644 index 2aafc67ebb..0000000000 --- a/cypress/config/cypress.dev.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "baseUrl": "http://localhost:8080/" -} - diff --git a/cypress/config/cypress.local.json b/cypress/config/cypress.local.json index d36314102c..00ea18a126 100644 --- a/cypress/config/cypress.local.json +++ b/cypress/config/cypress.local.json @@ -1,4 +1,5 @@ { "baseUrl": "http://localhost:8080/", - "hideXHRInCommandLog": true + "hideXHRInCommandLog": true, + "defaultCommandTimeout": 5000 } diff --git a/cypress/run_cypress_test.sh b/cypress/run_cypress_test.sh deleted file mode 100755 index 47fe7341ab..0000000000 --- a/cypress/run_cypress_test.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/bash - -declare -a BRANCH_ARRAY=("master" "production" "dataflow" "dataflow_production") - -npm run start & -wait-on http://localhost:8080 -echo "start TRAVIS_BRANCH=$TRAVIS_BRANCH" -if [[ "$TRAVIS_COMMIT_MESSAGE" == *"[dev-build]"* ]]; then - echo "dev-build" - npm run test:cypress:smoke -elif !(echo $BRANCH_ARRAY | grep -q $TRAVIS_BRANCH); then - echo "elif TRAVIS_BRANCH=$TRAVIS_BRANCH" - npm run test:cypress:branch -else - echo "else TRAVIS_BRANCH=$TRAVIS_BRANCH" - npm run test:cypress:ci -fi - - \ No newline at end of file diff --git a/package.json b/package.json index 72a0dd5a2b..040e8f2011 100644 --- a/package.json +++ b/package.json @@ -97,7 +97,6 @@ "test:coverage:watch": "jest --coverage --watchAll", "test:coverage:cypress:open": "cypress open --env coverage=true", "test:cypress": "cypress run --env testEnv=local", - "test:cypress:ci": "cypress run --env testEnv=local --record", "test:cypress:open": "cypress open --env testEnv=local", "test:cypress:open:disable-gpu": "cross-env ELECTRON_EXTRA_LAUNCH_ARGS=--disable-gpu cypress open --env testEnv=local", "test:cypress:smoke": "cypress run --spec 'cypress/e2e/smoke/single_student_canvas_test.js' --env testEnv=local", From e757aaad080be4f9273030012d7cea12ae6bb155 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Fri, 13 Sep 2024 11:08:04 -0400 Subject: [PATCH 30/36] include fixes that Ethan found in a closed PR --- src/components/document/sorted-section.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/document/sorted-section.tsx b/src/components/document/sorted-section.tsx index 5361b6b936..5b9f158399 100644 --- a/src/components/document/sorted-section.tsx +++ b/src/components/document/sorted-section.tsx @@ -23,7 +23,7 @@ interface IProps { secondarySort: SecondarySortType; } -export const SortedSection: React.FC = observer(function SortedDocuments(props: IProps) { +export const SortedSection: React.FC = observer(function SortedSection(props: IProps) { const { docFilter, documentGroup, idx, secondarySort } = props; const { persistentUI, sortedDocuments } = useStores(); const [showDocuments, setShowDocuments] = useState(false); @@ -51,7 +51,7 @@ export const SortedSection: React.FC = observer(function SortedDocuments const renderUngroupedDocument = (doc: IDocumentMetadata) => { const fullDocument = getDocument(doc.key); - if (!fullDocument) return
; + if (!fullDocument) return
; return Date: Tue, 17 Sep 2024 09:32:15 -0400 Subject: [PATCH 31/36] remove the local storage dev root id This dev root approach requires the firebase functions to be updated and there isn't time to do that, now. The refactoring is kept so applying the change will be easier in the future. --- firestore.rules | 3 ++- src/clue/components/clue-app-header.tsx | 2 -- src/lib/db.ts | 3 ++- src/lib/firebase.test.ts | 20 ++++++-------------- src/lib/firestore.test.ts | 7 ------- src/lib/root-id.ts | 18 +----------------- src/models/stores/user-context-provider.ts | 4 ++++ 7 files changed, 15 insertions(+), 42 deletions(-) diff --git a/firestore.rules b/firestore.rules index 72a79b3526..593a59c01d 100644 --- a/firestore.rules +++ b/firestore.rules @@ -426,7 +426,8 @@ service cloud.firestore { allow read, write: if isAuthed(); } - // Developers get a random unique key that their data is stored under + // In the future, developers might use a random unique key instead of + // the firebase user id. So this rule is relaxed inorder to allow that. match /dev/{devId}/{restOfPath=**} { allow read, write: if isAuthed(); } diff --git a/src/clue/components/clue-app-header.tsx b/src/clue/components/clue-app-header.tsx index 56a09e76ab..ef39da545c 100644 --- a/src/clue/components/clue-app-header.tsx +++ b/src/clue/components/clue-app-header.tsx @@ -9,7 +9,6 @@ import { ToggleGroup } from "@concord-consortium/react-components"; import { GroupModelType, GroupUserModelType } from "../../models/stores/groups"; import { CustomSelect } from "./custom-select"; import { useStores } from "../../hooks/use-stores"; -import { getDevId } from "../../lib/root-id"; // cf. https://mattferderer.com/use-sass-variables-in-typescript-and-javascript import styles from "./toggle-buttons.scss"; @@ -30,7 +29,6 @@ export const ClueAppHeaderComponent: React.FC = observer(function ClueAp const getUserTitle = () => { switch(appMode){ case "dev": - return `Firebase Root: ${getDevId()}`; case "qa": case "test": return `Firebase UID: ${db.firebase.userId}`; diff --git a/src/lib/db.ts b/src/lib/db.ts index 826f9f5960..f24ff93fb9 100644 --- a/src/lib/db.ts +++ b/src/lib/db.ts @@ -40,6 +40,7 @@ import { urlParams } from "../utilities/url-params"; import { firebaseConfig } from "./firebase-config"; import { UserModelType } from "../models/stores/user"; import { logExemplarDocumentEvent } from "../models/document/log-exemplar-document-event"; +import { AppMode } from "../models/stores/store-types"; import { DEBUG_FIRESTORE } from "./debug"; export type IDBConnectOptions = IDBAuthConnectOptions | IDBNonAuthConnectOptions; @@ -55,7 +56,7 @@ export interface IDBAuthConnectOptions extends IDBBaseConnectOptions { rawFirebaseJWT: string; } export interface IDBNonAuthConnectOptions extends IDBBaseConnectOptions { - appMode: "dev" | "test" | "demo" | "qa"; + appMode: Exclude; } export interface UserGroupMap { [key: string]: { diff --git a/src/lib/firebase.test.ts b/src/lib/firebase.test.ts index 9db8b9f986..b8732e95e2 100644 --- a/src/lib/firebase.test.ts +++ b/src/lib/firebase.test.ts @@ -1,6 +1,5 @@ import { DB } from "./db"; import { Firebase } from "./firebase"; -import { kClueDevIDKey } from "./root-id"; const mockStores = { appMode: "authed", @@ -23,31 +22,24 @@ describe("Firebase class", () => { const firebase = new Firebase(mockDB); expect(firebase.getRootFolder()).toBe("/authed/portals/test-portal/"); }); - it("should handle the dev appMode", () => { - window.localStorage.setItem(kClueDevIDKey, "random-id"); - const stores = {...mockStores, appMode: "dev"}; - const firestore = new Firebase({stores} as DB); - expect(firestore.getRootFolder()).toBe("/dev/random-id/portals/test-portal/"); - }); describe("should handle the demo appMode", () => { it("handles basic demo name", () => { const stores = {...mockStores, appMode: "demo", demo: { name: "test-demo" }}; - const firestore = new Firebase({stores} as DB); - expect(firestore.getRootFolder()).toBe("/demo/test-demo/portals/test-portal/"); + const firebase = new Firebase({stores} as DB); + expect(firebase.getRootFolder()).toBe("/demo/test-demo/portals/test-portal/"); }); it("handles empty demo name", () => { const stores = {...mockStores, appMode: "demo", demo: { name: "" }}; - const firestore = new Firebase({stores} as DB); - expect(firestore.getRootFolder()).toBe("/demo/test-portal/portals/test-portal/"); + const firebase = new Firebase({stores} as DB); + expect(firebase.getRootFolder()).toBe("/demo/test-portal/portals/test-portal/"); }); it("handles empty demo name and empty portal", () => { const stores = {...mockStores, appMode: "demo", demo: { name: "" }, user: { portal: ""}}; - const firestore = new Firebase({stores} as DB); - // FIXME - expect(firestore.getRootFolder()).toBe("/demo/demo/portals//"); + const firebase = new Firebase({stores} as DB); + expect(firebase.getRootFolder()).toBe("/demo/demo/portals//"); }); }); }); diff --git a/src/lib/firestore.test.ts b/src/lib/firestore.test.ts index 78fe1f6c28..956c689caa 100644 --- a/src/lib/firestore.test.ts +++ b/src/lib/firestore.test.ts @@ -1,6 +1,5 @@ import { DB } from "./db"; import { Firestore } from "./firestore"; -import { kClueDevIDKey } from "./root-id"; const mockStores = { appMode: "authed", @@ -66,12 +65,6 @@ describe("Firestore class", () => { const firestore = new Firestore(mockDB); expect(firestore.getRootFolder()).toBe("/authed/test-portal/"); }); - it("should handle the dev appMode", () => { - window.localStorage.setItem(kClueDevIDKey, "random-id"); - const stores = {...mockStores, appMode: "dev"}; - const firestore = new Firestore({stores} as DB); - expect(firestore.getRootFolder()).toBe("/dev/random-id/"); - }); describe("should handle the demo appMode", () => { it("handles basic demo name", () => { const stores = {...mockStores, diff --git a/src/lib/root-id.ts b/src/lib/root-id.ts index c75e476aca..f2d14f1a1f 100644 --- a/src/lib/root-id.ts +++ b/src/lib/root-id.ts @@ -1,19 +1,6 @@ -import { nanoid } from "nanoid"; import { IStores } from "../models/stores/stores"; import { escapeKey } from "./fire-utils"; -export const kClueDevIDKey = "clue-dev-id"; - -export function getDevId() { - let devId = window.localStorage.getItem(kClueDevIDKey); - if (!devId) { - const newDevId = nanoid(); - window.localStorage.setItem(kClueDevIDKey, newDevId); - devId = newDevId; - } - return devId; -} - type IRootDocIdStores = Pick; export function getRootId(stores: IRootDocIdStores, firebaseUserId: string) { @@ -35,10 +22,7 @@ export function getRootId(stores: IRootDocIdStores, firebaseUserId: string) { const escapedDemoName = demoName ? escapeKey(demoName) : demoName; return escapedDemoName || escapedPortal || "demo"; } - case "dev": { - return getDevId(); - } - // "test" and "qa" + // "dev", "qa", and "test" default: { return firebaseUserId; } diff --git a/src/models/stores/user-context-provider.ts b/src/models/stores/user-context-provider.ts index 72f5a157b5..e81d9e0299 100644 --- a/src/models/stores/user-context-provider.ts +++ b/src/models/stores/user-context-provider.ts @@ -11,6 +11,10 @@ export class UserContextProvider { this.stores = stores; } + /** + * This user context is sent to the Firebase functions so they know the context of the + * request. + */ get userContext() { const appMode = this.stores.appMode; const { name: demoName } = this.stores.demo; From 7041be447d37e9e5139bc8f3b34b844e123de248 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Tue, 17 Sep 2024 10:06:29 -0400 Subject: [PATCH 32/36] Add selected highlight to bars --- src/plugins/bar-graph/bar-graph-content.ts | 33 +++++++---- src/plugins/bar-graph/bar-graph-types.ts | 2 + src/plugins/bar-graph/chart-area.tsx | 68 +++++++++++++++++----- 3 files changed, 77 insertions(+), 26 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-content.ts b/src/plugins/bar-graph/bar-graph-content.ts index 60a1571984..bbbacfc2dd 100644 --- a/src/plugins/bar-graph/bar-graph-content.ts +++ b/src/plugins/bar-graph/bar-graph-content.ts @@ -1,7 +1,7 @@ import { types, Instance } from "mobx-state-tree"; -import { isNumber } from "lodash"; +import { isObject } from "lodash"; import { ITileContentModel, TileContentModel } from "../../models/tiles/tile-content"; -import { kBarGraphTileType, kBarGraphContentType } from "./bar-graph-types"; +import { kBarGraphTileType, kBarGraphContentType, BarInfo } from "./bar-graph-types"; import { getSharedModelManager } from "../../models/tiles/tile-environment"; import { SharedDataSet, SharedDataSetType } from "../../models/shared/shared-data-set"; import { clueDataColorInfo } from "../../utilities/color-utils"; @@ -72,30 +72,38 @@ export const BarGraphContentModel = TileContentModel return cases.reduce((acc, caseID) => { const cat = displayValue(dataSet.getStrValue(caseID.__id__, primary)); const subCat = displayValue(dataSet.getStrValue(caseID.__id__, secondary)); + const selected = dataSet.isCaseSelected(caseID.__id__); const index = acc.findIndex(r => r[primary] === cat); if (index >= 0) { const cur = acc[index][subCat]; - acc[index][subCat] = (isNumber(cur) ? cur : 0) + 1; + if (isObject(cur)) { + acc[index][subCat] = { count: cur.count + 1, selected: cur.selected || selected }; + } else { + acc[index][subCat] = { count: 1, selected }; + } } else { - const newRow = { [primary]: cat, [subCat]: 1 }; + const newRow = { [primary]: cat, [subCat]: { count: 1, selected } }; acc.push(newRow); } return acc; - }, [] as { [key: string]: number | string }[]); + }, [] as { [key: string]: BarInfo | string }[]); } else { // One-dimensional data return cases.reduce((acc, caseID) => { const cat = displayValue(dataSet.getStrValue(caseID.__id__, primary)); + const selected = dataSet.isCaseSelected(caseID.__id__); const index = acc.findIndex(r => r[primary] === cat); if (index >= 0) { const cur = acc[index].value; - acc[index].value = isNumber(cur) ? cur + 1 : 1; + if (isObject(cur)) { + acc[index].value = { count: cur.count + 1, selected: cur.selected || selected }; + } } else { - const newRow = { [primary]: cat, value: 1 }; + const newRow = { [primary]: cat, value: { count: 1, selected } }; acc.push(newRow); } return acc; - }, [] as { [key: string]: number | string }[]); + }, [] as { [key: string]: BarInfo | string }[]); } } })) @@ -106,13 +114,14 @@ export const BarGraphContentModel = TileContentModel return self.dataArray.map(d => d[primary] as string); }, get secondaryKeys() { - const primary = self.primaryAttribute; - if (!primary) return []; - return Array.from(new Set(self.dataArray.flatMap(d => Object.keys(d)).filter(k => k !== primary))); + const dataSet = self.sharedModel?.dataSet; + const secondary = self.secondaryAttribute; + if (!secondary || !dataSet || !self.cases) return []; + return Array.from(new Set(self.cases.map(caseID => displayValue(dataSet.getStrValue(caseID.__id__, secondary))))); }, get maxDataValue(): number { return self.dataArray.reduce((acc, row) => { - const rowValues = Object.values(row).filter(v => isNumber(v)) as number[]; + const rowValues = Object.values(row).map(v => isObject(v) ? v.count : 0); const maxInRow = Math.max(...rowValues); return Math.max(maxInRow, acc); }, 0); diff --git a/src/plugins/bar-graph/bar-graph-types.ts b/src/plugins/bar-graph/bar-graph-types.ts index 9bbbf9cc49..4a6f2eb124 100644 --- a/src/plugins/bar-graph/bar-graph-types.ts +++ b/src/plugins/bar-graph/bar-graph-types.ts @@ -3,3 +3,5 @@ export const kBarGraphTileType = "BarGraph"; export const kBarGraphContentType = "BarGraphContentModel"; export const kBarGraphDefaultHeight = 320; + +export type BarInfo = { count: number, selected: boolean }; diff --git a/src/plugins/bar-graph/chart-area.tsx b/src/plugins/bar-graph/chart-area.tsx index f250997188..cc7768771f 100644 --- a/src/plugins/bar-graph/chart-area.tsx +++ b/src/plugins/bar-graph/chart-area.tsx @@ -5,10 +5,12 @@ import { GridRows } from "@visx/grid"; import { Group } from "@visx/group"; import { scaleBand, scaleLinear } from "@visx/scale"; import { Bar, BarGroup } from "@visx/shape"; +import { PositionScale } from "@visx/shape/lib/types"; import { useBarGraphModelContext } from "./bar-graph-content-context"; import { CategoryPulldown } from "./category-pulldown"; import EditableAxisLabel from "./editable-axis-label"; import { logBarGraphEvent, roundTo5 } from "./bar-graph-utils"; +import { BarInfo } from "./bar-graph-types"; const margin = { top: 7, @@ -96,16 +98,13 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro {data.map((d) => { const key = d[primary] as string; - const val = d.value as number; + const info = d.value as BarInfo; + const x = primaryScale(key) || 0; + const y = countScale(info.count); + const w = primaryScale.bandwidth(); + const h = yMax - countScale(info.count); return ( - + ); })} @@ -122,21 +121,25 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro x0={(d) => d[primary] as string} x0Scale={primaryScale} x1Scale={secondaryScale} - yScale={countScale} + yScale={((info: BarInfo) => countScale(info?.count||0)) as PositionScale} > {(barGroups) => {barGroups.map((barGroup) => ( - + {barGroup.bars.map((bar) => { if (!bar.value) return null; - return ; })} @@ -192,3 +195,40 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro ); }); + +interface IBarHighlightProps { + x: number; + y: number; + width: number; + height: number; +} + +function BarHighlight({ x, y, width, height }: IBarHighlightProps) { + return( + + ); +} + +interface IBarWithHighlightProps { + x: number; + y: number; + width: number; + height: number; + color: string; + selected: boolean; +} + +function BarWithHighlight({ x, y, width, height, color, selected }: IBarWithHighlightProps) { + return ( + + {selected && } + + + ); +} From a39f2794ed5f5e28a5476943fae94a51ddbe9139 Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Tue, 17 Sep 2024 11:28:22 -0400 Subject: [PATCH 33/36] Click on bars to select --- .../bar-graph/bar-graph-content.test.ts | 26 ++++---- src/plugins/bar-graph/bar-graph-content.ts | 16 +++++ src/plugins/bar-graph/bar-graph-utils.ts | 5 +- src/plugins/bar-graph/chart-area.tsx | 65 ++++++++++++------- 4 files changed, 73 insertions(+), 39 deletions(-) diff --git a/src/plugins/bar-graph/bar-graph-content.test.ts b/src/plugins/bar-graph/bar-graph-content.test.ts index 565a7f56fc..79c70942b1 100644 --- a/src/plugins/bar-graph/bar-graph-content.test.ts +++ b/src/plugins/bar-graph/bar-graph-content.test.ts @@ -107,14 +107,14 @@ Object { content.setSharedModel(sharedSampleDataSet()); content.setPrimaryAttribute("att-s"); expect(content.dataArray).toEqual([ - { "att-s": "cat", "value": 2 }, - { "att-s": "owl","value": 2} + { "att-s": "cat", "value": { count: 2, selected: false }}, + { "att-s": "owl","value": { count: 2, selected: false }} ]); content.setPrimaryAttribute("att-l"); expect(content.dataArray).toEqual([ - { "att-l": "yard", "value": 3 }, - { "att-l": "forest", "value": 1 } + { "att-l": "yard", "value": { count: 3, selected: false }}, + { "att-l": "forest", "value": { count: 1, selected: false }} ]); }); @@ -122,8 +122,8 @@ Object { const content = TestingBarGraphContentModel.create({ }); content.setSharedModel(sharedSampleDataSet()); expect(content.dataArray).toEqual([ - { "att-s": "cat", "value": 2 }, - { "att-s": "owl","value": 2} + { "att-s": "cat", "value": { count: 2, selected: false }}, + { "att-s": "owl","value": { count: 2, selected: false }} ]); }); @@ -133,8 +133,8 @@ Object { content.setPrimaryAttribute("att-s"); content.setSecondaryAttribute("att-l"); expect(content.dataArray).toEqual([ - { "att-s": "cat", "yard": 2 }, - { "att-s": "owl", "yard": 1, "forest": 1 } + { "att-s": "cat", "yard": { count: 2, selected: false }}, + { "att-s": "owl", "yard": { count: 1, selected: false }, "forest": { count: 1, selected: false }} ]); }); @@ -146,15 +146,15 @@ Object { content.setPrimaryAttribute("att-s"); content.setSecondaryAttribute("att-l"); expect(content.dataArray).toEqual([ - { "att-s": "cat", "yard": 2 }, - { "att-s": "owl", "yard": 1, "(no value)": 1 } + { "att-s": "cat", "yard": { count: 2, selected: false }}, + { "att-s": "owl", "yard": { count: 1, selected: false}, "(no value)": { count: 1, selected: false }} ]); dataSet.dataSet?.attributes[0].setValue(3, undefined); // hide that owl entirely expect(content.dataArray).toEqual([ - { "att-s": "cat", "yard": 2 }, - { "att-s": "owl", "yard": 1 }, - { "att-s": "(no value)", "(no value)": 1 } + { "att-s": "cat", "yard": { count: 2, selected: false }}, + { "att-s": "owl", "yard": { count: 1, selected: false }}, + { "att-s": "(no value)", "(no value)": { count: 1, selected: false }} ]); }); diff --git a/src/plugins/bar-graph/bar-graph-content.ts b/src/plugins/bar-graph/bar-graph-content.ts index bbbacfc2dd..7ef2a9a53b 100644 --- a/src/plugins/bar-graph/bar-graph-content.ts +++ b/src/plugins/bar-graph/bar-graph-content.ts @@ -145,6 +145,22 @@ export const BarGraphContentModel = TileContentModel }, setSecondaryAttribute(attrId: string|undefined) { self.secondaryAttribute = attrId; + }, + selectCasesByValues(primaryVal: string, secondaryVal?: string) { + const dataSet = self.sharedModel?.dataSet; + const cases = self.cases; + const primaryAttribute = self.primaryAttribute; + if (!dataSet || !cases || !primaryAttribute) return; + const secondaryAttribute = self.secondaryAttribute; + if (!secondaryAttribute && secondaryVal) return; + let matchingCases = cases + .filter(caseID => displayValue(dataSet.getStrValue(caseID.__id__, primaryAttribute)) === primaryVal); + if (secondaryAttribute && secondaryVal) { + matchingCases = matchingCases + .filter(caseID => displayValue(dataSet.getStrValue(caseID.__id__, secondaryAttribute)) === secondaryVal); + } + const caseIds = matchingCases.map(caseID => caseID.__id__); + dataSet.setSelectedCases(caseIds); } })) .actions(self => ({ diff --git a/src/plugins/bar-graph/bar-graph-utils.ts b/src/plugins/bar-graph/bar-graph-utils.ts index e5a1d86810..a742c6c91b 100644 --- a/src/plugins/bar-graph/bar-graph-utils.ts +++ b/src/plugins/bar-graph/bar-graph-utils.ts @@ -37,9 +37,10 @@ export function updateBarGraphContentWithNewSharedModelIds( } // Define types here to document all possible values that this tile logs -type LoggableOperation = "setPrimaryAttribute" | "setSecondaryAttribute" | "setYAxisLabel"; +type LoggableOperation = "setPrimaryAttribute" | "setSecondaryAttribute" | "setYAxisLabel" | "selectCases"; type LoggableChange = { - attributeId?: string; + attributeId?: string | string[]; + attributeValue?: string | string[]; text?: string; }; diff --git a/src/plugins/bar-graph/chart-area.tsx b/src/plugins/bar-graph/chart-area.tsx index cc7768771f..d0e9d89f2a 100644 --- a/src/plugins/bar-graph/chart-area.tsx +++ b/src/plugins/bar-graph/chart-area.tsx @@ -92,6 +92,16 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro const labelWidth = (xMax/primaryKeys.length)-10; // setting width will wrap lines when needed + function handleClick(primaryValue: string, secondaryValue?: string) { + if (!model || !model.primaryAttribute) return; + model.selectCasesByValues(primaryValue, secondaryValue); + logBarGraphEvent(model, "selectCases", { + attributeId: + model.secondaryAttribute ? [model.primaryAttribute, model.secondaryAttribute] : model.primaryAttribute, + attributeValue: secondaryValue ? [primaryValue, secondaryValue] : primaryValue + }); + } + function simpleBars() { const color = barColor(primary); return ( @@ -104,7 +114,8 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro const w = primaryScale.bandwidth(); const h = yMax - countScale(info.count); return ( - + handleClick(key)} /> ); })} @@ -123,29 +134,34 @@ export const ChartArea = observer(function BarGraphChart({ width, height }: IPro x1Scale={secondaryScale} yScale={((info: BarInfo) => countScale(info?.count||0)) as PositionScale} > - {(barGroups) => - - {barGroups.map((barGroup) => ( - - {barGroup.bars.map((bar) => { - if (!bar.value) return null; - // BarGroup really expects the values to be pure numeric, but we're using objects. - // Alternatively, we could drop BarGroup and build the bars manually. - const val = bar.value as unknown as BarInfo; - return ; + {(barGroups) => { + return ( + + {barGroups.map((barGroup) => { + const primaryValue = data[barGroup.index][primary] as string; + return ( + + {barGroup.bars.map((bar) => { + if (!bar.value) return null; + // BarGroup really expects the values to be pure numeric, but we're using objects. + // Alternatively, we could drop BarGroup and build the bars manually. + const val = bar.value as unknown as BarInfo; + return handleClick(primaryValue, bar.key)} />; + })} + + ); })} - ))} - - } + ); + }} ); } @@ -222,13 +238,14 @@ interface IBarWithHighlightProps { height: number; color: string; selected: boolean; + onClick: () => void; } -function BarWithHighlight({ x, y, width, height, color, selected }: IBarWithHighlightProps) { +function BarWithHighlight({ x, y, width, height, color, selected, onClick }: IBarWithHighlightProps) { return ( {selected && } - + ); } From 861db2a029f3ff4d26273ec4f1f46a43e8104c3e Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Tue, 17 Sep 2024 15:42:45 -0400 Subject: [PATCH 34/36] Add tests --- .../tile_tests/bar_graph_tile_spec.js | 156 ++++++++++++++++++ cypress/support/elements/tile/BarGraphTile.js | 4 + .../support/elements/tile/TableToolTile.js | 3 + .../bar-graph/bar-graph-content.test.ts | 71 ++++++++ src/plugins/bar-graph/chart-area.tsx | 1 + 5 files changed, 235 insertions(+) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index 57a15d2fe2..530b77c360 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -335,4 +335,160 @@ context('Bar Graph Tile', function () { } }); + it.only('Synchronizes selection', function () { + beforeTest(); + + clueCanvas.addTile('bargraph'); + + // Table dataset for testing: + // 4 instances of X / Y / Z + // 2 instances of XX / Y / Z + // 1 instance of X / YY / Z + clueCanvas.addTile('table'); + tableTile.fillTable(tableTile.getTableTile(), [ + ['X', 'Y', 'Z'], + ['XX', 'Y', 'Z'], + ['X', 'YY', 'Z'], + ['X', 'Y', 'Z'], + ['XX', 'Y', 'Z'], + ['X', 'Y', 'Z'], + ['X', 'Y', 'Z'], + ]); + + barGraph.getTile().click(); + clueCanvas.clickToolbarButton('bargraph', 'link-tile'); + cy.get('select').select('Table Data 1'); + cy.get('.modal-button').contains("Graph It!").click(); + + cy.log("Check synchronization of case selection with one attribute"); + + // Selecting cases in the table should highlight the corresponding bars in the bar graph + tableTile.getSelectedRow(workspaces[0]).should('have.length', 0); + for (const workspace of workspaces) { + barGraph.getBarHighlight(workspace).should('have.length', 0); + } + tableTile.getTableIndexColumnCell().eq(0).click(); // first X Y Z case, X bar selected + tableTile.getSelectedRow(workspaces[0]).should('have.length', 1); + barGraph.getBarHighlight(workspaces[0]).should('have.length', 1); + barGraph.getBarHighlight(workspaces[1]).should('have.length', 1); + // Selection is local, volatile state, so remote workspace should not be affected + barGraph.getBarHighlight(workspaces[2]).should('have.length', 0); + tableTile.getTableIndexColumnCell().eq(1).click({shiftKey: true}); // first XX Y Z case, X and XX bars selected + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 2); + } + tableTile.getTableIndexColumnCell().eq(2).click({shiftKey: true}); // first X YY Z case, X and XX bars selected + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 2); + } + tableTile.getTableIndexColumnCell().eq(0).click({shiftKey: true}); + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 2); + } + tableTile.getTableIndexColumnCell().eq(1).click({shiftKey: true}); + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 1); + } + tableTile.getTableIndexColumnCell().eq(2).click({shiftKey: true}); + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 0); + } + + // Clicking on bars should select the corresponding cases in the table + tableTile.getSelectedRow(workspaces[0]).should('have.length', 0); + barGraph.getBar().eq(0).click(); + for (const workspace of workspaces.slice(0, 2)) { + tableTile.getSelectedRow(workspace).should('have.length', 5); // All "X" cases + } + barGraph.getBar().eq(1).click(); + for (const workspace of workspaces.slice(0, 2)) { + tableTile.getSelectedRow(workspace).should('have.length', 2); // All "XX" cases + } + // Unselect the two selected cases, which should be numbers 1 and 4 + tableTile.getTableIndexColumnCell().eq(1).click({shiftKey: true}); + tableTile.getTableIndexColumnCell().eq(4).click({shiftKey: true}); + tableTile.getSelectedRow(workspaces[0]).should('have.length', 0); + + cy.log("Check synchronization of case selection with two attributes"); + barGraph.getSortByMenuButton().click(); + barGraph.getChakraMenuItem().should('have.length', 3); + barGraph.getChakraMenuItem().eq(1).should('have.text', 'y').click(); + barGraph.getBar().should("have.length", 3); + + tableTile.getTableIndexColumnCell().eq(0).click(); // first X Y Z case, X Y bar selected + tableTile.getSelectedRow(workspaces[0]).should('have.length', 1); + barGraph.getBarHighlight(workspaces[0]).should('have.length', 1); + barGraph.getBarHighlight(workspaces[1]).should('have.length', 1); + // Selection is local, volatile state, so remote workspace should not be affected + barGraph.getBarHighlight(workspaces[2]).should('have.length', 0); + + tableTile.getTableIndexColumnCell().eq(1).click({shiftKey: true}); // first XX Y Z case, X Y and XX Y bars selected + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 2); + } + tableTile.getTableIndexColumnCell().eq(2).click({shiftKey: true}); // first X YY Z case, X Y, XX Y, and X YY bars selected + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 3); + } + tableTile.getTableIndexColumnCell().eq(0).click({shiftKey: true}); + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 2); + } + tableTile.getTableIndexColumnCell().eq(1).click({shiftKey: true}); + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 1); + } + tableTile.getTableIndexColumnCell().eq(2).click({shiftKey: true}); + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 0); + } + + // Clicking on bars should select the corresponding cases in the table + barGraph.getBar().eq(0).click(); + for (const workspace of workspaces.slice(0, 2)) { + tableTile.getSelectedRow(workspace).should('have.length', 4); // All "X / Y" cases + } + barGraph.getBar().eq(1).click(); + for (const workspace of workspaces.slice(0, 2)) { + tableTile.getSelectedRow(workspace).should('have.length', 1); // All "X / YY" cases + } + barGraph.getBar().eq(2).click(); + for (const workspace of workspaces.slice(0, 2)) { + tableTile.getSelectedRow(workspace).should('have.length', 2); // All "XX / Y" cases + } + + // Clicking bars in local read-only view also works and changes the main view, but not the remote view. + barGraph.getBar(workspaces[1]).eq(0).click(); + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 1); + tableTile.getSelectedRow(workspace).should('have.length', 4); // All "X / Y" cases + } + barGraph.getBar(workspaces[1]).eq(1).click(); + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 1); + tableTile.getSelectedRow(workspace).should('have.length', 1); // All "X / YY" cases + } + barGraph.getBar(workspaces[1]).eq(2).click(); + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 1); + tableTile.getSelectedRow(workspace).should('have.length', 2); // All "XX / Y" cases + } + // Unselect the two remaining selected cases + tableTile.getTableIndexColumnCell().eq(1).click({shiftKey: true}); + tableTile.getTableIndexColumnCell().eq(4).click({shiftKey: true}); + tableTile.getSelectedRow(workspaces[0]).should('have.length', 0); + + // Clicking bars in remote read-only view does not change the main view, but does change itself. + barGraph.getBarHighlight(workspaces[2]).should('have.length', 0); + tableTile.getSelectedRow(workspaces[2]).should('have.length', 0); + barGraph.getBar(workspaces[2]).eq(0).click(); + barGraph.getBarHighlight(workspaces[2]).should('have.length', 1); + tableTile.getSelectedRow(workspaces[2]).should('have.length', 4); // All "X / Y" cases + for (const workspace of workspaces.slice(0, 2)) { + barGraph.getBarHighlight(workspace).should('have.length', 0); + tableTile.getSelectedRow(workspace).should('have.length', 0); // All "XX / Y" cases + } + + }); + }); diff --git a/cypress/support/elements/tile/BarGraphTile.js b/cypress/support/elements/tile/BarGraphTile.js index d19a3b1d9e..fdf85d7b20 100644 --- a/cypress/support/elements/tile/BarGraphTile.js +++ b/cypress/support/elements/tile/BarGraphTile.js @@ -56,6 +56,10 @@ class BarGraphTile { return this.getChartArea(workspaceClass, tileIndex).find(`.visx-bar`); } + getBarHighlight(workspaceClass, tileIndex = 0) { + return this.getChartArea(workspaceClass, tileIndex).find(`.bar-highlight`); + } + getLegendArea(workspaceClass, tileIndex = 0) { return this.getTile(workspaceClass, tileIndex).find(`.bar-graph-legend`); } diff --git a/cypress/support/elements/tile/TableToolTile.js b/cypress/support/elements/tile/TableToolTile.js index 68fb922ff9..947a8172dc 100644 --- a/cypress/support/elements/tile/TableToolTile.js +++ b/cypress/support/elements/tile/TableToolTile.js @@ -41,6 +41,9 @@ class TableToolTile{ getTableRow(){ return cy.get('.canvas .rdg-row'); } + getSelectedRow(workspaceClass) { + return cy.get(`${wsclass(workspaceClass)} .canvas .rdg-row.highlighted`); + } getColumnHeaderText(i){ return cy.get('.column-header-cell .editable-header-cell .header-name').text(); } diff --git a/src/plugins/bar-graph/bar-graph-content.test.ts b/src/plugins/bar-graph/bar-graph-content.test.ts index 79c70942b1..e4788f7ca0 100644 --- a/src/plugins/bar-graph/bar-graph-content.test.ts +++ b/src/plugins/bar-graph/bar-graph-content.test.ts @@ -118,6 +118,28 @@ Object { ]); }); + it("returns expected array when a case is selected with primary attribute", () => { + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); + content.setPrimaryAttribute("att-s"); + content.sharedModel?.dataSet.setSelectedCases([content.sharedModel?.dataSet.cases[0].__id__]); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "value": { count: 2, selected: true }}, + { "att-s": "owl", "value": { count: 2, selected: false }} + ]); + content.sharedModel?.dataSet.setSelectedCases([content.sharedModel?.dataSet.cases[2].__id__]); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "value": { count: 2, selected: false }}, + { "att-s": "owl","value": { count: 2, selected: true }} + ]); + content.sharedModel?.dataSet.selectAllCases(); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "value": { count: 2, selected: true }}, + { "att-s": "owl","value": { count: 2, selected: true }} + ]); + + }); + it("sets first dataset attribute as the primary attribute by default", () => { const content = TestingBarGraphContentModel.create({ }); content.setSharedModel(sharedSampleDataSet()); @@ -138,6 +160,55 @@ Object { ]); }); + it("returns expected array when a case is selected with primary and secondary attributes", () => { + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); + content.setPrimaryAttribute("att-s"); + content.setSecondaryAttribute("att-l"); + content.sharedModel?.dataSet.setSelectedCases([content.sharedModel?.dataSet.cases[0].__id__]); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "yard": { count: 2, selected: true }}, + { "att-s": "owl", "yard": { count: 1, selected: false }, "forest": { count: 1, selected: false }} + ]); + content.sharedModel?.dataSet.setSelectedCases([content.sharedModel?.dataSet.cases[3].__id__]); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "yard": { count: 2, selected: false }}, + { "att-s": "owl", "yard": { count: 1, selected: false }, "forest": { count: 1, selected: true }} + ]); + content.sharedModel?.dataSet.selectAllCases(); + expect(content.dataArray).toEqual([ + { "att-s": "cat", "yard": { count: 2, selected: true }}, + { "att-s": "owl", "yard": { count: 1, selected: true }, "forest": { count: 1, selected: true }} + ]); + }); + + it("selects cases based on primary and secondary attributes", () => { + const content = TestingBarGraphContentModel.create({ }); + content.setSharedModel(sharedSampleDataSet()); + const dataSet = content.sharedModel?.dataSet; + expect(dataSet).toBeDefined(); + content.setPrimaryAttribute("att-s"); + content.setSecondaryAttribute("att-l"); + + content.selectCasesByValues("cat", undefined); + expect(dataSet?.selectedCaseIds.map(c => dataSet?.caseIndexFromID(c))).toEqual([0, 1]); + + content.selectCasesByValues("owl", undefined); + expect(dataSet?.selectedCaseIds.map(c => dataSet?.caseIndexFromID(c))).toEqual([2, 3]); + + content.selectCasesByValues("cat", "yard"); + expect(dataSet?.selectedCaseIds.map(c => dataSet?.caseIndexFromID(c))).toEqual([0, 1]); + + content.selectCasesByValues("owl", "yard"); + expect(dataSet?.selectedCaseIds.map(c => dataSet?.caseIndexFromID(c))).toEqual([2]); + + content.selectCasesByValues("owl", "forest"); + expect(dataSet?.selectedCaseIds.map(c => dataSet?.caseIndexFromID(c))).toEqual([3]); + + content.selectCasesByValues("cat", "forest"); + expect(dataSet?.selectedCaseIds.map(c => dataSet?.caseIndexFromID(c))).toEqual([]); + }); + it("fills in missing values with (no value)", () => { const content = TestingBarGraphContentModel.create({ }); const dataSet = sharedSampleDataSet(); diff --git a/src/plugins/bar-graph/chart-area.tsx b/src/plugins/bar-graph/chart-area.tsx index d0e9d89f2a..67364946dc 100644 --- a/src/plugins/bar-graph/chart-area.tsx +++ b/src/plugins/bar-graph/chart-area.tsx @@ -227,6 +227,7 @@ function BarHighlight({ x, y, width, height }: IBarHighlightProps) { width={width + 8} height={height + 8} fill="#14F49E" + className="bar-highlight" /> ); } From 8e73250f9601ab88b743d4983f3e37194b73782a Mon Sep 17 00:00:00 2001 From: Boris Goldowsky Date: Tue, 17 Sep 2024 15:48:43 -0400 Subject: [PATCH 35/36] Remove .only --- cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js index 530b77c360..b9accfff97 100644 --- a/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/bar_graph_tile_spec.js @@ -154,7 +154,7 @@ context('Bar Graph Tile', function () { } }); - it('Can link data ', function () { + it('Linking data', function () { beforeTest(); clueCanvas.addTile('bargraph'); @@ -335,7 +335,7 @@ context('Bar Graph Tile', function () { } }); - it.only('Synchronizes selection', function () { + it('Synchronizing selection', function () { beforeTest(); clueCanvas.addTile('bargraph'); From 2722e422e6b9b4fc0fded0b2e5b51b1e6b11db94 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Thu, 19 Sep 2024 10:04:17 -0400 Subject: [PATCH 36/36] Update firestore.rules Co-authored-by: Kirk Swenson --- firestore.rules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firestore.rules b/firestore.rules index 593a59c01d..da47f6fbc7 100644 --- a/firestore.rules +++ b/firestore.rules @@ -427,7 +427,7 @@ service cloud.firestore { } // In the future, developers might use a random unique key instead of - // the firebase user id. So this rule is relaxed inorder to allow that. + // the firebase user id. So this rule is relaxed in order to allow that. match /dev/{devId}/{restOfPath=**} { allow read, write: if isAuthed(); }