-
Notifications
You must be signed in to change notification settings - Fork 65
Contrib > Added truncate and truncateAfter options to numeric pipe #1856
Contrib > Added truncate and truncateAfter options to numeric pipe #1856
Conversation
To be consumed by future currency input control in development Resolves: #1643
Created #1857 to document the new options. |
Codecov Report
@@ Coverage Diff @@
## master #1856 +/- ##
======================================
Coverage 100% 100%
======================================
Files 412 414 +2
Lines 8614 8640 +26
Branches 1272 1278 +6
======================================
+ Hits 8614 8640 +26
Continue to review full report at Codecov.
|
…ud/skyux2 into contrib-feature-numeric-truncate
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.
Just the one check for the default. Everything else looks fine 👍
@@ -2,4 +2,6 @@ export class NumericOptions { | |||
public digits = 1; | |||
public format = 'number'; | |||
public iso = 'USD'; | |||
public truncate = true; |
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.
Are you sure we want this to default to true? Won't that immediately truncate/"break" anyone currently using the skyNumeric filter?
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.
@blackbaud-conorwright The “truncate” boolean is a new option, so we wouldn’t have to worry about a breaking change with that setting in particular.
Also, the pipe as it stands today truncates automatically; this would allow users to turn off the truncate ability altogether.
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.
Oh alright. I know it's a new option and was worried that having a new option default to applying its effect would be a breaking change. Though if it was always truncating beforehand then this is correct. All good then 👍
Original contribution: #1823
Resolves: #1643