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.

Sign up for GitHub

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

Jump to bottom

feat(@formatjs/intl-numberformat): implemented formatNumericRange and formatNumericRangeToParts #3860

Merged
merged 9 commits into from
Oct 30, 2022
Merged

feat(@formatjs/intl-numberformat): implemented formatNumericRange and formatNumericRangeToParts #3860

merged 9 commits into from
Oct 30, 2022

Conversation

ilteoood
Copy link
Contributor

@ilteoood ilteoood commented Oct 5, 2022 •
edited
Loading

Hi,
with this PR I implemented both formatNumericRange and formatNumericRangeToParts, with their dedicated abstract functions.

cc: @simoneb @micheleriva

@ilteoood ilteoood changed the title feat(@formatjs/intl,react-intl): implemented formatNumericRange and formatNumericRangeToParts feat(@formatjs/intl-numberformat): implemented formatNumericRange and formatNumericRangeToParts Oct 6, 2022
@ilteoood ilteoood requested a review from longlho October 29, 2022 14:27
@ilteoood
Copy link
Contributor Author

Hy @longlho,
I've addressed the comments about hardcoded separator and the NumberFormat input.

The only missing one is about spacing. Should it be addressed in another PR?

@longlho
Copy link
Member

longlho commented Oct 29, 2022

Yeah that's ok can you make a GH issue to track it?

@ilteoood ilteoood mentioned this pull request Oct 29, 2022
Closed
@ilteoood
Copy link
Contributor Author

Yeah that's ok can you make a GH issue to track it?

I opened #3896

@longlho longlho merged commit 113a169 into formatjs:main Oct 30, 2022
@longlho
Copy link
Member

longlho commented Oct 30, 2022

Thanks a lot for your contributions!

@sffc sffc mentioned this pull request Nov 16, 2022
Open
@klimashkin
Copy link

klimashkin commented May 2, 2023 •
edited
Loading

Quick questions, just to understand.
This change doesn't yet introduce formatRange from v3, right?

Version 8.3.0 says it adds formatNumericRange, but it's not in https://github.com/tc39/proposal-intl-numberformat-v3, so I was a bit confused

@klimashkin
Copy link

klimashkin commented May 3, 2023 •
edited
Loading

Actually, seems like this adds support for formatRange: 113a169#diff-b3e50f88c036f990339b6dfd60bd482a5beb27caac9c9b84c802010974eff4da

What confused me is that intl-numberformat/should-polyfill is not returning true for me if formatRange is not supported, for example, in current Firefox 112 (native support will be added in 116)

unional pushed a commit to unional/formatjs that referenced this pull request Jun 7, 2023
@ilteoood @unional
feat(@formatjs/intl-numberformat): implemented formatNumericRange and…
8577a1c 
… formatNumericRangeToParts (formatjs#3860)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sffc sffc sffc left review comments

@teobmg teobmg teobmg left review comments

@longlho longlho longlho approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ilteoood @longlho @klimashkin @sffc @teobmg