Stable chart height fix (#420)

* remove unneeded support for old dashboard localstorage chart-height key

* fix changing height of chart container

* remove unused metrics_correlation functionality

* 2.62.3

* 2.26.4

Author: jacekkolasa

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@netdata/dashboard",
-  "version": "2.26.2",
+  "version": "2.26.4",
   "homepage": ".",
   "main": "./lib/src/index-npm.js",
   "files": [

package.json

@@ -1,8 +1,6 @@
 //@ts-nocheck
 import { sortBy, reverse } from "ramda"
-import React, {
-  useLayoutEffect, useRef, useCallback, useState,
-} from "react"
+import React, { useLayoutEffect, useRef, useCallback } from "react"
 import classNames from "classnames"
 import { useUpdateEffect, useUnmount, useMount } from "react-use"
 // this version is needed because it contains a fix for handling constant value in the chart
@@ -28,11 +26,14 @@ import {
   selectSyncPanAndZoom,
   selectSpacePanelTransitionEndIsActive,
   selectAlarm,
-  selectTimezoneSetting
+  selectTimezoneSetting,
 } from "domains/global/selectors"
 import {
-  resetGlobalPanAndZoomAction, setCommonMaxAction, setCommonMinAction,
-  setGlobalPauseAction, resetGlobalPauseAction
+  resetGlobalPanAndZoomAction,
+  setCommonMaxAction,
+  setCommonMinAction,
+  setGlobalPauseAction,
+  resetGlobalPauseAction,
 } from "domains/global/actions"
 
 import { resetChartPanAndZoomAction } from "domains/chart/actions"
@@ -47,7 +48,10 @@ import { ChartMetadata, DygraphData } from "../../chart-types"
 import { selectResizeHeight } from "../../selectors"
 
 import {
-  getDygraphChartType, getDataForFakeStacked, transformColors, getDygraphFillAlpha,
+  getDygraphChartType,
+  getDataForFakeStacked,
+  transformColors,
+  getDygraphFillAlpha,
   hackDygraphIFrameTarps,
 } from "./dygraph/utils"
 import "./dygraph-chart.css"
@@ -56,7 +60,6 @@ import useProceededChart from "../../hooks/use-proceeded-chart"
 import useDygraphBadge from "../../hooks/useDygraphBadge"
 import ProceededChartDisclaimer from "./proceeded-chart-disclaimer"
 import AlarmBadge, { getBorderColor } from "./alarmBadge"
-import NeutralPill from "./neutralPill"
 
 // This is the threshold above which we assume chart shown duration has changed
 const timeframeThreshold = 5000
@@ -413,7 +416,6 @@ export const DygraphChart = ({
   }, [chartUuid, dispatch, isSyncPanAndZoom])
 
   const [isAlarmBadgeVisible, alarmBadgeRef, updateAlarmBadge] = useDygraphBadge() as any
-  const [isHighlightBadgeVisible, highlightBadgeRef, updateHighlightBadge] = useDygraphBadge() as any
 
   // setGlobalChartUnderlay is using state from closure (chartData.after), so we need to have always
   // the newest callback. Unfortunately we cannot use Dygraph.updateOptions() (library restriction)
@@ -428,7 +430,6 @@ export const DygraphChart = ({
     resetGlobalPanAndZoom,
     setGlobalChartUnderlay,
     updateAlarmBadge,
-    updateHighlightBadge,
     updateChartPanOrZoom,
     viewAfter,
     viewBefore,
@@ -447,14 +448,22 @@ export const DygraphChart = ({
     propsRef.current.resetGlobalPanAndZoom = resetGlobalPanAndZoom
     propsRef.current.setGlobalChartUnderlay = setGlobalChartUnderlay
     propsRef.current.updateAlarmBadge = updateAlarmBadge
-    propsRef.current.updateHighlightBadge = updateHighlightBadge
     propsRef.current.updateChartPanOrZoom = updateChartPanOrZoom
     propsRef.current.viewAfter = viewAfter
     propsRef.current.viewBefore = viewBefore
-  }, [alarm, chartData, globalChartUnderlay, hoveredX, immediatelyDispatchPanAndZoom,
-    resetGlobalPanAndZoom, setGlobalChartUnderlay, updateAlarmBadge, updateHighlightBadge, updateChartPanOrZoom, viewAfter, viewBefore])
-
-  const [localHighlight, setLocalHighlight] = useState<{before?: number, after?: number} | null>(null)
+  }, [
+    alarm,
+    chartData,
+    globalChartUnderlay,
+    hoveredX,
+    immediatelyDispatchPanAndZoom,
+    resetGlobalPanAndZoom,
+    setGlobalChartUnderlay,
+    updateAlarmBadge,
+    updateChartPanOrZoom,
+    viewAfter,
+    viewBefore,
+  ])
 
   const shouldSmoothPlot = useSelector(selectSmoothPlot)
   useLayoutEffect(() => {
@@ -561,29 +570,6 @@ export const DygraphChart = ({
             )
           }
 
-          const onMetricsCorralation = hasHashParam("metrics_correlation")
-          if (onMetricsCorralation && propsRef.current.globalChartUnderlay) {
-            const { before, masterID } = propsRef.current.globalChartUnderlay
-
-            if (masterID && masterID !== propsRef.current.chartData.id) {
-              propsRef.current.updateHighlightBadge(false)
-            } else {
-              const horizontalPadding = 3
-
-              propsRef.current.updateHighlightBadge(
-                true,
-                g,
-                g.toDomXCoord(before),
-                (pillRef, pillX, pillPosition, topMargin) => {
-                  pillRef.current.style.left = `${pillPosition + horizontalPadding}px`
-                  pillRef.current.style.top = topMargin
-                }
-              )
-            }
-          } else {
-            propsRef.current.updateHighlightBadge(false)
-          }
-
           // the chart is about to be drawn
           // this function renders global highlighted time-frame
 
@@ -669,28 +655,6 @@ export const DygraphChart = ({
               latestIsUserAction.current = true
               // @ts-ignore
               Dygraph.moveZoom(event, dygraph, context)
-
-              const onMetricsCorralation = hasHashParam("metrics_correlation")
-              if (onMetricsCorralation) {
-                const before = dygraph.toDataXCoord(event.offsetX)
-                const after = dygraphHighlightAfter.current
-                if (before - after > 1000) { // more than a sec
-                  setLocalHighlight({
-                    after,
-                    before,
-                  })
-                  propsRef.current.updateHighlightBadge(
-                    true,
-                    dygraph,
-                    dygraph.toDomXCoord(before),
-                    (pillRef, pillX, pillPosition, topMargin) => {
-                      pillRef.current.style.left = `${pillPosition}px`
-                      pillRef.current.style.top = topMargin
-                    }
-                  )
-                }
-              }
-
               event.preventDefault()
             } else if (context.isPanning) {
               latestIsUserAction.current = true
@@ -718,7 +682,6 @@ export const DygraphChart = ({
                 masterID: chartData.id,
               })
               dygraphHighlightAfter.current = null
-              setLocalHighlight(null)
               // eslint-disable-next-line no-param-reassign
               context.isZooming = false
 
@@ -732,13 +695,11 @@ export const DygraphChart = ({
               // eslint-disable-next-line no-underscore-dangle
               dygraph.drawGraph_(false)
             } else if (context.isPanning) {
-
               latestIsUserAction.current = true
               // @ts-ignore
               Dygraph.endPan(event, dygraph, context)
               propsRef.current.immediatelyDispatchPanAndZoom()
             } else if (context.isZooming) {
-
               latestIsUserAction.current = true
               // @ts-ignore
               Dygraph.endZoom(event, dygraph, context)
@@ -1205,11 +1166,12 @@ export const DygraphChart = ({
       )}
       {alarm?.value && hasLegend && (
         // @ts-ignore
-        <AlarmBadge isVisible={isAlarmBadgeVisible} ref={alarmBadgeRef} status={alarm.status} label={alarm.value} />
-      )}
-      {hasLegend && (
-        // @ts-ignore
-        <NeutralPill isVisible={isHighlightBadgeVisible} ref={highlightBadgeRef} {...globalChartUnderlay} {...localHighlight} />
+        <AlarmBadge
+          isVisible={isAlarmBadgeVisible}
+          ref={alarmBadgeRef}
+          status={alarm.status}
+          label={alarm.value}
+        />
       )}
       <div className="dygraph-chart__labels-hidden" id={hiddenLabelsElementId} />
     </>

src/domains/chart/components/lib-charts/dygraph-chart.tsx

@@ -4,7 +4,6 @@ import { setResizeHeightAction } from "domains/chart/actions"
 import { LEGEND_BOTTOM_SINGLE_LINE_HEIGHT } from "domains/chart/utils/legend-utils"
 import { useDispatch } from "store/redux-separate-context"
 
-export const LOCALSTORAGE_HEIGHT_KEY_PREFIX_OLD = "chart_heights."
 export const LOCALSTORAGE_HEIGHT_KEY_PREFIX = "chart_height."
 
 interface Props {

src/domains/chart/components/lib-charts/neutralPill.js

@@ -1,7 +1,4 @@
-import {
-  LOCALSTORAGE_HEIGHT_KEY_PREFIX,
-  LOCALSTORAGE_HEIGHT_KEY_PREFIX_OLD,
-} from "domains/chart/components/resize-handler"
+import { LOCALSTORAGE_HEIGHT_KEY_PREFIX } from "domains/chart/components/resize-handler"
 
 import { LEGEND_BOTTOM_SINGLE_LINE_HEIGHT } from "domains/chart/utils/legend-utils"
 import { Attributes } from "./transformDataAttributes"
@@ -17,8 +14,6 @@ type GetPortalNodeStyles = (
   minWidth: string | undefined
 }
 
-const oldDefaultHeights = ["180px", "90px"]
-
 const getHeightFromLocalStorage = (heightID: string, isLegendOnBottom: boolean) => {
   const persitedHeight = localStorage.getItem(`${LOCALSTORAGE_HEIGHT_KEY_PREFIX}${heightID}`)
   if (persitedHeight) {
@@ -31,20 +26,6 @@ const getHeightFromLocalStorage = (heightID: string, isLegendOnBottom: boolean)
     }px`
   }
 
-  // We'll support the old key for few months, so that user's custom heights will be working,
-  // but we'll save any changes only to the new key.
-  const persistedHeightOld = localStorage.getItem(
-    `${LOCALSTORAGE_HEIGHT_KEY_PREFIX_OLD}${heightID}`,
-  )
-  if (persistedHeightOld) {
-    if (oldDefaultHeights.includes(persistedHeightOld)) {
-      // If saved value looks like `oldDefaultHeights`, then it is most likely an automatic value
-      // from old dashboard. Don't import it anymore. On next resize the height will be persisted
-      // to the new key.
-      return null
-    }
-    return persistedHeightOld
-  }
   return null
 }
 

src/domains/chart/components/resize-handler.tsx

@@ -294,6 +294,7 @@ body.modal-open {
 
 .netdata-chartblock-container {
     display: inline-block;
+    overflow: hidden;
 }
 
 /* https://github.com/seiyria/bootstrap-slider/issues/746 */