feat: multi currency support #1873

0xdef1cafe · 2022-05-24T17:25:16Z

Description

builds on top of other PR where data fetching is implemented
adds view layer changes required to consume this data
adds settings selection switcher behind flag
swapping is not implemented in this PR - and will be added in another PR

Notice

Have you followed the guidelines in our Contributing guide?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Pull Request Type

🐛 Bug fix (Non-breaking Change: Fixes an issue)
🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

wip

Risk

significant - touches pretty much everything.

Testing

full regression test of the app with the flag OFF (default) required

Screenshots (if applicable)

no visual changes should be present

…ture

cloudflare-workers-and-pages · 2022-05-24T17:25:55Z

Deploying with Cloudflare Pages

Latest commit:	`5978a6e`
Status:	✅ Deploy successful!
Preview URL:	https://4454269f.web-29e.pages.dev

View logs

0xdef1cafe · 2022-07-20T17:27:06Z

update on this PR - the currency formatting issue on charts is fixed, and i can't see any chart regressions with this flag on and USD selected, or with the flag off.

@stackedq has implemented currency selection in the settings modal, i think we can merge this with the flag off, and tackle multi currency trade support in a separate PR, then get the flag on and feature released.

0xdef1cafe · 2022-07-20T18:58:08Z

next issue to implement trade support #1995

src/components/Modals/Settings/SettingsList.tsx

src/hooks/useFetchPriceHistories/useFetchPriceHistories.ts

src/components/Modals/Settings/FiatCurrencies.tsx

gomesalexandre

Tested both with the flag on and off:

With the flag on, I couldn't find any place where the selected currency wasn't displayed.
Seeing the right separator for Euro when selecting French as a language which seems to be the intent from looking at the implementation, but I'm not sure is right. Such separator should be determined by the currency, not the language. Happy for this to be improved in a follow-up PR.
The currency is always present before the value, which should be after by convention in the case of e.g Euro. I'm not sure if this is something we want to be granular with depending on the selected currency, but if so, it should be matching the convention of said currency. Again, happy for this to be tackled in a follow-up PR
I didn't notice any regressions on the existing behavior with the flag off, so I'm happy to merge this as is after the merge conflicts and the blocking comments are tackled.

* feat: init walletconnect integration * chore: lint * fix: wallet init * fix: wallet init * fix: add imagedelivery to headers * chore: use package version * chore: show/hide WalletConnect based on flag * fix: headers for macos/ios safari * fix: show feature flag nav item for alt host * perf: add env var for better local development * chore: remove unecessary code * chore: remove package dep * chore: add hdwallet-walletconnect alpha dependency * fix: add walletconnect csp entries * fix: utilize feature flag * fix: add mobileEnabled field to WalletConnect config * chore: run linter * fix: remove usage of process.env * chore: run linter * fix: apply code review suggestions * fix: use structured logging * fix: apply code review suggestions * fix: update csp for wallet logos * chore: add flag to sample.env * chore: yarn.lock * docs: fleek info * fix: img source csp * chore: revert readme to develop * fix: image fetched by walletconnect * feat: WalletConnect rejection * chore: lint * chore: lint * chore: update hdwallet-walletconnect dependency * chore: upgrade hdwallet-walletconnect * fix: set wc connect modal error to null on init * chore: run linter * fix: walletconnect refresh bug * chore: upgrade hdwallet dependency * chore: bump hdwallet dependencies * fix: csp for walletconnect desktop logos * chore: bump hdwallet package versions * fix: disable trade max with walletconnect * fix: disable sendmax with walletconnect * chore: run linter * chore: update hdwallet dependencies * chore: run linter * fix: hide fiat sendMax with WalletConnect * fix: stylistic changes * fix: typo * feat: add copy to walletconnect option in selectmodal * fix: constrain icon width for selectmodal options * fix: update return value Co-authored-by: pastaghost <[email protected]> Co-authored-by: 0xdef1cafe <[email protected]>

Bumps [terser](https://github.com/terser/terser) from 4.8.0 to 4.8.1. - [Release notes](https://github.com/terser/terser/releases) - [Changelog](https://github.com/terser/terser/blob/master/CHANGELOG.md) - [Commits](https://github.com/terser/terser/commits) --- updated-dependencies: - dependency-name: terser dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: dogecoin initial * add dogecoin * use p2pkh accountType for dgubs add dogecoin else if to handleSend :( add dogecoin case to handleSendMax :( :( * wrap doge plugin in a feature flag * pass chainId to utxoAccountParams * remove reference to dogecoin testnet * web clean up * fix: update old utility to new caip method * chore: add REACT_APP_FEATURE_DOGECOIN=true to sample.env * fix: update swapper logic to pass correct param * chore: version bump doge libs * fix: add dogecoin label to accounts page * fix: don't show tokens under dogecoin account Co-authored-by: 0xdef1cafe <[email protected]> Co-authored-by: Apotheosis <[email protected]>

gomesalexandre · 2022-07-22T11:49:02Z

@stackedq Some more conflicts after #2116 has been merged

reallybeard · 2022-07-22T21:04:53Z

Fixed the conflicts with my new defi ui stuff, a lot of it isn't needed anymore since we let the amount component do the heavy lifting.

@stackedq I do have a suggestion on the currency selection, can we not sort the list on click? I'm fine with it being sorted on mount, but its really weird that it jumps the item to the top when you make a selection. Same goes for the currency format selection.

stackedq · 2022-07-23T11:11:51Z

@reallybeard changed some code to achieve the UX you described, please take a look to see if you're happy with the results.

gomesalexandre

Retested according to the previous criteria and it generally works as expected (minus a smallish comment WRT translations but that's for globalization and can be done separately) / didn't find any regression.

Re:

@stackedq I do have a suggestion on the currency selection, can we not sort the list on click? I'm fine with it being sorted on mount, but its really weird that it jumps the item to the top when you make a selection. Same goes for the currency format selection.

Tested this one out and that's behaving as suggested by @reallybeard as well.

Just me being a nit, but I've found it that the second "Currency Format" option isn't exactly matching the example in the option, see the option vs. the actual values formatting, missing the thousands separator:

Given this is such a small issue, and this is under feature flag, I'm going to accept this so we can move forward, and tackle small visual issues separately as follow-ups.
My biggest concern given the size of the PR would be whether or not this produces regressions, which from looking at the implementation and testing over, isn't the case.

🍬

src/assets/translations/en/main.json

src/components/AssetHeader/AssetMarketData.tsx

src/components/Modals/Settings/SettingsRouter.tsx

src/hooks/useFetchPriceHistories/useFetchPriceHistories.ts

gomesalexandre · 2022-07-23T14:39:21Z

Given the high risk of this, let's get @shapeshift/operations to test the Fleek branch

reallybeard · 2022-07-23T15:18:31Z

Looks good @stackedq working really nice!

gomesalexandre

Tested again as a sanity check, still working as previously.
The currency format option value is also now matching the actual formatting

🍬

0xdef1cafe · 2022-07-25T18:03:26Z

going to merge this for the following reasons

architecturally, this was previously discussed and approved by @cjthompson and recently tested and approved by @gomesalexandre and @reallybeard
@shapeshift/operations has signed off on this today
it's behind a flag
this branch is a bastard child

0xdef1cafe added 22 commits May 20, 2022 12:08

chore: update packages

0ebecab

chore: add new headers

2223141

feat: add new translations

7378dde

feat: add fiat market data

39e429f

feat: fetch fiat market data in app context

caaca76

chore: blacklist large things from redux store

425ac00

fix: more correct type defs and balance chart fixes

d1e2814

chore: fix headers comment

bc599ae

chore: consolidate to priceAtDate

aca507e

chore: only upgrade market service package

c4a4fba

chore: use same source of truth for mock test data as real data struc…

6e28f9f

…ture

fix: optimizations and bug fixes

0dba950

fix: market data refetch

844a4c5

chore: merge develop

bbf9c8e

Merge branch 'develop' into fiat-market-data-slice

f69d726

chore: rename booleans for readability

15c5d6c

chore: tighter type def for setFiatMarketData

61d58f4

chore: freeze initial price history for safety

a855abf

chore: uppercase const

56382a4

chore: consistent market data selector naming

bd4e5a0

feat: view layer changes to support multi currency display

149fa31

feat: add currency selection to settings modal

4e7e823

0xdef1cafe changed the base branch from develop to fiat-market-data-slice May 24, 2022 17:25

0xdef1cafe added 3 commits May 24, 2022 11:38

chore: add multi currency feature flag

fd2cfae

feat: balance chart changes

0bf62f0

chore: delete duplicate test with wrong file extension

4017fc3

Base automatically changed from fiat-market-data-slice to develop May 24, 2022 18:43

0xdef1cafe mentioned this pull request May 24, 2022

feat: fiat currencies support for application #1337

Closed

5 tasks

chore: merge develop

8e0b91d

0xdef1cafe requested a review from gomesalexandre July 20, 2022 18:51

chore: fix typo

8e6d4bf

kaladinlight reviewed Jul 20, 2022

View reviewed changes

src/components/Modals/Settings/SettingsList.tsx Outdated Show resolved Hide resolved

kaladinlight reviewed Jul 20, 2022

View reviewed changes

src/hooks/useFetchPriceHistories/useFetchPriceHistories.ts Show resolved Hide resolved

kaladinlight reviewed Jul 20, 2022

View reviewed changes

src/components/Modals/Settings/FiatCurrencies.tsx Outdated Show resolved Hide resolved

gomesalexandre reviewed Jul 20, 2022

View reviewed changes

GMSteuart and others added 6 commits July 21, 2022 07:11

fix: typo and lodash import

2535512

fix: conflict resolved

9a4018b

fix: conflict resolved

8cab0f6

Merge branch 'develop' into multi-currency-support

7d0b929

feat: keeping order of fiat currencies and currency formats

c4f1981

gomesalexandre previously approved these changes Jul 23, 2022

View reviewed changes

fix: applying suggested changes

cbd8ff4

stackedq dismissed gomesalexandre’s stale review via cbd8ff4 July 24, 2022 10:34

gomesalexandre approved these changes Jul 24, 2022

View reviewed changes

reallybeard approved these changes Jul 25, 2022

View reviewed changes

0xdef1cafe merged commit 56ba3b5 into develop Jul 25, 2022

0xdef1cafe deleted the multi-currency-support branch July 25, 2022 18:05

0xdef1cafe mentioned this pull request Jul 26, 2022

chore: create release v1.47.0 [DO NOT MERGE] #2239

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: multi currency support #1873

feat: multi currency support #1873

0xdef1cafe commented May 24, 2022 •

edited

Loading

cloudflare-workers-and-pages bot commented May 24, 2022 •

edited

Loading

0xdef1cafe commented Jul 20, 2022 •

edited

Loading

0xdef1cafe commented Jul 20, 2022

gomesalexandre left a comment

gomesalexandre commented Jul 22, 2022

reallybeard commented Jul 22, 2022 •

edited

Loading

stackedq commented Jul 23, 2022

gomesalexandre left a comment

gomesalexandre commented Jul 23, 2022

reallybeard commented Jul 23, 2022

gomesalexandre left a comment

0xdef1cafe commented Jul 25, 2022

feat: multi currency support #1873

feat: multi currency support #1873

Conversation

0xdef1cafe commented May 24, 2022 • edited Loading

Description

Notice

Pull Request Type

Issue (if applicable)

Risk

Testing

Screenshots (if applicable)

cloudflare-workers-and-pages bot commented May 24, 2022 • edited Loading

Deploying with Cloudflare Pages

0xdef1cafe commented Jul 20, 2022 • edited Loading

0xdef1cafe commented Jul 20, 2022

gomesalexandre left a comment

Choose a reason for hiding this comment

gomesalexandre commented Jul 22, 2022

reallybeard commented Jul 22, 2022 • edited Loading

stackedq commented Jul 23, 2022

gomesalexandre left a comment

Choose a reason for hiding this comment

gomesalexandre commented Jul 23, 2022

reallybeard commented Jul 23, 2022

gomesalexandre left a comment

Choose a reason for hiding this comment

0xdef1cafe commented Jul 25, 2022

0xdef1cafe commented May 24, 2022 •

edited

Loading

cloudflare-workers-and-pages bot commented May 24, 2022 •

edited

Loading

0xdef1cafe commented Jul 20, 2022 •

edited

Loading

reallybeard commented Jul 22, 2022 •

edited

Loading