Skip to content
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

fix(toDecimal): teach toDecimal how to handle negative units #690

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

jparise
Copy link
Contributor

@jparise jparise commented Dec 6, 2022

Previously, toDecimal would format an amount like -1050 as '-10.-50', when we'd expect '-10.50'. We now use the absolute value of all but the first unit when building the decimal string.

@vercel
Copy link

vercel bot commented Dec 6, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
dinerojs ✅ Ready (Inspect) Visit Preview Dec 9, 2022 at 5:40PM (UTC)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 6, 2022

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 8890d00:

Sandbox Source
@dinero.js/example-cart-react Configuration
@dinero.js/example-cart-vue Configuration
@dinero.js/example-pricing-react Configuration
@dinero.js/example-starter Configuration

Copy link
Collaborator

@sarahdayan sarahdayan left a comment

Choose a reason for hiding this comment

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

Great catch, thank you!

One small change and we'll be good to go.

packages/core/src/api/toDecimal.ts Outdated Show resolved Hide resolved
@sarahdayan sarahdayan changed the title fix: teach toDecimal how to handle negative units fix(toDecimal): teach toDecimal how to handle negative units Dec 9, 2022
Previously, toDecimal would format an amount like -1050 as '-10.-50',
when we'd expect '-10.50'. We now use the absolute value of all but the
first unit when building the decimal string.
Copy link
Collaborator

@sarahdayan sarahdayan left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@sarahdayan sarahdayan merged commit 81c5566 into dinerojs:main Dec 9, 2022
@jparise jparise deleted the todecimal-negative branch December 9, 2022 21:57
jparise added a commit to jparise/dinero.js that referenced this pull request Dec 10, 2022
Change dinerojs#690 fixed the general case of formatting negative unit values,
but was one remaining case where formatting was still incorrect: when
the first unit is -0, the resulting string didn't include the leading
negative sign.

This change identifies that exact case (negative value, leading zero)
and pads the resulting string with a leading negative sign.

Fixes dinerojs#692
jparise added a commit to jparise/dinero.js that referenced this pull request Dec 10, 2022
Change dinerojs#690 fixed the general case of formatting negative unit values,
but was one remaining case where formatting was still incorrect: when
the first unit is -0, the resulting string didn't include the leading
negative sign.

This change identifies that exact case (negative value, leading zero)
and pads the resulting string with a leading negative sign.

Fixes dinerojs#692
jparise added a commit to jparise/dinero.js that referenced this pull request Dec 11, 2022
Change dinerojs#690 fixed the general case of formatting negative unit values,
but was one remaining case where formatting was still incorrect: when
the first unit is -0, the resulting string didn't include the leading
negative sign.

This change identifies that exact case (negative value, leading zero)
and pads the resulting string with a leading negative sign.

Fixes dinerojs#692
jparise added a commit to jparise/dinero.js that referenced this pull request Dec 12, 2022
Change dinerojs#690 fixed the general case of formatting negative unit values,
but was one remaining case where formatting was still incorrect: when
the first unit is -0, the resulting string didn't include the leading
negative sign.

This change identifies that exact case (negative value, leading zero)
and pads the resulting string with a leading negative sign.

Fixes dinerojs#692
sarahdayan added a commit that referenced this pull request Dec 19, 2022
Change #690 fixed the general case of formatting negative unit values,
but was one remaining case where formatting was still incorrect: when
the first unit is -0, the resulting string didn't include the leading
negative sign.

This change identifies that exact case (negative value, leading zero)
and pads the resulting string with a leading negative sign.

Fixes #692

Co-authored-by: Sarah Dayan <5370675+sarahdayan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants