Sliders: Update behavior and style tweak (#29795)

* Sliders: Update behavior and style tweak * More style tweaks, and changed new graph opacity option from 0 - 1 to 1 to 100 * Updated point size max * Fixed hooks useCallback dependency issue * Update test
2025-01-07 22:53:56 -06:00 · 2020-12-14 11:34:12 +01:00 · 2020-12-14 11:34:12 +01:00 · 98c0b09564
commit 98c0b09564
parent d5a5461ced
9 changed files with 87 additions and 84 deletions
--- a/packages/grafana-ui/src/components/OptionsUI/slider.tsx
+++ b/packages/grafana-ui/src/components/OptionsUI/slider.tsx
@ -1,4 +1,4 @@
-import React, { useCallback } from 'react';
+import React from 'react';
 import { FieldConfigEditorProps, SliderFieldConfigSettings } from '@grafana/data';
 import { Slider } from '../Slider/Slider';

@ -8,20 +8,16 @@ export const SliderValueEditor: React.FC<FieldConfigEditorProps<number, SliderFi
  item,
 }) => {
  const { settings } = item;
-  const onValueAfterChange = useCallback(
-    (value?: number) => {
-      onChange(value);
-    },
-    [onChange]
-  );
+
  const initialValue = typeof value === 'number' ? value : typeof value === 'string' ? +value : 0;
+
  return (
    <Slider
      value={initialValue}
      min={settings?.min || 0}
      max={settings?.max || 100}
      step={settings?.step}
-      onAfterChange={onValueAfterChange}
+      onChange={onChange}
    />
  );
 };
--- a/packages/grafana-ui/src/components/Slider/Slider.story.tsx
+++ b/packages/grafana-ui/src/components/Slider/Slider.story.tsx
@ -21,7 +21,7 @@ const SliderWrapper = () => {
  const { min, max, orientation, reverse, step } = getKnobs();
  const stepValue = step ? 10 : undefined;
  return (
-    <div style={{ width: '200px', height: '200px' }}>
+    <div style={{ width: '300px', height: '300px' }}>
      <Slider min={min} max={max} step={stepValue} orientation={orientation} value={10} reverse={reverse} />
    </div>
  );
--- a/packages/grafana-ui/src/components/Slider/Slider.tsx
+++ b/packages/grafana-ui/src/components/Slider/Slider.tsx
@ -5,6 +5,7 @@ import { Global } from '@emotion/core';
 import { useTheme } from '../../themes/ThemeContext';
 import { getStyles } from './styles';
 import { SliderProps } from './types';
+import { Input } from '../Input/Input';

 /**
 * @public
@ -24,31 +25,45 @@ export const Slider: FunctionComponent<SliderProps> = ({
  const styles = getStyles(theme, isHorizontal);
  const SliderWithTooltip = SliderComponent;
  const [slidervalue, setSliderValue] = useState<number>(value || min);
-  const onSliderChange = useCallback((v: number) => {
-    setSliderValue(v);

-    if (onChange) {
-      onChange(v);
-    }
-  }, []);
-  const onSliderInputChange = useCallback((e: ChangeEvent<HTMLInputElement>) => {
-    let v = +e.target.value;
+  const onSliderChange = useCallback(
+    (v: number) => {
+      setSliderValue(v);

-    v > max && (v = max);
-    v < min && (v = min);
+      if (onChange) {
+        onChange(v);
+      }
+    },
+    [setSliderValue, onChange]
+  );

-    setSliderValue(v);
+  const onSliderInputChange = useCallback(
+    (e: ChangeEvent<HTMLInputElement>) => {
+      let v = +e.target.value;

-    if (onChange) {
-      onChange(v);
-    }
+      if (Number.isNaN(v)) {
+        v = 0;
+      }
+
+      v > max && (v = max);
+      v < min && (v = min);
+
+      setSliderValue(v);
+
+      if (onChange) {
+        onChange(v);
+      }
+
+      if (onAfterChange) {
+        onAfterChange(v);
+      }
+    },
+    [setSliderValue, onAfterChange]
+  );

-    if (onAfterChange) {
-      onAfterChange(v);
-    }
-  }, []);
  const sliderInputClassNames = !isHorizontal ? [styles.sliderInputVertical] : [];
  const sliderInputFieldClassNames = !isHorizontal ? [styles.sliderInputFieldVertical] : [];
+
  return (
    <div className={cx(styles.container, styles.slider)}>
      {/** Slider tooltip's parent component is body and therefore we need Global component to do css overrides for it. */}
@ -65,9 +80,10 @@ export const Slider: FunctionComponent<SliderProps> = ({
          vertical={!isHorizontal}
          reverse={reverse}
        />
-        <input
+        {/* Uses text input so that the number spinners are not shown */}
+        <Input
+          type="text"
          className={cx(styles.sliderInputField, ...sliderInputFieldClassNames)}
-          type="number"
          value={`${slidervalue}`} // to fix the react leading zero issue
          onChange={onSliderInputChange}
          min={min}
--- a/packages/grafana-ui/src/components/Slider/styles.ts
+++ b/packages/grafana-ui/src/components/Slider/styles.ts
@ -3,6 +3,7 @@ import { GrafanaTheme } from '@grafana/data';
 import { focusCss } from '../../themes/mixins';
 import { css as cssCore } from '@emotion/core';
 import { css } from 'emotion';
+import tinycolor from 'tinycolor2';

 export const getFocusStyle = (theme: GrafanaTheme) => css`
  &:focus {
@ -11,18 +12,21 @@ export const getFocusStyle = (theme: GrafanaTheme) => css`
 `;

 export const getStyles = stylesFactory((theme: GrafanaTheme, isHorizontal: boolean) => {
-  const trackColor = theme.isLight ? theme.palette.gray5 : theme.palette.dark6;
-  const container = isHorizontal
-    ? css`
-        width: 100%;
-      `
-    : css`
-        height: 100%;
-        margin: ${theme.spacing.sm} ${theme.spacing.lg} ${theme.spacing.sm} ${theme.spacing.sm};
-      `;
+  const { spacing, palette } = theme;
+  const railColor = theme.isLight ? palette.gray5 : palette.dark6;
+  const trackColor = theme.isLight ? palette.blue85 : palette.blue77;
+  const handleColor = theme.isLight ? palette.blue85 : palette.blue80;
+  const blueOpacity = tinycolor(handleColor)
+    .setAlpha(0.2)
+    .toString();
+  const hoverSyle = `box-shadow: 0px 0px 0px 6px ${blueOpacity}`;

  return {
-    container,
+    container: css`
+      width: 100%;
+      margin: ${isHorizontal ? 'none' : `${spacing.sm} ${spacing.lg} ${spacing.sm} ${spacing.sm}`};
+      height: ${isHorizontal ? 'auto' : '100%'};
+    `,
    slider: css`
      .rc-slider {
        display: flex;
@ -33,32 +37,24 @@ export const getStyles = stylesFactory((theme: GrafanaTheme, isHorizontal: boole
        margin-top: -10px;
      }
      .rc-slider-handle {
-        border: solid 2px ${theme.palette.blue77};
-        background-color: ${theme.palette.blue77};
-      }
-      .rc-slider-handle:hover {
-        border-color: ${theme.palette.blue77};
-      }
-      .rc-slider-handle:focus {
-        border-color: ${theme.palette.blue77};
-        box-shadow: none;
-      }
-      .rc-slider-handle:active {
-        border-color: ${theme.palette.blue77};
-        box-shadow: none;
-      }
-      .rc-slider-handle-click-focused:focus {
-        border-color: ${theme.palette.blue77};
+        border: none;
+        background-color: ${handleColor};
+        cursor: pointer;
      }
+      .rc-slider-handle:hover,
+      .rc-slider-handle:active,
+      .rc-slider-handle:focus,
+      .rc-slider-handle-click-focused:focus,
      .rc-slider-dot-active {
-        border-color: ${theme.palette.blue77};
+        ${hoverSyle};
      }
      .rc-slider-track {
-        background-color: ${theme.palette.blue77};
+        background-color: ${trackColor};
      }
      .rc-slider-rail {
-        background-color: ${trackColor};
-        border: 1px solid ${trackColor};
+        background-color: ${railColor};
+        border: 1px solid ${railColor};
+        cursor: pointer;
      }
    `,
    /** Global component from @emotion/core doesn't accept computed classname string returned from css from emotion.
@ -104,15 +100,11 @@ export const getStyles = stylesFactory((theme: GrafanaTheme, isHorizontal: boole
      }
    `,
    sliderInputField: css`
-      display: flex;
-      flex-grow: 0;
-      flex-basis: 50px;
      margin-left: ${theme.spacing.lg};
-      height: ${theme.spacing.formInputHeight}px;
-      text-align: center;
-      border-radius: ${theme.border.radius.sm};
-      border: 1px solid ${theme.colors.formInputBorder};
-      ${getFocusStyle(theme)};
+      width: 60px;
+      input {
+        text-align: center;
+      }
    `,
    sliderInputFieldVertical: css`
      margin: 0 0 ${theme.spacing.lg} 0;
--- a/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.test.ts
+++ b/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.test.ts
@ -138,7 +138,7 @@ describe('UPlotConfigBuilder', () => {
      drawStyle: DrawStyle.Line,
      scaleKey: 'scale-x',
      fillColor: '#ff0000',
-      fillOpacity: 0.5,
+      fillOpacity: 50,
      showPoints: PointVisibility.Auto,
      pointSize: 5,
      pointColor: '#00ff00',
--- a/packages/grafana-ui/src/components/uPlot/config/UPlotSeriesBuilder.ts
+++ b/packages/grafana-ui/src/components/uPlot/config/UPlotSeriesBuilder.ts
@ -81,13 +81,12 @@ export class UPlotSeriesBuilder extends PlotConfigBuilder<SeriesProps, Series> {
    }

    let fillConfig: any | undefined;
-    if (fillColor && fillOpacity !== 0) {
+    let fillOpacityNumber = fillOpacity ?? 0;
+    if (fillColor && fillOpacityNumber !== 0) {
      fillConfig = {
-        fill: fillOpacity
-          ? tinycolor(fillColor)
-              .setAlpha(fillOpacity)
-              .toRgbString()
-          : fillColor,
+        fill: tinycolor(fillColor)
+          .setAlpha(fillOpacityNumber / 100)
+          .toRgbString(),
      };
    }

--- a/public/app/plugins/panel/graph3/snapshots/migrations.test.ts.snap
+++ b/public/app/plugins/panel/graph3/snapshots/migrations.test.ts.snap
@ -33,7 +33,7 @@ Object {
      "custom": Object {
        "axisPlacement": "hidden",
        "drawStyle": "line",
-        "fillOpacity": 0.5,
+        "fillOpacity": 50,
        "lineInterpolation": "stepAfter",
        "lineWidth": 1,
        "pointSize": 6,
@ -66,7 +66,7 @@ Object {
      "custom": Object {
        "axisPlacement": "auto",
        "drawStyle": "line",
-        "fillOpacity": 0.5,
+        "fillOpacity": 50,
        "lineInterpolation": "stepAfter",
        "lineWidth": 5,
        "pointSize": 6,
@ -115,7 +115,7 @@ Object {
        "axisLabel": "Y111",
        "axisPlacement": "auto",
        "drawStyle": "line",
-        "fillOpacity": 0.1,
+        "fillOpacity": 10,
        "lineWidth": 1,
        "pointSize": 6,
        "showPoints": "never",
--- a/public/app/plugins/panel/graph3/migrations.ts
+++ b/public/app/plugins/panel/graph3/migrations.ts
@ -109,7 +109,7 @@ export function flotToGraphOptions(angular: any): { fieldConfig: FieldConfigSour
          case 'fill':
            rule.properties.push({
              id: 'custom.fillOpacity',
-              value: v / 10.0, // was 0-10
+              value: v * 10, // was 0-10, new graph is 0 - 100
            });
            break;
          case 'points':
@ -170,7 +170,7 @@ export function flotToGraphOptions(angular: any): { fieldConfig: FieldConfigSour
    graph.pointSize = 2 + angular.pointradius * 2;
  }
  if (isNumber(angular.fill)) {
-    graph.fillOpacity = angular.fill / 10; // fill is 0-10
+    graph.fillOpacity = angular.fill * 10; // fill was 0 - 10, new is 0 to 100
  }
  graph.spanNulls = angular.nullPointMode === NullValueMode.Null;
  if (angular.steppedLine) {
--- a/public/app/plugins/panel/graph3/module.tsx
+++ b/public/app/plugins/panel/graph3/module.tsx
@ -57,11 +57,11 @@ export const plugin = new PanelPlugin<Options, GraphFieldConfig>(GraphPanel)
        .addSliderInput({
          path: 'fillOpacity',
          name: 'Fill area opacity',
-          defaultValue: 0.1,
+          defaultValue: 10,
          settings: {
            min: 0,
-            max: 1,
-            step: 0.1,
+            max: 100,
+            step: 1,
          },
          showIf: c => c.drawStyle !== DrawStyle.Points,
        })
@ -91,10 +91,10 @@ export const plugin = new PanelPlugin<Options, GraphFieldConfig>(GraphPanel)
          defaultValue: 5,
          settings: {
            min: 1,
-            max: 10,
+            max: 40,
            step: 1,
          },
-          showIf: c => c.showPoints !== PointVisibility.Never,
+          showIf: c => c.showPoints !== PointVisibility.Never || c.drawStyle === DrawStyle.Points,
        })
        .addRadio({
          path: 'axisPlacement',