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

feat: make formatTime and formatNumber accept undefined type #307

Merged
merged 1 commit into from
Mar 4, 2020

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Mar 4, 2020

🏆 Enhancements

  • Make formatTime and formatNumber handle undefined type. The logic is already there, but the argument type was still string

cc @ktmud

@kristw kristw requested a review from a team as a code owner March 4, 2020 19:49
@codecov
Copy link

codecov bot commented Mar 4, 2020

Codecov Report

Merging #307 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #307   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         103    103           
  Lines        1223   1223           
  Branches      300    300           
=====================================
  Hits         1223   1223
Impacted Files Coverage Δ
...et-ui-number-format/src/NumberFormatterRegistry.ts 100% <ø> (ø) ⬆️
...perset-ui-time-format/src/TimeFormatterRegistry.ts 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b95092...f939a37. Read the comment docs.

@@ -43,5 +43,8 @@ describe('NumberFormatterRegistry', () => {
expect(registry.format('.2f', 100)).toEqual('100.00');
expect(registry.format(',d', 100)).toEqual('100');
});
it('falls back to the default formatter if the format is undefined', () => {
expect(registry.format(undefined, 1000)).toEqual('1k');
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding the test case!

@netlify
Copy link

netlify bot commented Mar 4, 2020

Deploy preview for superset-ui ready!

Built with commit f939a37

https://deploy-preview-307--superset-ui.netlify.com

Copy link
Contributor

@ktmud ktmud left a comment

Choose a reason for hiding this comment

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

:lgtm:

@kristw kristw merged commit 768cfd5 into master Mar 4, 2020
@delete-merged-branch delete-merged-branch bot deleted the kristw--optional branch March 4, 2020 20:10
@ktmud
Copy link
Contributor

ktmud commented Mar 4, 2020

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants