-
Notifications
You must be signed in to change notification settings - Fork 65
Added truncate and truncateAfter options to numeric pipe #1823
Added truncate and truncateAfter options to numeric pipe #1823
Conversation
To be consumed by future currency input control in development Resolves: blackbaud#1643
Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED Commit: ce8bf1f (Please note that this is a fully automated comment.) |
Codecov Report
@@ Coverage Diff @@
## master #1823 +/- ##
==========================================
- Coverage 99.98% 99.98% -0.01%
==========================================
Files 410 411 +1
Lines 8592 8535 -57
Branches 1269 1256 -13
==========================================
- Hits 8591 8534 -57
Misses 1 1
Continue to review full report at Codecov.
|
1234567 without truncation | ||
</sky-definition-list-label> | ||
<sky-definition-list-value> | ||
{{1234567 | skyNumeric:{truncate: false} }} |
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.
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.
I didn’t mess with the default for the digits option which is 1 in the NumericOptions class. When not truncating a decimal, the min and max fraction value are both set to that digits option so as to not mess with the output.
I didn’t mess with the default for the digits option which is 1 in the NumericOptions class. When not truncating a decimal, the min and max fraction value are both set to that digits option so as to not mess with the output.
From: Steve Brush [mailto:[email protected]]
Sent: Friday, July 13, 2018 12:35 PM
To: blackbaud/skyux2 <[email protected]>
Cc: Jacob Wood <[email protected]>; Author <[email protected]>
Subject: Re: [blackbaud/skyux2] Added truncate and truncateAfter options to numeric pipe (#1823)
@Blackbaud-SteveBrush requested changes on this pull request.
________________________________
In src/demos/numeric/numeric-demo.component.html<#1823 (comment)>:
@@ -115,6 +115,22 @@
{{1234567 | skyNumeric}}
</sky-definition-list-value>
</p>
+ <p>
+ <sky-definition-list-label>
+ 1234567 without truncation
+ </sky-definition-list-label>
+ <sky-definition-list-value>
+ {{1234567 | skyNumeric:{truncate: false} }}
This produced the following:
[Image removed by sender. screen shot 2018-07-13 at 12 27 58 pm]<https://user-images.githubusercontent.com/12497062/42702912-e3e229b6-8698-11e8-9673-59946e0cb702.png>
Is that expected, or should it just be 1,234,567?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#1823 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AmmBkKS7p1N7BGIyCHEZUoRVfcVAva6vks5uGMwXgaJpZM4VOyp2>.
|
15501 with truncation after 10000 and 1 digit | ||
</sky-definition-list-label> | ||
<sky-definition-list-value> | ||
{{15501 | skyNumeric:{digits: 1, truncateAfter: 10000} }} |
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-JacobWood If I change truncateAfter
to 1000
, the result doesn't change. Is that expected?
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.
Yes, original affects of the this component would only occur when the value
is greater than the truncateAfter
, with 15501 being greater than both 1000
and 10000
they would be the same. When set to 10000
, the value 9999
should not be truncated.
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.
I gotcha, thanks for the explanation!
Closing in favor of: #1856 |
Tests failed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 899ef89 (Please note that this is a fully automated comment.) |
To be consumed by future currency input control in development
Resolves: #1643