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

[Wallet] Hotfix local currency #1481

Merged
merged 4 commits into from
Oct 25, 2019
Merged

Conversation

jeanregisser
Copy link
Contributor

Description

Fixed some regressions introduced by #1325

  • crash when changing the local currency from the settings (caused by incorrect usage of react hooks)
  • incorrect exchange rate values displayed for exchanges
  • gold received from faucet showing as local currency

This will probably need further refactoring, the logic is easy to break given the different use cases.

Tested

image
image
image
image
image
image

Other changes

N/A

Related issues

Backwards compatibility

Yes

This is to prevent incorrect usage of hooks
- fix crash when changing local currency from the settings due to hooks
incorrectly used
- fix dollar exchange rate value shown converted to local currency in the
exchange confirmation card
- fix received gold amounts shown in the local currency
@jeanregisser jeanregisser force-pushed the jeanregisser/hotfix-local-currency branch from 02863db to 00ea1cc Compare October 25, 2019 12:30
Comment on lines +17 to 18
// tslint:disable-next-line: react-hooks-nesting
const selectedCurrencyCode = useLocalCurrencyCode() || DEFAULT_CURRENCY_CODE
Copy link
Contributor Author

Choose a reason for hiding this comment

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

false positive because we're using || DEFAULT_CURRENCY_CODE

@@ -67,11 +67,12 @@ const renderTopSection = (props: Props) => {
const renderAmountSection = (props: Props) => {
const { currency, type, value } = props

// tslint:disable react-hooks-nesting
Copy link
Contributor Author

Choose a reason for hiding this comment

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

false positive because this function is called inside another render function and that lint rules can't figure it out.

@codecov
Copy link

codecov bot commented Oct 25, 2019

Codecov Report

Merging #1481 into master will increase coverage by <.01%.
The diff coverage is 67.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1481      +/-   ##
==========================================
+ Coverage   65.49%   65.49%   +<.01%     
==========================================
  Files         271      271              
  Lines        8144     8165      +21     
  Branches      491      501      +10     
==========================================
+ Hits         5334     5348      +14     
- Misses       2697     2699       +2     
- Partials      113      118       +5
Flag Coverage Δ
#mobile 65.49% <67.64%> (ø) ⬆️
Impacted Files Coverage Δ
...s/mobile/src/localCurrency/SelectLocalCurrency.tsx 87.5% <ø> (ø) ⬆️
...kages/mobile/src/send/TransferConfirmationCard.tsx 83.67% <100%> (+2.04%) ⬆️
...kages/mobile/src/transactions/TransferFeedItem.tsx 83.33% <100%> (+0.87%) ⬆️
...kages/mobile/src/transactions/ExchangeFeedItem.tsx 80.32% <55%> (-8.14%) ⬇️
...s/mobile/src/exchange/ExchangeConfirmationCard.tsx 92.5% <77.77%> (-4.28%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2dee1ca...e337c28. Read the comment docs.

@jmrossy jmrossy merged commit e8a2ecd into master Oct 25, 2019
@jmrossy jmrossy deleted the jeanregisser/hotfix-local-currency branch October 25, 2019 13:19
aaronmgdr added a commit that referenced this pull request Oct 29, 2019
…repo into aaronmgdr/invite-page

* 'aaronmgdr/invite-page' of github.com:celo-org/celo-monorepo: (63 commits)
  Fix compile error during local clean build (#1506)
  Update to RN 61 and AndroidX (#1343)
  Set usage of shuffled round robin in the genesis block (#1464)
  Add spanish backup key for backup key flow (#1500)
  Fix sync tests by pulling genesis block to determine epoch length (#1504)
  [Wallet] fix missing full name error alert (#1496)
  [Wallet + Verification Pool] Add details about generating an app-signature (#1482)
  Deploy celo's image of ethstats (#1421)
  Storing previous randomness values (#1197)
  [Wallet] Fix wei invite bug (#1489)
  Point all packages to latest ganache-cli master (#1488)
  Point end-to-end tests back to master (#1469)
  [Wallet] Migrate app view functions to contractkit (#1381)
  [Wallet] Add script to translate locale strings (#1485)
  [Wallet] Update wallet celo client version and add missing translations for backup flow (#1483)
  [Wallet] Hotfix local currency (#1481)
  [Wallet] Remove QR debouncing to improve responsiveness (#1480)
  [Wallet] Upgrade app version to v1.5.1 (#1463)
  Update governance end-to-end tests to work with changed precompile (#1476)
  Fixes key_placer.sh when encrypting files (#1465)
  ...
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.

Application crashes when changing the local currency
2 participants