Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat: add duration formatter #209

Merged
merged 2 commits into from
Aug 26, 2019
Merged

feat: add duration formatter #209

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92bfbbb
feat: add duration formatter
villebro Aug 24, 2019
e718a0b
fix: address review comments
villebro Aug 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/superset-ui-number-format/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
},
"dependencies": {
"@types/d3-format": "^1.3.0",
"d3-format": "^1.3.2"
"d3-format": "^1.3.2",
"pretty-ms": "^5.0.0"
},
"peerDependencies": {
"@superset-ui/core": "^0.11.0"
Expand Down
7 changes: 7 additions & 0 deletions packages/superset-ui-number-format/src/NumberFormats.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ const DOLLAR_SIGNED = '+$,.2f';
const DOLLAR_ROUND = '$,d';
const DOLLAR_ROUND_SIGNED = '+$,d';

const DURATION = 'DURATION';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert the changes to this file. The duration formatter should be optional and not registered by default. There are other apps using this package and some do not need duration formatter. You can register these formatters in incubator-superset's setupFormatter.js. That way other apps can take advantage of tree-shaking and won't have to bundle pretty-ms.

const DURATION_MS = 'DURATION_MS';
const DURATION_S = 'DURATION_S';

const FLOAT_1_POINT = ',.1f';
const FLOAT_2_POINT = ',.2f';
const FLOAT_3_POINT = ',.3f';
Expand Down Expand Up @@ -39,6 +43,9 @@ const NumberFormats = {
DOLLAR_ROUND,
DOLLAR_ROUND_SIGNED,
DOLLAR_SIGNED,
DURATION,
DURATION_MS,
DURATION_S,
FLOAT,
FLOAT_1_POINT,
FLOAT_2_POINT,
Expand Down
4 changes: 4 additions & 0 deletions packages/superset-ui-number-format/src/NumberFormatterRegistry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { RegistryWithDefaultKey, OverwritePolicy } from '@superset-ui/core';
import createD3NumberFormatter from './factories/createD3NumberFormatter';
import createDurationFormatter from './factories/createDurationFormatter';
import createSmartNumberFormatter from './factories/createSmartNumberFormatter';
import NumberFormats from './NumberFormats';
import NumberFormatter from './NumberFormatter';
Expand All @@ -14,6 +15,9 @@ export default class NumberFormatterRegistry extends RegistryWithDefaultKey<
overwritePolicy: OverwritePolicy.WARN,
});

this.registerValue(NumberFormats.DURATION, createDurationFormatter());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert the changes to this file. The duration formatter should be optional and not registered by default. There are other apps using this package and some do not need duration formatter. You can register these formatters in incubator-superset's setupFormatter.js. That way other apps can take advantage of tree-shaking and won't have to bundle pretty-ms when they are not using.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add a section in README to instruct how to make duration formatter available.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok that makes sense. I wasn't able to find setupFormatter.js or any reference to getNumberFormatterRegistry() in incubator-superset, but I think I have a good idea of how it should be done. So if it's ok I'll open a new PR to add instructions to README.md once 0.12 is released and durationFormatter gets implemented in the backend.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I made a typo. It is setupFormatters.js. For README I meant packages/superset-ui-number-format/README.md

this.registerValue(NumberFormats.DURATION_MS, createDurationFormatter({ multiplier: 1 }));
this.registerValue(NumberFormats.DURATION_S, createDurationFormatter({ multiplier: 1000 }));
this.registerValue(NumberFormats.SMART_NUMBER, createSmartNumberFormatter());
this.registerValue(
NumberFormats.SMART_NUMBER_SIGNED,
Expand Down
21 changes: 21 additions & 0 deletions packages/superset-ui-number-format/src/factories/createDurationFormatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import prettyMsFormatter from 'pretty-ms';
import NumberFormatter from '../NumberFormatter';
import NumberFormats from '../NumberFormats';

export default function createDurationFormatter(
config: {
villebro marked this conversation as resolved.
Show resolved Hide resolved
description?: string;
id?: string;
label?: string;
multiplier?: number;
} = {},
) {
const { description, id, label, multiplier = 1 } = config;

return new NumberFormatter({
description,
formatFunc: value => prettyMsFormatter(value * multiplier),
id: id || NumberFormats.DURATION,
label: label || `Duration formatter`,
});
}
1 change: 1 addition & 0 deletions packages/superset-ui-number-format/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export {
} from './NumberFormatterRegistrySingleton';

export { default as createD3NumberFormatter } from './factories/createD3NumberFormatter';
export { default as createDurationFormatter } from './factories/createDurationFormatter';
export {
default as createSiAtMostNDigitFormatter,
} from './factories/createSiAtMostNDigitFormatter';
3 changes: 3 additions & 0 deletions packages/superset-ui-number-format/test/NumberFormatterRegistry.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ describe('NumberFormatterRegistry', () => {
it('return the value with the specified format', () => {
expect(registry.format('.2f', 100)).toEqual('100.00');
expect(registry.format(',d', 100)).toEqual('100');
expect(registry.format('DURATION', 1000)).toEqual('1s');
expect(registry.format('DURATION_MS', 1000)).toEqual('1s');
expect(registry.format('DURATION_S', 1)).toEqual('1s');
});
});
});
29 changes: 29 additions & 0 deletions packages/superset-ui-number-format/test/factories/createDurationFormatter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import NumberFormatter from '../../src/NumberFormatter';
import createDurationFormatter from '../../src/factories/createDurationFormatter';

describe('createDurationFormatter()', () => {
it('creates an instance of NumberFormatter', () => {
const formatter = createDurationFormatter();
const formatterMs = createDurationFormatter({ multiplier: 1 });
const formatterS = createDurationFormatter({ multiplier: 1000 });
expect(formatter).toBeInstanceOf(NumberFormatter);
expect(formatterMs).toBeInstanceOf(NumberFormatter);
expect(formatterS).toBeInstanceOf(NumberFormatter);
});
it('format milliseconds in human readable format', () => {
const formatter = createDurationFormatter();
expect(formatter(0)).toBe('0ms');
expect(formatter(1000)).toBe('1s');
expect(formatter(1337)).toBe('1.3s');
expect(formatter(60 * 1000)).toBe('1m');
expect(formatter(90 * 1000)).toBe('1m 30s');
});
it('format seconds in human readable format', () => {
const formatter = createDurationFormatter({ multiplier: 1000 });
expect(formatter(0.5)).toBe('500ms');
expect(formatter(1)).toBe('1s');
expect(formatter(30)).toBe('30s');
expect(formatter(60)).toBe('1m');
expect(formatter(90)).toBe('1m 30s');
});
});
2 changes: 2 additions & 0 deletions packages/superset-ui-number-format/test/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
createD3NumberFormatter,
createDurationFormatter,
createSiAtMostNDigitFormatter,
formatNumber,
getNumberFormatter,
Expand All @@ -13,6 +14,7 @@ describe('index', () => {
it('exports modules', () => {
[
createD3NumberFormatter,
createDurationFormatter,
createSiAtMostNDigitFormatter,
formatNumber,
getNumberFormatter,
Expand Down