From 3b0356a165fab252958ed99910e51ce40362d6cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 14 Jan 2021 11:29:44 +0100 Subject: [PATCH] Decimals: Improving auto decimals logic for high numbers and scaled units (#30262) * Big improvement to decimals logic is finally working * Removed unused elements from old solution * Updated test --- .../src/field/displayProcessor.test.ts | 46 ++++++++--------- .../src/field/displayProcessor.ts | 50 +++++++++++-------- .../panel/singlestat/specs/singlestat.test.ts | 4 +- public/vendor/flot/jquery.flot.js | 4 +- 4 files changed, 54 insertions(+), 50 deletions(-) diff --git a/packages/grafana-data/src/field/displayProcessor.test.ts b/packages/grafana-data/src/field/displayProcessor.test.ts index 79820dceb6b..28fde146235 100644 --- a/packages/grafana-data/src/field/displayProcessor.test.ts +++ b/packages/grafana-data/src/field/displayProcessor.test.ts @@ -1,4 +1,4 @@ -import { getDecimalsForValue, getDisplayProcessor, getRawDisplayProcessor } from './displayProcessor'; +import { getDisplayProcessor, getRawDisplayProcessor } from './displayProcessor'; import { DisplayProcessor, DisplayValue } from '../types/displayValue'; import { MappingType, ValueMapping } from '../types/valueMapping'; import { FieldConfig, FieldType, ThresholdsMode } from '../types'; @@ -147,14 +147,20 @@ describe('Format value', () => { it('should set auto decimals, 1 significant', () => { const value = 3.23; const instance = getDisplayProcessorFromConfig({ decimals: null }); - expect(instance(value).text).toEqual('3.2'); + expect(instance(value).text).toEqual('3.23'); }); it('should set auto decimals, 2 significant', () => { const value = 0.0245; const instance = getDisplayProcessorFromConfig({ decimals: null }); - expect(instance(value).text).toEqual('0.025'); + expect(instance(value).text).toEqual('0.0245'); + }); + + it('should set auto decimals correctly for value 0.333333333333', () => { + const value = 1 / 3; + const instance = getDisplayProcessorFromConfig({ decimals: null }); + expect(instance(value).text).toEqual('0.333'); }); it('should use override decimals', () => { @@ -205,7 +211,7 @@ describe('Format value', () => { const value = 1000; const instance = getDisplayProcessorFromConfig({ decimals: null, unit: 'short' }); const disp = instance(value); - expect(disp.text).toEqual('1.000'); + expect(disp.text).toEqual('1.0'); expect(disp.suffix).toEqual(' K'); }); @@ -213,7 +219,7 @@ describe('Format value', () => { const value = 1200; const instance = getDisplayProcessorFromConfig({ decimals: null, unit: 'short' }); const disp = instance(value); - expect(disp.text).toEqual('1.200'); + expect(disp.text).toEqual('1.2'); expect(disp.suffix).toEqual(' K'); }); @@ -221,7 +227,7 @@ describe('Format value', () => { const value = 1250; const instance = getDisplayProcessorFromConfig({ decimals: null, unit: 'short' }); const disp = instance(value); - expect(disp.text).toEqual('1.250'); + expect(disp.text).toEqual('1.25'); expect(disp.suffix).toEqual(' K'); }); @@ -229,7 +235,15 @@ describe('Format value', () => { const value = 1000000; const instance = getDisplayProcessorFromConfig({ decimals: null, unit: 'short' }); const disp = instance(value); - expect(disp.text).toEqual('1.000'); + expect(disp.text).toEqual('1.0'); + expect(disp.suffix).toEqual(' Mil'); + }); + + it('with value 15000000 and unit short', () => { + const value = 1500000; + const instance = getDisplayProcessorFromConfig({ decimals: null, unit: 'short' }); + const disp = instance(value); + expect(disp.text).toEqual('1.5'); expect(disp.suffix).toEqual(' Mil'); }); }); @@ -349,21 +363,3 @@ describe('getRawDisplayProcessor', () => { expect(result).toEqual({ text: expected, numeric: null }); }); }); - -describe('getDecimalsForValue', () => { - it.each` - value | expected - ${0} | ${0} - ${13.37} | ${0} - ${-13.37} | ${0} - ${12679.3712345811212} | ${0} - ${-12679.3712345811212} | ${0} - ${0.3712345} | ${2} - ${-0.37123458} | ${2} - ${-0.04671994403853774} | ${3} - ${0.04671994403853774} | ${3} - `('should return correct suggested decimal count', ({ value, expected }) => { - const result = getDecimalsForValue(value); - expect(result.decimals).toEqual(expected); - }); -}); diff --git a/packages/grafana-data/src/field/displayProcessor.ts b/packages/grafana-data/src/field/displayProcessor.ts index b6dd87991aa..27a4a8ce320 100644 --- a/packages/grafana-data/src/field/displayProcessor.ts +++ b/packages/grafana-data/src/field/displayProcessor.ts @@ -137,41 +137,49 @@ function toStringProcessor(value: any): DisplayValue { return { text: _.toString(value), numeric: toNumber(value) }; } +function getSignificantDigitCount(n: number) { + //remove decimal and make positive + n = Math.abs(+String(n).replace('.', '')); + if (n === 0) { + return 0; + } + + // kill the 0s at the end of n + while (n !== 0 && n % 10 === 0) { + n /= 10; + } + + // get number of digits + return Math.floor(Math.log(n) / Math.LN10) + 1; +} + export function getDecimalsForValue(value: number, decimalOverride?: DecimalCount): DecimalInfo { if (_.isNumber(decimalOverride)) { // It's important that scaledDecimals is null here return { decimals: decimalOverride, scaledDecimals: null }; } - let dec = -Math.floor(Math.log(Math.abs(value)) / Math.LN10) + 1; - const magn = Math.pow(10, -dec); - const norm = value / magn; // norm is between 1.0 and 10.0 - let size; - - if (norm < 1.5) { - size = 1; - } else if (norm < 3) { - size = 2; - // special case for 2.5, requires an extra decimal - if (norm > 2.25) { - size = 2.5; - ++dec; - } - } else if (norm < 7.5) { - size = 5; - } else { - size = 10; + if (value === 0) { + return { decimals: 0, scaledDecimals: 0 }; } - size *= magn; + const digits = getSignificantDigitCount(value); + const log10 = Math.floor(Math.log(Math.abs(value)) / Math.LN10); + let dec = -log10 + 1; + const magn = Math.pow(10, -dec); + const norm = value / magn; // norm is between 1.0 and 10.0 + + // special case for 2.5, requires an extra decimal + if (norm > 2.25) { + ++dec; + } - // reduce starting decimals if not needed if (value % 1 === 0) { dec = 0; } const decimals = Math.max(0, dec); - const scaledDecimals = decimals - Math.floor(Math.log(size) / Math.LN10) + 2; + const scaledDecimals = decimals - log10 + digits - 1; return { decimals, scaledDecimals }; } diff --git a/public/app/plugins/panel/singlestat/specs/singlestat.test.ts b/public/app/plugins/panel/singlestat/specs/singlestat.test.ts index ade7fbed9f3..ca96a1f2fab 100644 --- a/public/app/plugins/panel/singlestat/specs/singlestat.test.ts +++ b/public/app/plugins/panel/singlestat/specs/singlestat.test.ts @@ -257,7 +257,7 @@ describe('SingleStatCtrl', () => { }); it('should set formatted value', () => { - expect(ctx.data.display!.text).toBe('100'); + expect(ctx.data.display!.text).toBe('100.0'); }); } ); @@ -373,7 +373,7 @@ describe('SingleStatCtrl', () => { }); it('should set formatted falue', () => { - expect(ctx.data.display!.text).toBe('100'); + expect(ctx.data.display!.text).toBe('100.0'); }); } ); diff --git a/public/vendor/flot/jquery.flot.js b/public/vendor/flot/jquery.flot.js index 22cb2b8ee26..204eace7ba4 100644 --- a/public/vendor/flot/jquery.flot.js +++ b/public/vendor/flot/jquery.flot.js @@ -3059,7 +3059,7 @@ Licensed under the MIT license. if (!e.cancelable) { return; } - + if (!eventHolder.is(e.target) && eventHolder.has(e.target).length === 0) { triggerClickHoverEvent("plotleave", e, function (s) { false; }); return; @@ -3080,7 +3080,7 @@ Licensed under the MIT license. } var original = e.originalEvent; - + if (original.changedTouches.length === 0) { return e; }