From c7a6a6f6bcea6ccccc3d5306b43a32cabeb06fbf Mon Sep 17 00:00:00 2001 From: Kirk Swenson Date: Tue, 20 Aug 2024 17:01:43 -0700 Subject: [PATCH] fix: graph response to collection hierarchy change (#1418) --- v3/cypress/e2e/hierarchical-table.spec.ts | 4 +--- v3/cypress/support/elements/table-tile.ts | 1 - .../components/graph/components/casedots.tsx | 4 ++-- v3/src/components/graph/components/graph.tsx | 12 ++++++++++++ v3/src/models/data/collection.ts | 18 +++++++++++++----- v3/src/models/data/data-set.ts | 8 ++++++++ 6 files changed, 36 insertions(+), 11 deletions(-) diff --git a/v3/cypress/e2e/hierarchical-table.spec.ts b/v3/cypress/e2e/hierarchical-table.spec.ts index a48464fbc6..e403cd26d2 100644 --- a/v3/cypress/e2e/hierarchical-table.spec.ts +++ b/v3/cypress/e2e/hierarchical-table.spec.ts @@ -9,7 +9,7 @@ context("hierarchical collections", () => { const queryParams = "?sample=mammals&dashboard&mouseSensor" const url = `${Cypress.config("index")}${queryParams}` cy.visit(url) - cy.wait(2500) + cy.wait(1000) }) hierarchical.tests.forEach((h: HierarchicalTest) => { // FIXME: enable skipped tests @@ -23,13 +23,11 @@ context("hierarchical collections", () => { table.moveAttributeToParent(attribute.name, attribute.move) table.getColumnHeaders(collection.index+1).should("not.contain", attribute.name) table.getAttribute(attribute.name, collection.index).should("have.text", attribute.name) - cy.wait(2000) }) table.getCollectionTitle(collection.index).should("have.text", collection.name) table.getColumnHeaders(collection.index).should("have.length", collection.attributes.length+1) table.getNumOfRows().should("contain", collection.cases+2) // +1 for the header row, +1 for input row table.verifyAttributeValues(collection.attributes, values, collection.index) - cy.wait(2000) cy.log("Testing expanding/collapsing...") table.verifyCollapseAllGroupsButton(collection.index+1) diff --git a/v3/cypress/support/elements/table-tile.ts b/v3/cypress/support/elements/table-tile.ts index 3de9d17db9..4017d5b1af 100644 --- a/v3/cypress/support/elements/table-tile.ts +++ b/v3/cypress/support/elements/table-tile.ts @@ -281,7 +281,6 @@ export const TableTileElements = { attributes.forEach(a => { const attribute = a.name for (let rowIndex = 0; rowIndex < values[attribute].length; rowIndex++) { - cy.wait(1000) this.getAttributeValue(attribute, rowIndex+2, collectionIndex).then(cell => { expect(values[attribute]).to.include(cell.text()) }) diff --git a/v3/src/components/graph/components/casedots.tsx b/v3/src/components/graph/components/casedots.tsx index ef3bfba767..9dab0fac0c 100644 --- a/v3/src/components/graph/components/casedots.tsx +++ b/v3/src/components/graph/components/casedots.tsx @@ -85,10 +85,10 @@ export const CaseDots = function CaseDots(props: { xLength = layout.getAxisMultiScale('bottom')?.length ?? 0, yLength = layout.getAxisMultiScale('left')?.length ?? 0, getScreenX = (anID: string) => { - return pointRadius + randomPointsRef.current[anID].x * (xLength - 2 * pointRadius) + return pointRadius + randomPointsRef.current[anID]?.x * (xLength - 2 * pointRadius) }, getScreenY = (anID: string) => { - return yLength - (pointRadius + randomPointsRef.current[anID].y * (yLength - 2 * pointRadius)) + return yLength - (pointRadius + randomPointsRef.current[anID]?.y * (yLength - 2 * pointRadius)) }, getLegendColor = dataConfiguration?.attributeID('legend') ? dataConfiguration?.getLegendColorForCase : undefined diff --git a/v3/src/components/graph/components/graph.tsx b/v3/src/components/graph/components/graph.tsx index a0c132681d..dfadaacb1a 100644 --- a/v3/src/components/graph/components/graph.tsx +++ b/v3/src/components/graph/components/graph.tsx @@ -123,6 +123,18 @@ export const Graph = observer(function Graph({graphController, graphRef, pixiPoi {name: "Graph.handleAttributeConfigurationChange"}, graphModel) }, [graphController, graphModel]) + // Respond to collection addition/removal. Note that this can fire in addition to the collection + // map changes reaction below, but that fires too early, so this gives the graph another chance. + useEffect(() => { + return dataset && mstReaction( + () => dataset.syncCollectionLinksCount, + () => { + graphModel.dataConfiguration._updateFilteredCasesCollectionID() + graphModel.dataConfiguration._invalidateCases() + graphController.callMatchCirclesToData() + }, { name: "Graph.mstReaction [syncCollectionLinksCount]" }, dataset) + }, [dataset, graphController, graphModel.dataConfiguration]) + useEffect(function handleAttributeCollectionMapChange() { const constructAttrCollections = () => { diff --git a/v3/src/models/data/collection.ts b/v3/src/models/data/collection.ts index c8d9663022..90eb2b9d3e 100644 --- a/v3/src/models/data/collection.ts +++ b/v3/src/models/data/collection.ts @@ -59,10 +59,10 @@ export const CollectionModel = V2Model caseGroupMap: new Map() })) .actions(self => ({ - setParent(parent: ICollectionModel) { + setParent(parent?: ICollectionModel) { self.parent = parent }, - setChild(child: ICollectionModel) { + setChild(child?: ICollectionModel) { self.child = child }, setItemData(itemData: IItemData) { @@ -323,12 +323,14 @@ export const CollectionModel = V2Model self.groupKeyCaseIds = new Map(self._groupKeyCaseIds) } - // changes to this collection's attributes invalidate grouping and persistent ids + // changes to a parent collection's attributes invalidate grouping and persistent ids addDisposer(self, reaction( () => self.sortedDataAttributes.map(attr => attr.id), () => { - self.groupKeyCaseIds.clear() - if (self.child) self.itemData.invalidate() + if (self.child) { + self.groupKeyCaseIds.clear() + self.itemData.invalidate() + } }, { name: "CollectionModel.sortedDataAttributes reaction", equals: comparer.structural } )) }, @@ -371,12 +373,18 @@ export function isCollectionModel(model?: IAnyStateTreeNode): model is ICollecti export function syncCollectionLinks(collections: ICollectionModel[], itemData: IItemData) { collections.forEach((collection, index) => { + if (index === 0) { + collection.setParent() + } if (index > 0) { collection.setParent(collections[index - 1]) } if (index < collections.length - 1) { collection.setChild(collections[index + 1]) } + if (index === collections.length - 1) { + collection.setChild() + } collection.setItemData(itemData) }) } diff --git a/v3/src/models/data/data-set.ts b/v3/src/models/data/data-set.ts index b54efba612..e926be6e4d 100644 --- a/v3/src/models/data/data-set.ts +++ b/v3/src/models/data/data-set.ts @@ -151,6 +151,8 @@ export const DataSet = V2Model.named("DataSet").props({ caseInfoMap: new Map(), // map from item ID to the child case containing it itemIdChildCaseMap: new Map(), + // incremented when collection parent/child links are updated + syncCollectionLinksCount: 0, transactionCount: 0, // the id of the interactive frame handling this dataset // used by the Collaborative plugin @@ -332,6 +334,11 @@ export const DataSet = V2Model.named("DataSet").props({ } } })) +.actions(self => ({ + incSyncCollectionLinksCount() { + ++self.syncCollectionLinksCount + } +})) .extend(self => { function getCollection(collectionId?: string): ICollectionModel | undefined { if (!isAlive(self)) { @@ -1101,6 +1108,7 @@ export const DataSet = V2Model.named("DataSet").props({ invalidate: () => self.invalidateCases() } syncCollectionLinks(self.collections, itemData) + self.incSyncCollectionLinksCount() self.invalidateCases() }, { name: "DataSet.collections", equals: comparer.structural, fireImmediately: true }