fix: use correct dimensions and hoverable points for Band Plot and un…

…tangle unnecessary code (#385)

* fix: use correct dimensions and hoverable points for Band Plot and untangle unnecessary code

* feat: memoize the calculation of hoverable points in Band Plot

* feat: remove rendering of "upper" and "lower" that are orphans without a "main"

* fix: ensure "upper" and "lower" exists before creating hover point indices for them

* refactor: use destructuring and short circuit

* refactor: destructure the Float64Array

* chore: add comment clarifying what gets rendered

giraffe/src/components/BandLayer.tsx

@@ -7,13 +7,9 @@ import {simplifyLineData} from '../utils/lineData'
 import {useCanvas} from '../utils/useCanvas'
 import {drawBands} from '../utils/drawBands'
 import {useHoverPointIndices} from '../utils/useHoverPointIndices'
-import {isDefined} from '../utils/isDefined'
+import {useBandHoverColumns} from '../utils/useBandHoverColumns'
 import {FILL} from '../constants/columnKeys'
-import {
-  getBandHoverIndices,
-  getLineLengths,
-  getBandBoundaries,
-} from '../utils/getBandHoverIndices'
+import {getBandHoverIndices, getLineLengths} from '../utils/getBandHoverIndices'
 import {
   groupLineIndicesIntoBands,
   alignMinMaxWithBand,
@@ -47,15 +43,12 @@ export const BandLayer: FunctionComponent<Props> = props => {
   )
 
   const drawBandsOptions = {
-    fill: spec.columnGroupMaps.fill,
+    bandIndexMap: spec.bandIndexMap,
     interpolation: config.interpolation,
     lineData: simplifiedLineData,
     lineWidth: config.lineWidth,
     lineOpacity: config.lineOpacity,
-    lowerColumnName,
-    rowColumnName,
     shadeOpacity: config.shadeOpacity,
-    upperColumnName,
   }
 
   const canvasRef = useRef<HTMLCanvasElement>(null)
@@ -79,61 +72,25 @@ export const BandLayer: FunctionComponent<Props> = props => {
     hoverDimension = config.hoverDimension
   }
 
-  const groupColData = spec.table.getColumn(FILL, 'number')
-  const hoverXYColumnData = {
-    xs: [],
-    ys: [],
-    groupColData: [],
-  }
-  const {rowIndices} = spec.bandIndexMap
-
-  rowIndices.forEach(rowIndex => {
-    if (isDefined(rowIndex)) {
-      hoverXYColumnData.xs = hoverXYColumnData.xs.concat(
-        spec.lineData[rowIndex].xs
-      )
-      hoverXYColumnData.ys = hoverXYColumnData.ys.concat(
-        spec.lineData[rowIndex].ys
-      )
-      hoverXYColumnData.groupColData = hoverXYColumnData.groupColData.concat(
-        ...groupColData.filter(index => index === rowIndex)
-      )
-    }
-  })
-
-  // Get the min and max indices of the corresponding hovered line(s)
-  //   by using an 'x' dimension hover
-  const hoverAsXIndices = useHoverPointIndices(
-    'x',
+  // Band Plot allows hovering on the nearest band or bands,
+  // and any hoverable point should be associate with a band
+  const hoverableColumnData = useBandHoverColumns(
     hoverX,
     hoverY,
-    spec.table.getColumn(config.x, 'number'),
-    spec.table.getColumn(config.y, 'number'),
+    spec.lineData,
     spec.table.getColumn(FILL, 'number'),
-    xScale,
-    yScale,
+    spec.bandIndexMap,
     width,
     height
   )
 
-  const bandBoundaries = getBandBoundaries(
-    hoverAsXIndices,
-    groupColData,
-    groupLineIndicesIntoBands(
-      spec.columnGroupMaps.fill,
-      lowerColumnName,
-      rowColumnName,
-      upperColumnName
-    )
-  )
-
   const hoverRowIndices = useHoverPointIndices(
     hoverDimension,
     hoverX,
     hoverY,
-    hoverXYColumnData.xs,
-    hoverXYColumnData.ys,
-    hoverXYColumnData.groupColData,
+    hoverableColumnData.xs,
+    hoverableColumnData.ys,
+    hoverableColumnData.groupColData,
     xScale,
     yScale,
     width,
@@ -145,8 +102,13 @@ export const BandLayer: FunctionComponent<Props> = props => {
   const bandHoverIndices = getBandHoverIndices(
     lineLengths,
     hoverRowIndices,
-    hoverXYColumnData.groupColData,
-    bandBoundaries
+    hoverableColumnData.groupColData,
+    groupLineIndicesIntoBands(
+      spec.columnGroupMaps.fill,
+      lowerColumnName,
+      rowColumnName,
+      upperColumnName
+    )
   )
 
   const hasHoverData = hoverRowIndices && hoverRowIndices.length > 0

giraffe/src/transforms/band.test.ts

@@ -164,39 +164,6 @@ describe('band transform utils', () => {
 
   describe('creates the bands to be rendered', () => {
     it('creates a line with lower and upper when they are available in the lineData', () => {
-      const fill = {
-        columnKeys: ['result', '_field', '_measurement', 'cpu', 'host'],
-        mappings: [
-          {
-            cpu: 'cpu0',
-            host: 'localhost',
-            result: 'min',
-            _field: 'usage_system',
-            _measurement: 'cpu',
-          },
-          {
-            cpu: 'cpu0',
-            host: 'localhost',
-            result: 'max',
-            _field: 'usage_system',
-            _measurement: 'cpu',
-          },
-          {
-            cpu: 'cpu0',
-            host: 'localhost',
-            result: 'mean',
-            _field: 'usage_system',
-            _measurement: 'cpu',
-          },
-          {
-            cpu: 'cpu1',
-            host: 'localhost',
-            result: 'max',
-            _field: 'usage_system',
-            _measurement: 'cpu',
-          },
-        ],
-      }
       const lineData = {
         0: {
           fill: 'rgb(106, 103, 205)',
@@ -250,54 +217,20 @@ describe('band transform utils', () => {
           ],
         },
       }
-      const result = getBands(fill, lineData, 'min', 'mean', 'max')
+      const bandIndexMap = {
+        upperIndices: [2],
+        rowIndices: [0],
+        lowerIndices: [1],
+      }
+
+      const result = getBands(lineData, bandIndexMap)
       expect(Array.isArray(result)).toEqual(true)
-      expect(result[0].fill).toEqual(lineData[2].fill)
+      expect(result[0].fill).toEqual(lineData[0].fill)
       expect(result[0].lower).toBeDefined()
       expect(result[0].upper).toBeDefined()
     })
 
     it('creates a line without the lower or upper when corresponding lower or upper is missing from lineData', () => {
-      const fill = {
-        columnKeys: ['result', '_field', '_measurement', 'cpu', 'host'],
-        mappings: [
-          {
-            cpu: 'cpu1',
-            host: 'localhost',
-            result: 'min',
-            _field: 'usage_system',
-            _measurement: 'cpu',
-          },
-          {
-            cpu: 'cpu0',
-            host: 'localhost',
-            result: 'max',
-            _field: 'usage_system',
-            _measurement: 'cpu',
-          },
-          {
-            cpu: 'cpu0',
-            host: 'localhost',
-            result: 'mean',
-            _field: 'usage_system',
-            _measurement: 'cpu',
-          },
-          {
-            cpu: 'cpu1',
-            host: 'localhost',
-            result: 'mean',
-            _field: 'usage_system',
-            _measurement: 'cpu',
-          },
-          {
-            cpu: 'cpu2',
-            host: 'localhost',
-            result: 'mean',
-            _field: 'usage_system',
-            _measurement: 'cpu',
-          },
-        ],
-      }
       const lineData = {
         0: {
           fill: 'rgb(106, 103, 205)',
@@ -385,18 +318,24 @@ describe('band transform utils', () => {
           ],
         },
       }
-      const result = getBands(fill, lineData, 'min', 'mean', 'max')
+      const bandIndexMap = {
+        upperIndices: [3, null, null],
+        rowIndices: [0, 1, 2],
+        lowerIndices: [null, 4, null],
+      }
+
+      const result = getBands(lineData, bandIndexMap)
       expect(Array.isArray(result)).toEqual(true)
 
-      expect(result[0].fill).toEqual(lineData[2].fill)
+      expect(result[0].fill).toEqual(lineData[0].fill)
       expect(result[0].lower).toBeUndefined()
       expect(result[0].upper).toBeDefined()
 
-      expect(result[1].fill).toEqual(lineData[3].fill)
+      expect(result[1].fill).toEqual(lineData[1].fill)
       expect(result[1].lower).toBeDefined()
       expect(result[1].upper).toBeUndefined()
 
-      expect(result[2].fill).toEqual(lineData[4].fill)
+      expect(result[2].fill).toEqual(lineData[2].fill)
       expect(result[2].lower).toBeUndefined()
       expect(result[2].upper).toBeUndefined()
     })