-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(Amount): Negative Amount support #2320
Conversation
|
✅ PR title follows Conventional Commits specification. |
Pausing on this for some time since it got a little large task. Will pick it up as proper task in next sprint |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit d9e314d:
|
/** | ||
* Pollyfill function to get around the node 18 error | ||
* | ||
* This function is maintained by i18nify team. Reach out to them for any change regarding this. |
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.
Should this come from i18nify library itself then instead of being in blade repo?
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 did suggest them this but they are saying they'll just document it on their end in documentation and consumers will have to add it. Can discuss it later separately as well. Wanted to get it resolved earlier for negative amount thing. But yup overall I too feel this should be in their repo
/** | ||
* Wrapper that uses pollyfill of i18nify team | ||
*/ | ||
const pollyfilledFormatNumberByParts: typeof formatNumberByParts = (value, options) => { |
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.
Also since this is a polyfill are we conditionally calling it or are we always calling it regardless.
eg: checking for formatToParts existence
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've put conditions inside this function itself. So we can always call this instead of formatNumberByParts
. formatToParts is supported in most major browsers we target so its fine to not check that
Description
Changes
Additional Information
Component Checklist