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

Use different formatter for different currency #1432

Merged
merged 14 commits into from
Dec 2, 2020

Conversation

xuhongxu96
Copy link
Contributor

@xuhongxu96 xuhongxu96 commented Nov 12, 2020

Fixes #424 , #1204 , #1438 .

Before, the Calculator will use CurrencyFormatter per system locale for all currencies.
So, users cannot input USD with fraction digits in Japanese locale, because JPY doesn't allow fraction digits.

Description of the changes:

  • Allow to use different formatter for different currency
    • By adding m_currencyFormatter1, m_currencyFormatter2

How changes were validated:

  • Functional UI tests (it does not work because there is no full currency list in CI)
  • Add unit test case TestCurrencyFormattingLogic for UnitConverterViewModel

Copy link
Contributor

@joseartrivera joseartrivera left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! This seems to be working great, just a few questions to answer, let me know what you think.

src/CalcViewModel/UnitConverterViewModel.cpp Show resolved Hide resolved
src/CalcViewModel/UnitConverterViewModel.cpp Outdated Show resolved Hide resolved
src/CalcViewModel/UnitConverterViewModel.cpp Show resolved Hide resolved
Reset currency before tests
Fix: input is blocked after switching to currency with less fractional digits
To fix incorrect result after switching currency with less fractional digits
Copy link
Contributor

@joseartrivera joseartrivera left a comment

Choose a reason for hiding this comment

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

Works great, thanks again for the fix!

@joseartrivera joseartrivera merged commit 61d06b2 into microsoft:master Dec 2, 2020
@xuhongxu96 xuhongxu96 deleted the hox/fix-424 branch December 3, 2020 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Application fails to take float value for Currency converter when Japanese Calendar is enabled
3 participants