From d0abddfc8a08539bd074bc197ccdff839f874920 Mon Sep 17 00:00:00 2001 From: Ethan McElroy Date: Thu, 12 Oct 2023 17:03:35 -0400 Subject: [PATCH 1/3] fix: Adornments not always rendered for all subplots (PT-186164083) PT Story: https://www.pivotaltracker.com/story/show/186164083 --- .../graph/adornments/adornment-models.ts | 16 ++++++++++++---- .../univariate-measure-adornment-model.ts | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/v3/src/components/graph/adornments/adornment-models.ts b/v3/src/components/graph/adornments/adornment-models.ts index e056a6cc80..83748e66cb 100644 --- a/v3/src/components/graph/adornments/adornment-models.ts +++ b/v3/src/components/graph/adornments/adornment-models.ts @@ -76,18 +76,23 @@ export const AdornmentModel = types.model("AdornmentModel", { const { xAttrId, xCats, yAttrId, yCats, topAttrId, topCats, rightAttrId, rightCats } = options const topCatCount = topCats.length || 1 const rightCatCount = rightCats.length || 1 - const xCatCount = xCats.length || 1 const yCatCount = yCats.length || 1 + const xCatCount = xCats.length || 1 const columnCount = topCatCount * xCatCount const rowCount = rightCatCount * yCatCount const cellKey: Record = {} + + // Determine which categories are associated with the cell's axes using the provided index value and + // the attributes and categories present in the graph. const topIndex = xCatCount > 0 ? Math.floor(index / xCatCount) % topCatCount : index % topCatCount const topCat = topCats[topIndex] - const rightIndex = yCatCount > 0 - ? Math.floor(index / yCatCount) % rightCats.length - : index % rightCats.length + const rightIndex = topCatCount > 0 && yCatCount > 1 + ? Math.floor(index / (topCatCount * yCatCount)) % rightCatCount + : yCatCount > 0 + ? Math.floor(index / yCatCount) % rightCatCount + : index % rightCatCount const rightCat = rightCats[rightIndex] const yCat = topCats.length > 0 ? yCats[Math.floor(index / columnCount) % yCatCount] @@ -95,10 +100,13 @@ export const AdornmentModel = types.model("AdornmentModel", { const xCat = rightCats.length > 0 ? xCats[Math.floor(index / rowCount) % xCatCount] : xCats[index % xCats.length] + + // Assign the categories determined above to the associated properties of the cellKey object. if (topAttrId) cellKey[topAttrId] = topCat if (rightAttrId) cellKey[rightAttrId] = rightCat if (yAttrId && yCats[0]) cellKey[yAttrId] = yCat if (xAttrId && xCats[0]) cellKey[xAttrId] = xCat + return cellKey } })) diff --git a/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts b/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts index ba07bec0c0..2d62887727 100644 --- a/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts +++ b/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts @@ -101,7 +101,7 @@ export const UnivariateMeasureAdornmentModel = AdornmentModel const attrId = dataConfig.primaryAttributeID for (let i = 0; i < totalCount; ++i) { const cellKey = self.setCellKey(options, i) - const instanceKey = self.instanceKey(cellKey) + const instanceKey = self.instanceKey(cellKey) const value = Number(self.computeMeasureValue(attrId, cellKey, dataConfig)) if (!self.measures.get(instanceKey) || resetPoints) { self.addMeasure(value, instanceKey) From 192c7a5989891257101eb22a7924c18c19de35ff Mon Sep 17 00:00:00 2001 From: Ethan McElroy Date: Thu, 12 Oct 2023 17:30:59 -0400 Subject: [PATCH 2/3] chore: Make setCellKey a view and adjust name --- .../graph/adornments/adornment-models.test.ts | 2 +- .../graph/adornments/adornment-models.ts | 18 +++++++++--------- .../graph/adornments/adornments-store.test.ts | 2 +- .../movable-line-adornment-model.ts | 2 +- .../movable-point-adornment-model.ts | 2 +- .../movable-value-adornment-model.ts | 2 +- .../univariate-measure-adornment-model.ts | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/v3/src/components/graph/adornments/adornment-models.test.ts b/v3/src/components/graph/adornments/adornment-models.test.ts index 9f784be5e3..b7c47fb678 100644 --- a/v3/src/components/graph/adornments/adornment-models.test.ts +++ b/v3/src/components/graph/adornments/adornment-models.test.ts @@ -52,7 +52,7 @@ describe("AdornmentModel", () => { rightCats: ["new", "used"] } const adornment = AdornmentModel.create({type: "Movable Line"}) - const cellKey = adornment.setCellKey(options, 0) + const cellKey = adornment.generateCellKey(options, 0) expect(cellKey).toEqual({abc123: "pizza", def456: "red", ghi789: "small", jkl012: "new"}) }) it("will create an instance key value from given category values", () => { diff --git a/v3/src/components/graph/adornments/adornment-models.ts b/v3/src/components/graph/adornments/adornment-models.ts index 83748e66cb..0dd7db5805 100644 --- a/v3/src/components/graph/adornments/adornment-models.ts +++ b/v3/src/components/graph/adornments/adornment-models.ts @@ -63,16 +63,8 @@ export const AdornmentModel = types.model("AdornmentModel", { }, get isUnivariateMeasure() { return false - } - })) - .actions(self => ({ - setVisibility(isVisible: boolean) { - self.isVisible = isVisible - }, - updateCategories(options: IUpdateCategoriesOptions) { - // derived models should override to update their models when categories change }, - setCellKey(options: IUpdateCategoriesOptions, index: number) { + generateCellKey(options: IUpdateCategoriesOptions, index: number) { const { xAttrId, xCats, yAttrId, yCats, topAttrId, topCats, rightAttrId, rightCats } = options const topCatCount = topCats.length || 1 const rightCatCount = rightCats.length || 1 @@ -110,6 +102,14 @@ export const AdornmentModel = types.model("AdornmentModel", { return cellKey } })) + .actions(self => ({ + setVisibility(isVisible: boolean) { + self.isVisible = isVisible + }, + updateCategories(options: IUpdateCategoriesOptions) { + // derived models should override to update their models when categories change + } + })) export interface IAdornmentModel extends Instance {} export const UnknownAdornmentModel = AdornmentModel diff --git a/v3/src/components/graph/adornments/adornments-store.test.ts b/v3/src/components/graph/adornments/adornments-store.test.ts index 15429f4ce2..5f9dc520fc 100644 --- a/v3/src/components/graph/adornments/adornments-store.test.ts +++ b/v3/src/components/graph/adornments/adornments-store.test.ts @@ -15,7 +15,7 @@ const mockAdornment = { instanceKey: () => "mock-count-adornment", isUnivariateMeasure: false, isVisible: false, - setCellKey: () => ({}), + generateCellKey: () => ({}), setVisibility: () => true, updateCategories: () => ({}), type: "Mock Adornment" diff --git a/v3/src/components/graph/adornments/movable-line/movable-line-adornment-model.ts b/v3/src/components/graph/adornments/movable-line/movable-line-adornment-model.ts index 5183f313a7..09fbefee8f 100644 --- a/v3/src/components/graph/adornments/movable-line/movable-line-adornment-model.ts +++ b/v3/src/components/graph/adornments/movable-line/movable-line-adornment-model.ts @@ -56,7 +56,7 @@ export const MovableLineAdornmentModel = AdornmentModel const rowCount = rightCats?.length || 1 const totalCount = rowCount * columnCount for (let i = 0; i < totalCount; ++i) { - const cellKey = self.setCellKey(options, i) + const cellKey = self.generateCellKey(options, i) const instanceKey = self.instanceKey(cellKey) if (!self.lines.get(instanceKey) || resetPoints) { self.setInitialLine(xAxis, yAxis, instanceKey) diff --git a/v3/src/components/graph/adornments/movable-point/movable-point-adornment-model.ts b/v3/src/components/graph/adornments/movable-point/movable-point-adornment-model.ts index 8decc85a0d..aa07487a12 100644 --- a/v3/src/components/graph/adornments/movable-point/movable-point-adornment-model.ts +++ b/v3/src/components/graph/adornments/movable-point/movable-point-adornment-model.ts @@ -35,7 +35,7 @@ export const MovablePointAdornmentModel = AdornmentModel const rowCount = rightCats?.length || 1 const totalCount = rowCount * columnCount for (let i = 0; i < totalCount; ++i) { - const cellKey = self.setCellKey(options, i) + const cellKey = self.generateCellKey(options, i) const instanceKey = self.instanceKey(cellKey) if (!self.points.get(instanceKey) || resetPoints) { self.setInitialPoint(xAxis, yAxis, instanceKey) diff --git a/v3/src/components/graph/adornments/movable-value/movable-value-adornment-model.ts b/v3/src/components/graph/adornments/movable-value/movable-value-adornment-model.ts index 289f4e9cab..4bd3c1df77 100644 --- a/v3/src/components/graph/adornments/movable-value/movable-value-adornment-model.ts +++ b/v3/src/components/graph/adornments/movable-value/movable-value-adornment-model.ts @@ -130,7 +130,7 @@ export const MovableValueAdornmentModel = AdornmentModel self.setAxisMax(axisMax) for (let i = 0; i < totalCount; ++i) { - const subPlotKey = self.setCellKey(options, i) + const subPlotKey = self.generateCellKey(options, i) const instanceKey = self.instanceKey(subPlotKey) // Each array in the model's values map should have the same length as all the others. If there are no existing // values for the current instance key, check if there is at least one array in the map. If there is, copy those diff --git a/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts b/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts index 2d62887727..3124370e16 100644 --- a/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts +++ b/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts @@ -100,7 +100,7 @@ export const UnivariateMeasureAdornmentModel = AdornmentModel const totalCount = rowCount * columnCount const attrId = dataConfig.primaryAttributeID for (let i = 0; i < totalCount; ++i) { - const cellKey = self.setCellKey(options, i) + const cellKey = self.generateCellKey(options, i) const instanceKey = self.instanceKey(cellKey) const value = Number(self.computeMeasureValue(attrId, cellKey, dataConfig)) if (!self.measures.get(instanceKey) || resetPoints) { From 9dca574598084f999009c2ff3c0ae91f3f1d1aa1 Mon Sep 17 00:00:00 2001 From: Ethan McElroy Date: Fri, 13 Oct 2023 11:55:48 -0400 Subject: [PATCH 3/3] chore: rename view that returns cellKey --- v3/src/components/graph/adornments/adornment-models.test.ts | 2 +- v3/src/components/graph/adornments/adornment-models.ts | 2 +- v3/src/components/graph/adornments/adornments-store.test.ts | 2 +- .../adornments/movable-line/movable-line-adornment-model.ts | 2 +- .../adornments/movable-point/movable-point-adornment-model.ts | 2 +- .../adornments/movable-value/movable-value-adornment-model.ts | 2 +- .../univariate-measures/univariate-measure-adornment-model.ts | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/v3/src/components/graph/adornments/adornment-models.test.ts b/v3/src/components/graph/adornments/adornment-models.test.ts index b7c47fb678..827c59d9cb 100644 --- a/v3/src/components/graph/adornments/adornment-models.test.ts +++ b/v3/src/components/graph/adornments/adornment-models.test.ts @@ -52,7 +52,7 @@ describe("AdornmentModel", () => { rightCats: ["new", "used"] } const adornment = AdornmentModel.create({type: "Movable Line"}) - const cellKey = adornment.generateCellKey(options, 0) + const cellKey = adornment.cellKey(options, 0) expect(cellKey).toEqual({abc123: "pizza", def456: "red", ghi789: "small", jkl012: "new"}) }) it("will create an instance key value from given category values", () => { diff --git a/v3/src/components/graph/adornments/adornment-models.ts b/v3/src/components/graph/adornments/adornment-models.ts index 0dd7db5805..c72ae53d66 100644 --- a/v3/src/components/graph/adornments/adornment-models.ts +++ b/v3/src/components/graph/adornments/adornment-models.ts @@ -64,7 +64,7 @@ export const AdornmentModel = types.model("AdornmentModel", { get isUnivariateMeasure() { return false }, - generateCellKey(options: IUpdateCategoriesOptions, index: number) { + cellKey(options: IUpdateCategoriesOptions, index: number) { const { xAttrId, xCats, yAttrId, yCats, topAttrId, topCats, rightAttrId, rightCats } = options const topCatCount = topCats.length || 1 const rightCatCount = rightCats.length || 1 diff --git a/v3/src/components/graph/adornments/adornments-store.test.ts b/v3/src/components/graph/adornments/adornments-store.test.ts index 5f9dc520fc..9c28cb11fc 100644 --- a/v3/src/components/graph/adornments/adornments-store.test.ts +++ b/v3/src/components/graph/adornments/adornments-store.test.ts @@ -15,7 +15,7 @@ const mockAdornment = { instanceKey: () => "mock-count-adornment", isUnivariateMeasure: false, isVisible: false, - generateCellKey: () => ({}), + cellKey: () => ({}), setVisibility: () => true, updateCategories: () => ({}), type: "Mock Adornment" diff --git a/v3/src/components/graph/adornments/movable-line/movable-line-adornment-model.ts b/v3/src/components/graph/adornments/movable-line/movable-line-adornment-model.ts index 09fbefee8f..ae54711cc4 100644 --- a/v3/src/components/graph/adornments/movable-line/movable-line-adornment-model.ts +++ b/v3/src/components/graph/adornments/movable-line/movable-line-adornment-model.ts @@ -56,7 +56,7 @@ export const MovableLineAdornmentModel = AdornmentModel const rowCount = rightCats?.length || 1 const totalCount = rowCount * columnCount for (let i = 0; i < totalCount; ++i) { - const cellKey = self.generateCellKey(options, i) + const cellKey = self.cellKey(options, i) const instanceKey = self.instanceKey(cellKey) if (!self.lines.get(instanceKey) || resetPoints) { self.setInitialLine(xAxis, yAxis, instanceKey) diff --git a/v3/src/components/graph/adornments/movable-point/movable-point-adornment-model.ts b/v3/src/components/graph/adornments/movable-point/movable-point-adornment-model.ts index aa07487a12..4c912d2b34 100644 --- a/v3/src/components/graph/adornments/movable-point/movable-point-adornment-model.ts +++ b/v3/src/components/graph/adornments/movable-point/movable-point-adornment-model.ts @@ -35,7 +35,7 @@ export const MovablePointAdornmentModel = AdornmentModel const rowCount = rightCats?.length || 1 const totalCount = rowCount * columnCount for (let i = 0; i < totalCount; ++i) { - const cellKey = self.generateCellKey(options, i) + const cellKey = self.cellKey(options, i) const instanceKey = self.instanceKey(cellKey) if (!self.points.get(instanceKey) || resetPoints) { self.setInitialPoint(xAxis, yAxis, instanceKey) diff --git a/v3/src/components/graph/adornments/movable-value/movable-value-adornment-model.ts b/v3/src/components/graph/adornments/movable-value/movable-value-adornment-model.ts index 4bd3c1df77..dd5e360514 100644 --- a/v3/src/components/graph/adornments/movable-value/movable-value-adornment-model.ts +++ b/v3/src/components/graph/adornments/movable-value/movable-value-adornment-model.ts @@ -130,7 +130,7 @@ export const MovableValueAdornmentModel = AdornmentModel self.setAxisMax(axisMax) for (let i = 0; i < totalCount; ++i) { - const subPlotKey = self.generateCellKey(options, i) + const subPlotKey = self.cellKey(options, i) const instanceKey = self.instanceKey(subPlotKey) // Each array in the model's values map should have the same length as all the others. If there are no existing // values for the current instance key, check if there is at least one array in the map. If there is, copy those diff --git a/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts b/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts index 3124370e16..9c65e5e786 100644 --- a/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts +++ b/v3/src/components/graph/adornments/univariate-measures/univariate-measure-adornment-model.ts @@ -100,7 +100,7 @@ export const UnivariateMeasureAdornmentModel = AdornmentModel const totalCount = rowCount * columnCount const attrId = dataConfig.primaryAttributeID for (let i = 0; i < totalCount; ++i) { - const cellKey = self.generateCellKey(options, i) + const cellKey = self.cellKey(options, i) const instanceKey = self.instanceKey(cellKey) const value = Number(self.computeMeasureValue(attrId, cellKey, dataConfig)) if (!self.measures.get(instanceKey) || resetPoints) {