Merge pull request #15504 from grafana/thresholds-sort-order

Fixing array direction, adding simple render test
2025-02-25 18:55:37 -06:00 · 2019-02-18 18:35:11 +01:00 · 2019-02-18 18:35:11 +01:00 · caa477591b
commit caa477591b
parent 3d994b16d0 93e8edfddd
6 changed files with 84 additions and 57 deletions
--- a/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx
+++ b/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx
@ -84,9 +84,9 @@ describe('Get thresholds formatted', () => {
  it('should get the correct formatted values when thresholds are added', () => {
    const { instance } = setup({
      thresholds: [
-        { index: 2, value: 75, color: '#6ED0E0' },
-        { index: 1, value: 50, color: '#EAB839' },
        { index: 0, value: -Infinity, color: '#7EB26D' },
+        { index: 1, value: 50, color: '#EAB839' },
+        { index: 2, value: 75, color: '#6ED0E0' },
      ],
    });

--- a/packages/grafana-ui/src/components/Gauge/Gauge.tsx
+++ b/packages/grafana-ui/src/components/Gauge/Gauge.tsx
@ -98,16 +98,15 @@ export class Gauge extends PureComponent<Props> {
  getFormattedThresholds() {
    const { maxValue, minValue, thresholds, theme } = this.props;

-    const thresholdsSortedByIndex = [...thresholds].sort((t1, t2) => t1.index - t2.index);
-    const lastThreshold = thresholdsSortedByIndex[thresholdsSortedByIndex.length - 1];
+    const lastThreshold = thresholds[thresholds.length - 1];

    return [
-      ...thresholdsSortedByIndex.map(threshold => {
+      ...thresholds.map(threshold => {
        if (threshold.index === 0) {
          return { value: minValue, color: getColorFromHexRgbOrName(threshold.color, theme.type) };
        }

-        const previousThreshold = thresholdsSortedByIndex[threshold.index - 1];
+        const previousThreshold = thresholds[threshold.index - 1];
        return { value: threshold.value, color: getColorFromHexRgbOrName(previousThreshold.color, theme.type) };
      }),
      { value: maxValue, color: getColorFromHexRgbOrName(lastThreshold.color, theme.type) },
--- a/packages/grafana-ui/src/components/ThresholdsEditor/ThresholdsEditor.test.tsx
+++ b/packages/grafana-ui/src/components/ThresholdsEditor/ThresholdsEditor.test.tsx
@ -1,9 +1,8 @@
 import React, { ChangeEvent } from 'react';
-import { shallow } from 'enzyme';
-
+import { mount } from 'enzyme';
 import { ThresholdsEditor, Props } from './ThresholdsEditor';

-const setup = (propOverrides?: object) => {
+const setup = (propOverrides?: Partial<Props>) => {
  const props: Props = {
    onChange: jest.fn(),
    thresholds: [],
@ -11,12 +10,26 @@ const setup = (propOverrides?: object) => {

  Object.assign(props, propOverrides);

-  return shallow(<ThresholdsEditor {...props} />).instance() as ThresholdsEditor;
+  const wrapper = mount(<ThresholdsEditor {...props} />);
+  const instance = wrapper.instance() as ThresholdsEditor;
+
+  return {
+    instance,
+    wrapper,
+  };
 };

+describe('Render', () => {
+  it('should render with base threshold', () => {
+    const { wrapper } = setup();
+
+    expect(wrapper).toMatchSnapshot();
+  });
+});
+
 describe('Initialization', () => {
  it('should add a base threshold if missing', () => {
-    const instance = setup();
+    const { instance } = setup();

    expect(instance.state.thresholds).toEqual([{ index: 0, value: -Infinity, color: '#7EB26D' }]);
  });
@ -24,7 +37,7 @@ describe('Initialization', () => {

 describe('Add threshold', () => {
  it('should not add threshold at index 0', () => {
-    const instance = setup();
+    const { instance } = setup();

    instance.onAddThreshold(0);

@ -32,32 +45,32 @@ describe('Add threshold', () => {
  });

  it('should add threshold', () => {
-    const instance = setup();
+    const { instance } = setup();

    instance.onAddThreshold(1);

    expect(instance.state.thresholds).toEqual([
-      { index: 1, value: 50, color: '#EAB839' },
      { index: 0, value: -Infinity, color: '#7EB26D' },
+      { index: 1, value: 50, color: '#EAB839' },
    ]);
  });

  it('should add another threshold above a first', () => {
-    const instance = setup({
+    const { instance } = setup({
      thresholds: [{ index: 0, value: -Infinity, color: '#7EB26D' }, { index: 1, value: 50, color: '#EAB839' }],
    });

    instance.onAddThreshold(2);

    expect(instance.state.thresholds).toEqual([
-      { index: 2, value: 75, color: '#6ED0E0' },
-      { index: 1, value: 50, color: '#EAB839' },
      { index: 0, value: -Infinity, color: '#7EB26D' },
+      { index: 1, value: 50, color: '#EAB839' },
+      { index: 2, value: 75, color: '#6ED0E0' },
    ]);
  });

  it('should add another threshold between first and second index', () => {
-    const instance = setup({
+    const { instance } = setup({
      thresholds: [
        { index: 0, value: -Infinity, color: '#7EB26D' },
        { index: 1, value: 50, color: '#EAB839' },
@ -68,10 +81,10 @@ describe('Add threshold', () => {
    instance.onAddThreshold(2);

    expect(instance.state.thresholds).toEqual([
-      { index: 3, value: 75, color: '#6ED0E0' },
-      { index: 2, value: 62.5, color: '#EF843C' },
-      { index: 1, value: 50, color: '#EAB839' },
      { index: 0, value: -Infinity, color: '#7EB26D' },
+      { index: 1, value: 50, color: '#EAB839' },
+      { index: 2, value: 62.5, color: '#EF843C' },
+      { index: 3, value: 75, color: '#6ED0E0' },
    ]);
  });
 });
@ -83,7 +96,7 @@ describe('Remove threshold', () => {
      { index: 1, value: 50, color: '#EAB839' },
      { index: 2, value: 75, color: '#6ED0E0' },
    ];
-    const instance = setup({ thresholds });
+    const { instance } = setup({ thresholds });

    instance.onRemoveThreshold(thresholds[0]);

@ -96,9 +109,7 @@ describe('Remove threshold', () => {
      { index: 1, value: 50, color: '#EAB839' },
      { index: 2, value: 75, color: '#6ED0E0' },
    ];
-    const instance = setup({
-      thresholds,
-    });
+    const { instance } = setup({ thresholds });

    instance.onRemoveThreshold(thresholds[1]);

@ -116,7 +127,7 @@ describe('change threshold value', () => {
      { index: 1, value: 50, color: '#EAB839' },
      { index: 2, value: 75, color: '#6ED0E0' },
    ];
-    const instance = setup({ thresholds });
+    const { instance } = setup({ thresholds });

    const mockEvent = ({ target: { value: '12' } } as any) as ChangeEvent<HTMLInputElement>;

@ -126,7 +137,7 @@ describe('change threshold value', () => {
  });

  it('should update value', () => {
-    const instance = setup();
+    const { instance } = setup();
    const thresholds = [
      { index: 0, value: -Infinity, color: '#7EB26D' },
      { index: 1, value: 50, color: '#EAB839' },
@ -150,24 +161,24 @@ describe('change threshold value', () => {
 });

 describe('on blur threshold value', () => {
-  it('should resort rows and update indexes', () => {
-    const instance = setup();
+  it.only('should resort rows and update indexes', () => {
+    const { instance } = setup();
    const thresholds = [
      { index: 0, value: -Infinity, color: '#7EB26D' },
      { index: 1, value: 78, color: '#EAB839' },
      { index: 2, value: 75, color: '#6ED0E0' },
    ];

-    instance.state = {
+    instance.setState({
      thresholds,
-    };
+    });

    instance.onBlur();

    expect(instance.state.thresholds).toEqual([
-      { index: 2, value: 78, color: '#EAB839' },
-      { index: 1, value: 75, color: '#6ED0E0' },
      { index: 0, value: -Infinity, color: '#7EB26D' },
+      { index: 1, value: 75, color: '#6ED0E0' },
+      { index: 2, value: 78, color: '#EAB839' },
    ]);
  });
 });
--- a/packages/grafana-ui/src/components/ThresholdsEditor/ThresholdsEditor.tsx
+++ b/packages/grafana-ui/src/components/ThresholdsEditor/ThresholdsEditor.tsx
@ -1,7 +1,7 @@
 import React, { PureComponent, ChangeEvent } from 'react';
 import { Threshold } from '../../types';
-import { ColorPicker } from '../ColorPicker/ColorPicker';
-import { PanelOptionsGroup } from '../PanelOptionsGroup/PanelOptionsGroup';
+import { ColorPicker } from '..';
+import { PanelOptionsGroup } from '..';
 import { colors } from '../../utils';
 import { getColorFromHexRgbOrName, ThemeContext } from '@grafana/ui';

@ -54,16 +54,16 @@ export class ThresholdsEditor extends PureComponent<Props, State> {
    const value = afterThresholdValue - (afterThresholdValue - beforeThresholdValue) / 2;

    // Set a color
-    const color = colors.filter(c => newThresholds.some(t => t.color === c) === false)[0];
+    const color = colors.filter(c => !newThresholds.some(t => t.color === c))[0];

    this.setState(
      {
        thresholds: this.sortThresholds([
          ...newThresholds,
          {
+            color,
            index,
            value: value as number,
-            color,
          },
        ]),
      },
@ -137,10 +137,11 @@ export class ThresholdsEditor extends PureComponent<Props, State> {
  onBlur = () => {
    this.setState(prevState => {
      const sortThresholds = this.sortThresholds([...prevState.thresholds]);
-      let index = sortThresholds.length - 1;
+      let index = 0;
      sortThresholds.forEach(t => {
-        t.index = index--;
+        t.index = index++;
      });
+
      return { thresholds: sortThresholds };
    });

@ -153,12 +154,13 @@ export class ThresholdsEditor extends PureComponent<Props, State> {

  sortThresholds = (thresholds: Threshold[]) => {
    return thresholds.sort((t1, t2) => {
-      return t2.value - t1.value;
+      return t1.value - t2.value;
    });
  };

  renderInput = (threshold: Threshold) => {
    const value = threshold.index === 0 ? 'Base' : threshold.value;
+
    return (
      <div className="thresholds-row-input-inner">
        <span className="thresholds-row-input-inner-arrow" />
@ -190,30 +192,32 @@ export class ThresholdsEditor extends PureComponent<Props, State> {

  render() {
    const { thresholds } = this.state;
-
    return (
      <ThemeContext.Consumer>
        {theme => {
          return (
            <PanelOptionsGroup title="Thresholds">
              <div className="thresholds">
-                {thresholds.map((threshold, index) => {
-                  return (
-                    <div className="thresholds-row" key={`${threshold.index}-${index}`}>
-                      <div
-                        className="thresholds-row-add-button"
-                        onClick={() => this.onAddThreshold(threshold.index + 1)}
-                      >
-                        <i className="fa fa-plus" />
+                {thresholds
+                  .slice(0)
+                  .reverse()
+                  .map((threshold, index) => {
+                    return (
+                      <div className="thresholds-row" key={`${threshold.index}-${index}`}>
+                        <div
+                          className="thresholds-row-add-button"
+                          onClick={() => this.onAddThreshold(threshold.index + 1)}
+                        >
+                          <i className="fa fa-plus" />
+                        </div>
+                        <div
+                          className="thresholds-row-color-indicator"
+                          style={{ backgroundColor: getColorFromHexRgbOrName(threshold.color, theme.type) }}
+                        />
+                        <div className="thresholds-row-input">{this.renderInput(threshold)}</div>
                      </div>
-                      <div
-                        className="thresholds-row-color-indicator"
-                        style={{ backgroundColor: getColorFromHexRgbOrName(threshold.color, theme.type) }}
-                      />
-                      <div className="thresholds-row-input">{this.renderInput(threshold)}</div>
-                    </div>
-                  );
-                })}
+                    );
+                  })}
              </div>
            </PanelOptionsGroup>
          );
--- a/packages/grafana-ui/src/components/ThresholdsEditor/snapshots/ThresholdsEditor.test.tsx.snap
+++ b/packages/grafana-ui/src/components/ThresholdsEditor/snapshots/ThresholdsEditor.test.tsx.snap
@ -0,0 +1,7 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Render should render with base threshold 1`] = `
+<ContextConsumer>
+  <Component />
+</ContextConsumer>
+`;
--- a/public/app/features/dashboard/state/DashboardMigrator.ts
+++ b/public/app/features/dashboard/state/DashboardMigrator.ts
@ -399,6 +399,12 @@ export class DashboardMigrator {
            prefix: panel.options.prefix,
            suffix: panel.options.suffix,
          };
+
+          // correct order
+          if (panel.options.thresholds) {
+            panel.options.thresholds.reverse();
+          }
+
          // this options prop was due to a bug
          delete panel.options.options;
          delete panel.options.unit;