-
Notifications
You must be signed in to change notification settings - Fork 15
fix(FormattedBytes): show 1_000 with another unit #1901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,12 @@ describe('formatNumericValues', () => { | |
const result = formatNumericValues(1024, 2048); | ||
expect(result).toEqual(['1', `2${UNBREAKABLE_GAP}k`]); | ||
}); | ||
|
||
it('should format values without units (less than 1000)', () => { | ||
const result1 = formatNumericValues(10, 20); | ||
expect(result1).toEqual(['10', `20`]); | ||
Comment on lines
+26
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before in such case |
||
}); | ||
|
||
it('should format value with label if set', () => { | ||
const result = formatNumericValues(1024, 2048, undefined, undefined, true); | ||
expect(result).toEqual([`1${UNBREAKABLE_GAP}k`, `2${UNBREAKABLE_GAP}k`]); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,10 @@ import type {FormatToSizeArgs, FormatValuesArgs} from './common'; | |
import {formatNumber, roundToPrecision} from './dataFormatters'; | ||
|
||
const sizes = { | ||
noUnit: { | ||
value: 1, | ||
label: '', | ||
}, | ||
thousand: { | ||
value: 1_000, | ||
label: i18n('label_thousand'), | ||
|
@@ -25,43 +29,19 @@ const sizes = { | |
|
||
export type Digits = keyof typeof sizes; | ||
|
||
/** | ||
* This function is needed to keep more than 3 digits of the same size. | ||
* | ||
* @param significantDigits - number of digits above 3 | ||
* @returns size to format value to get required number of digits | ||
* | ||
* By default value converted to the next size when it's above 1000, | ||
* so we have 900k and 1m. To extend it additional significantDigits could be set | ||
* | ||
* significantDigits value added above default 3 | ||
* | ||
* significantDigits = 1 - 9 000k and 10m | ||
* | ||
* significantDigits = 2 - 90 000m and 100b | ||
* | ||
* significantDigits = 3 - 900 000b and 1000t | ||
*/ | ||
export const getNumberWithSignificantDigits = (value: number, significantDigits: number) => { | ||
const multiplier = 10 ** significantDigits; | ||
|
||
const thousandLevel = sizes.thousand.value * multiplier; | ||
const millionLevel = sizes.million.value * multiplier; | ||
const billionLevel = sizes.billion.value * multiplier; | ||
const trillionLevel = sizes.trillion.value * multiplier; | ||
|
||
let size: Digits = 'thousand'; | ||
|
||
if (value > thousandLevel) { | ||
export const getNumberSizeUnit = (value: number) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May we use http://numeraljs.com/ instead of custom unit getter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We use custom getter, because together with auto formatting we can pass specific unit - it's helpful for data inside tables (data on every row should be with the same unit). AFAIK |
||
let size: Digits = 'noUnit'; | ||
|
||
if (value >= sizes.thousand.value) { | ||
size = 'thousand'; | ||
} | ||
if (value >= millionLevel) { | ||
if (value >= sizes.million.value) { | ||
size = 'million'; | ||
} | ||
if (value >= billionLevel) { | ||
if (value >= sizes.billion.value) { | ||
size = 'billion'; | ||
} | ||
if (value >= trillionLevel) { | ||
if (value >= sizes.trillion.value) { | ||
size = 'trillion'; | ||
} | ||
|
||
|
@@ -75,17 +55,18 @@ const formatToSize = ({value, size = 'thousand', precision = 0}: FormatToSizeArg | |
}; | ||
|
||
const addSizeLabel = (result: string, size: Digits, delimiter = UNBREAKABLE_GAP) => { | ||
return result + delimiter + sizes[size].label; | ||
const label = sizes[size].label; | ||
if (!label) { | ||
return result; | ||
} | ||
|
||
return result + delimiter + label; | ||
}; | ||
|
||
/** | ||
* @param significantDigits - number of digits above 3 | ||
*/ | ||
export const formatNumberWithDigits = ({ | ||
value, | ||
size, | ||
withSizeLabel = true, | ||
significantDigits = 0, | ||
delimiter, | ||
...params | ||
}: FormatValuesArgs<Digits>) => { | ||
|
@@ -95,7 +76,7 @@ export const formatNumberWithDigits = ({ | |
|
||
const numValue = Number(value); | ||
|
||
const sizeToConvert = size ?? getNumberWithSignificantDigits(numValue, significantDigits); | ||
const sizeToConvert = size ?? getNumberSizeUnit(numValue); | ||
|
||
const result = formatToSize({value: numValue, size: sizeToConvert, ...params}); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we don't use
significantDigits
param any more. What do you think about remove it completely? It seems it will simplify logic for formatting values a lot.