Merge pull request #1685

* [#188590950] Bug fix: Graph: crash on axis switch from categorical pa… * * This change fixes a cypress test that failed. The problem was that …
concord-consortium · Dec 16, 2024 · 1d68742 · 1d68742
1 parent 9cbd5b5
commit 1d68742
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 70 deletions.
diff --git a/v3/src/components/graph/adornments/count/count-adornment-component.tsx b/v3/src/components/graph/adornments/count/count-adornment-component.tsx
@@ -46,13 +46,13 @@ export const CountAdornment = observer(function CountAdornment(props: IAdornment
       // movable values present at the same time.
       if (graphModel.pointDisplayType === "bins") {
         const { binWidth, minBinEdge, maxBinEdge, totalNumberOfBins } = graphModel.binDetails()
-        return [
+        return binWidth !== undefined ? [
           // Build and spread an array of numeric values corresponding to the bin boundaries. Using totalNumberOfBins
           // for length, start at minBinEdge and increment by binWidth using each bin's index. Afterward, add
           // maxBinEdge to complete the region boundaries array.
           ...Array.from({ length: totalNumberOfBins }, (_, i) => minBinEdge + i * binWidth),
           maxBinEdge
-        ]
+        ] : []
       }
       return adornmentsStore?.subPlotRegionBoundaries(instanceKey, scale) ?? []
   }, [adornmentsStore, graphModel, instanceKey, scale])

diff --git a/v3/src/components/graph/components/binneddotplotdots.tsx b/v3/src/components/graph/components/binneddotplotdots.tsx
@@ -47,6 +47,8 @@ export const BinnedDotPlotDots = observer(function BinnedDotPlotDots(props: Plot
     const { binWidth, minBinEdge, totalNumberOfBins } = graphModel.binDetails()
     const binBoundariesArea = select(binBoundariesRef.current)
 
+    if (binWidth === undefined) return
+
     binBoundariesArea.selectAll("path").remove()
     for (let i = 1; i < totalNumberOfBins; i++) {
       const primaryBoundaryOrigin = primaryAxisScale(minBinEdge + i * binWidth)
@@ -82,9 +84,11 @@ export const BinnedDotPlotDots = observer(function BinnedDotPlotDots(props: Plot
     primaryAxisScaleCopy.current = primaryAxisScale.copy()
     graphModel.setDragBinIndex(binIndex)
     const { binWidth, minBinEdge } = graphModel.binDetails()
-    const newBinAlignment = minBinEdge + binIndex * binWidth
-    lowerBoundaryRef.current = primaryAxisScale(newBinAlignment)
-    graphModel.setDynamicBinAlignment(newBinAlignment)
+    if (binWidth !== undefined) {
+      const newBinAlignment = minBinEdge + binIndex * binWidth
+      lowerBoundaryRef.current = primaryAxisScale(newBinAlignment)
+      graphModel.setDynamicBinAlignment(newBinAlignment)
+    }
   }, [dataConfig, graphModel, primaryAxisScale])
 
   const handleDragBinBoundary = useCallback((event: MouseEvent) => {

diff --git a/v3/src/components/graph/components/histogram.tsx b/v3/src/components/graph/components/histogram.tsx
@@ -49,6 +49,8 @@ export const Histogram = observer(function Histogram({ abovePointsGroupRef, pixi
       numExtraPrimaryBands, pointDiameter, primaryAttrID, primaryAxisScale, primaryPlace, secondaryAttrID,
       secondaryBandwidth, totalNumberOfBins
     }
+    if (binWidth === undefined) return
+
     const { bins } = computeBinPlacements(binPlacementProps)
     const primaryScaleDiff = primaryAxisScale(binWidth) - primaryAxisScale(0)
     const getWidth = () => primaryIsBottom ? primaryScaleDiff / numExtraPrimaryBands : secondaryNumericUnitLength

diff --git a/v3/src/components/graph/hooks/use-dot-plot.ts b/v3/src/components/graph/hooks/use-dot-plot.ts
@@ -86,7 +86,7 @@ export const useDotPlot = (pixiPoints?: PixiPoints) => {
     const { primaryCoord, extraPrimaryCoord } = computePrimaryCoord(computePrimaryCoordProps)
     let primaryScreenCoord = primaryCoord + extraPrimaryCoord
 
-    if (graphModel.pointDisplayType !== "histogram") {
+    if (binWidth !== undefined && graphModel.pointDisplayType !== "histogram") {
       const caseValue = dataDisplayGetNumericValue(dataset, anID, primaryAttrID) ?? -1
       const binForCase = determineBinForCase(caseValue, binWidth, minBinEdge)
       primaryScreenCoord = adjustCoordForStacks({
@@ -111,7 +111,7 @@ export const useDotPlot = (pixiPoints?: PixiPoints) => {
     }
     let secondaryScreenCoord = computeSecondaryCoord(secondaryCoordProps)
 
-    if (graphModel.pointDisplayType !== "histogram") {
+    if (binWidth !== undefined && graphModel.pointDisplayType !== "histogram") {
       const onePixelOffset = primaryIsBottom ? -1 : 1
       const casePrimaryValue = dataDisplayGetNumericValue(dataset, anID, primaryAttrID) ?? -1
       const binForCase = determineBinForCase(casePrimaryValue, binWidth, minBinEdge)

diff --git a/v3/src/components/graph/models/graph-content-model.ts b/v3/src/components/graph/models/graph-content-model.ts
@@ -120,6 +120,13 @@ export const GraphContentModel = DataDisplayContentModel
     setPointOverlap(overlap: number) {
       self.pointOverlap = overlap
     },
+    setBinWidth(width: number | undefined) {
+      self._binWidth = isFiniteNumber(width) ? width : undefined
+      self.dynamicBinWidth = undefined
+    },
+    setDynamicBinWidth(width: number) {
+      self.dynamicBinWidth = width
+    },
   }))
   .views(self => ({
     get graphPointLayerModel(): IGraphPointLayerModel {
@@ -177,8 +184,8 @@ export const GraphContentModel = DataDisplayContentModel
         clampPosMinAtZero: self.pointDisplayType === "bars" || self.pointsFusedIntoBars
       }
     },
-    binWidthFromData(minValue: number, maxValue: number): number {
-      if (minValue === Infinity || maxValue === -Infinity) return 1
+    binWidthFromData(minValue: number, maxValue: number) {
+      if (minValue === Infinity || maxValue === -Infinity || minValue === maxValue) return undefined
       const kDefaultNumberOfBins = 4
 
       const binRange = maxValue !== minValue
@@ -217,26 +224,24 @@ export const GraphContentModel = DataDisplayContentModel
       const maxValue = caseDataArray.reduce((max, aCaseData) => {
         return Math.max(max, dataDisplayGetNumericValue(dataset, aCaseData.caseID, primaryAttributeID) ?? max)
       }, -Infinity)
-
-      if (minValue === Infinity || maxValue === -Infinity) {
-        return { binAlignment: 0, binWidth: 1, minBinEdge: 0, maxBinEdge: 0, minValue: 0, maxValue: 0,
-                 totalNumberOfBins: 0 }
+      const binWidth = (initialize || !self.binWidth)
+        ? self.binWidthFromData(minValue, maxValue) : self.binWidth
+      if (minValue === Infinity || maxValue === -Infinity || binWidth === undefined) {
+        return { binAlignment: 0, binWidth: undefined, minBinEdge: 0, maxBinEdge: 0, minValue: 0, maxValue: 0,
+          totalNumberOfBins: 0 }
       }
 
-      const binWidth = initialize || !self.binWidth
-                         ? self.binWidthFromData(minValue, maxValue)
-                         : self.binWidth
       const binAlignment = initialize || !self.binAlignment
-                             ? Math.floor(minValue / binWidth) * binWidth
-                             : self.binAlignment
+        ? Math.floor(minValue / binWidth) * binWidth
+        : self.binAlignment
       const minBinEdge = binAlignment - Math.ceil((binAlignment - minValue) / binWidth) * binWidth
       // Calculate the total number of bins needed to cover the range from the minimum data value
       // to the maximum data value, adding a small constant to ensure the max value is contained.
       const totalNumberOfBins = Math.ceil((maxValue - minBinEdge) / binWidth + 0.000001)
       const maxBinEdge = minBinEdge + (totalNumberOfBins * binWidth)
 
       return { binAlignment, binWidth, minBinEdge, maxBinEdge, minValue, maxValue, totalNumberOfBins }
-    }
+    },
   }))
   .actions(self => ({
     afterCreate() {
@@ -294,34 +299,28 @@ export const GraphContentModel = DataDisplayContentModel
     setDynamicBinAlignment(alignment: number) {
       self.dynamicBinAlignment = alignment
     },
-    setBinWidth(width: number) {
-      self._binWidth = isFiniteNumber(width) ? width : undefined
-      self.dynamicBinWidth = undefined
-    },
-    setDynamicBinWidth(width: number) {
-      self.dynamicBinWidth = width
-    },
     binnedAxisTicks(formatter?: (value: number) => string): { tickValues: number[], tickLabels: string[] } {
       const tickValues: number[] = []
       const tickLabels: string[] = []
       const { binWidth, totalNumberOfBins, minBinEdge } = self.binDetails()
+      if (binWidth !== undefined) {
+        let currentStart = minBinEdge
+        let binCount = 0
 
-      let currentStart = minBinEdge
-      let binCount = 0
-
-      while (binCount < totalNumberOfBins) {
-        const currentEnd = currentStart + binWidth
-        if (formatter) {
-          const formattedCurrentStart = formatter(currentStart)
-          const formattedCurrentEnd = formatter(currentEnd)
-          tickValues.push(currentStart + (binWidth / 2))
-          tickLabels.push(`[${formattedCurrentStart}, ${formattedCurrentEnd})`)
-        } else {
-          tickValues.push(currentStart + binWidth)
-          tickLabels.push(`${currentEnd}`)
+        while (binCount < totalNumberOfBins) {
+          const currentEnd = currentStart + binWidth
+          if (formatter) {
+            const formattedCurrentStart = formatter(currentStart)
+            const formattedCurrentEnd = formatter(currentEnd)
+            tickValues.push(currentStart + (binWidth / 2))
+            tickLabels.push(`[${formattedCurrentStart}, ${formattedCurrentEnd})`)
+          } else {
+            tickValues.push(currentStart + binWidth)
+            tickLabels.push(`${currentEnd}`)
+          }
+          currentStart += binWidth
+          binCount++
         }
-        currentStart += binWidth
-        binCount++
       }
       return { tickValues, tickLabels }
     },
@@ -343,17 +342,19 @@ export const GraphContentModel = DataDisplayContentModel
       const tickLabels: string[] = []
       const { binWidth, totalNumberOfBins, minBinEdge } = self.binDetails()
 
-      let currentStart = minBinEdge
-      let binCount = 0
+      if (binWidth !== undefined) {
+        let currentStart = minBinEdge
+        let binCount = 0
 
-      while (binCount < totalNumberOfBins) {
-        const currentEnd = currentStart + binWidth
-        const formattedCurrentStart = formatter(currentStart)
-        const formattedCurrentEnd = formatter(currentEnd)
-        tickValues.push(currentStart + (binWidth / 2))
-        tickLabels.push(`[${formattedCurrentStart}, ${formattedCurrentEnd})`)
-        currentStart += binWidth
-        binCount++
+        while (binCount < totalNumberOfBins) {
+          const currentEnd = currentStart + binWidth
+          const formattedCurrentStart = formatter(currentStart)
+          const formattedCurrentEnd = formatter(currentEnd)
+          tickValues.push(currentStart + (binWidth / 2))
+          tickLabels.push(`[${formattedCurrentStart}, ${formattedCurrentEnd})`)
+          currentStart += binWidth
+          binCount++
+        }
       }
       return { tickValues, tickLabels }
     },
@@ -374,12 +375,14 @@ export const GraphContentModel = DataDisplayContentModel
 
       if (self.pointDisplayType === "histogram") {
         const { binWidth, minBinEdge } = self.binDetails()
-        const binIndex = Math.floor((Number(value) - minBinEdge) / binWidth)
-        matchingCases = allCases?.filter(aCase => {
-          const caseValue = dataDisplayGetNumericValue(dataset, aCase.__id__, attrID) ?? 0
-          const bin = Math.floor((caseValue - minBinEdge) / binWidth)
-          return bin === binIndex
-        }) as ICase[] ?? []
+        if (binWidth !== undefined) {
+          const binIndex = Math.floor((Number(value) - minBinEdge) / binWidth)
+          matchingCases = allCases?.filter(aCase => {
+            const caseValue = dataDisplayGetNumericValue(dataset, aCase.__id__, attrID) ?? 0
+            const bin = Math.floor((caseValue - minBinEdge) / binWidth)
+            return bin === binIndex
+          }) as ICase[] ?? []
+        }
       } else if (attrID && value) {
         matchingCases = allCases?.filter(aCase => dataset?.getStrValue(aCase.__id__, attrID) === value) as ICase[] ?? []
       }
@@ -425,18 +428,20 @@ export const GraphContentModel = DataDisplayContentModel
           return true
         })
         const { binWidth, minBinEdge } = self.binDetails()
-        const binIndex = Math.floor((Number(casePrimaryValue) - minBinEdge) / binWidth)
-        const firstCount = allMatchingCases.length
-        const secondCount = casesInSubPlot.length
-        const percent = float(100 * firstCount / secondCount)
-        const minBinValue = minBinEdge + binIndex * binWidth
-        const maxBinValue = minBinEdge + (binIndex + 1) * binWidth
-        // "<n> of <total> (<p>%) are ≥ L and < U"
-        const attrArray = [ firstCount, secondCount, percent, minBinValue, maxBinValue ]
-        const translationKey = firstCount === 1
-                                 ? "DG.HistogramView.barTipNoLegendSingular"
-                                 : "DG.HistogramView.barTipNoLegendPlural"
-        tipText = t(translationKey, {vars: attrArray})
+        if (binWidth !== undefined) {
+          const binIndex = Math.floor((Number(casePrimaryValue) - minBinEdge) / binWidth)
+          const firstCount = allMatchingCases.length
+          const secondCount = casesInSubPlot.length
+          const percent = float(100 * firstCount / secondCount)
+          const minBinValue = minBinEdge + binIndex * binWidth
+          const maxBinValue = minBinEdge + (binIndex + 1) * binWidth
+          // "<n> of <total> (<p>%) are ≥ L and < U"
+          const attrArray = [firstCount, secondCount, percent, minBinValue, maxBinValue]
+          const translationKey = firstCount === 1
+            ? "DG.HistogramView.barTipNoLegendSingular"
+            : "DG.HistogramView.barTipNoLegendPlural"
+          tipText = t(translationKey, {vars: attrArray})
+        }
       } else {
         const topSplitMatches = self.matchingCasesForAttr(topSplitAttrID, caseTopSplitValue)
         const rightSplitMatches = self.matchingCasesForAttr(rightSplitAttrID, caseRightSplitValue)
@@ -645,7 +650,7 @@ export const GraphContentModel = DataDisplayContentModel
       const secondaryPlace = secondaryRole === "y" ? "left" : "bottom"
       const extraPrimAttrRole = primaryRole === "x" ? "topSplit" : "rightSplit"
       const extraSecAttrRole = primaryRole === "x" ? "rightSplit" : "topSplit"
-      const maxCellCaseCount = self.pointDisplayType === "histogram"
+      const maxCellCaseCount = (self.pointDisplayType === "histogram" && binWidth !== undefined)
         ? maxCellLength(extraPrimAttrRole, extraSecAttrRole, binWidth, minValue, totalNumberOfBins)
         : maxOverAllCells(extraPrimAttrRole, extraSecAttrRole)
       const countAxis = NumericAxisModel.create({