redash icon indicating copy to clipboard operation
redash copied to clipboard

replaced numeral-js with native numbers and percent formatting

Open AndrewChubatiuk opened this issue 1 year ago • 5 comments

What type of PR is this?

replaced numeral js with a native numbers formatting

  • [x] Refactor
  • [ ] Feature
  • [ ] Bug Fix
  • [ ] New Query Runner (Data Source)
  • [ ] New Alert Destination
  • [ ] Other

Description

How is this tested?

  • [ ] Unit tests (pytest, jest)
  • [ ] E2E Tests (Cypress)
  • [ ] Manually
  • [ ] N/A

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

AndrewChubatiuk avatar May 04 '24 20:05 AndrewChubatiuk

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 63.76%. Comparing base (372adfe) to head (328dc16). Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6945   +/-   ##
=======================================
  Coverage   63.76%   63.76%           
=======================================
  Files         161      161           
  Lines       13085    13085           
  Branches     1812     1812           
=======================================
  Hits         8344     8344           
  Misses       4438     4438           
  Partials      303      303           

codecov[bot] avatar May 04 '24 20:05 codecov[bot]

With this "native numbers" approach, does that mean it'll handle numbers larger than MAX_SAFE_INTEGER?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER

justinclift avatar May 05 '24 02:05 justinclift

With this "native numbers" approach, does that mean it'll handle numbers larger than MAX_SAFE_INTEGER?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER

it parses numbers from string, so length can be huge enough. it has more powerful functionality than numeral-js https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat

AndrewChubatiuk avatar May 05 '24 03:05 AndrewChubatiuk

@eradman What do you reckon? You're our resident front end expert. :smile:

justinclift avatar May 05 '24 06:05 justinclift

I can't review this with my current skill set, as I don't know Typescript at all (yet), and it's not likely for my to start learning it in the near future.

So, it'll need someone else to look over it.

justinclift avatar May 06 '24 12:05 justinclift