shapeshift / web

ShapeShift Web
https://app.shapeshift.com
MIT License
159 stars 180 forks source link

feat: multi currency support #1873

Closed 0xdef1cafe closed 1 year ago

0xdef1cafe commented 2 years ago

Description

Notice

Pull Request Type

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

cloudflare-pages[bot] commented 2 years ago

Deploying with  Cloudflare Pages  Cloudflare Pages

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

View logs

0xdef1cafe commented 1 year ago

converting to draft until @stackedq is able to look at currency formatting and chart issues

https://discord.com/channels/554694662431178782/984598894883250226

willyogo commented 1 year ago

Question from @stackedq:

GM y'all, Multi currency support (https://github.com/shapeshift/web/pull/1873) needs some decisions on the way forward, currently the browser locale remains as "en-US" no matter which language or currency is selected, this introduce a problem where numbers are always shown like 1,234.56 instead of 1.234,56 which is the standard format for some locales out there. there are three ways we can make this:

Product's answer: Can we give users the following settings?

  1. Language (default english)
  2. Currency (default USD)
  3. Number format (either 1,234.56 or 1.234,56, with default of 1,234.56)
stackedq commented 1 year ago

@willyogo definitely, is there any UI design in your mind for number format selection?

0xdef1cafe commented 1 year ago

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 commented 1 year ago

next issue to implement trade support https://github.com/shapeshift/web/issues/1995

gomesalexandre commented 1 year ago

@stackedq Some more conflicts after https://github.com/shapeshift/web/pull/2116 has been merged

reallybeard commented 1 year ago

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 commented 1 year ago

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

gomesalexandre commented 1 year ago

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

reallybeard commented 1 year ago

Looks good @stackedq working really nice!

0xdef1cafe commented 1 year ago

going to merge this for the following reasons