shapeshift / web

ShapeShift Web
https://app.shapeshift.com
MIT License
168 stars 178 forks source link

default currency format is unusable with multiple specific iOS mobile keyboards #7060

Closed NeOMakinG closed 5 months ago

NeOMakinG commented 5 months ago

Overview

On my french phone, with the default currency number formatter, I can't use any swap/bridge/send because I don't have access to the dot key on the keyboard which is required for amounts under 0 (eg 0.1 ETH, 0.0001 BTC):

image

If I select another format:

image

It's not working fine as it uses coma for amounts under 0

References and additional details

We might want to adapt the default currency format based on the local lang or change the keyboard is it's technically possible.

Acceptance Criteria

I'm able to perform swap/bridge/send operations by default right after installing the mobile application if I have a french or any other keyboards using commas as under 0 separator

Need By Date

No response

Screenshots/Mockups

No response

Estimated effort

No response

NeOMakinG commented 5 months ago

@firebomb1 you might be interested by this one as well as it seems you are the translation 🐐

gomesalexandre commented 5 months ago

I'm able to repro as well:

And despite the two former, still seing the comma formatter, so assuming region takes precedence here?

NeOMakinG commented 5 months ago

I couldn't reproduce on android as the android keyboard has both comma and dots

firebomb1 commented 5 months ago

Thanks for the ping! Same, no issue on Android. I'm chatting with @NeOMakinG on Discord as I don't have an iOS or macOS device to test this, but I've suggested few solutions we can test.

NeOMakinG commented 5 months ago

After investigation:

Keyboard display depends on the Region:

US region uses the . separator

image

while if as an example you are french, you have the , separator

There are no ways to tweak that for now, except using the number or text input type with a decimal allowance pattern, which would allow some other characters than numbers, ',' and '.'

If we choose the number solution but without the numeric keyboard, should we do this only for iOS devices as this problem is with iOS? 2 choices:

0xean commented 5 months ago

https://github.com/zoontek/react-native-localize - maybe able to leverage something like this

NeOMakinG commented 5 months ago

Finally after further investigations, and thanks to @0xean suggestion, we have multiple choices, one particular looks very interesting to me:

More context:

Changing the "Region" change the number format by default and you can change the number format if you want

image
0xean commented 5 months ago

react-native-localize permits to get the number formatting selected in the settings (https://github.com/zoontek/react-native-localize?tab=readme-ov-file#getnumberformatsettings), we could leverage this to change the default settings of our formatter based on the number formatting selected on the device, be careful we might need to handle the fact that the user changed the app settings and so display another keyboard in that case? (We could simply add a formatter field to the window object and so the shapeshift app would detect it to change the default formatter)

Can you give an idea of the level of effort to get this done?

NeOMakinG commented 5 months ago

Honestly it's pretty simple, I would say 1: Here are the 4 settings we need to push to the window object through the webview:

if one of these 4 settings are existing on window on the shapeshift web side, change the default CurrencyFormats to the matching one (we will need a mapping between thoses 4 settings and 4 default CurrencyFormats, a simple object to map them)

image

We need a way to use the decimal input mode if the current setting matches the default setting with the window attribute, or use the base keyboard with more than numbers if the window attribute exists and the default settings doesn't match. For this, I would probably introduce a useNumberInputMode hook to consume inside all those occurrences

TLDR: 2 small PR, one on mobile side, one on web side, the biggest one is on web side: change the default setting matching the window attribute, then when you render any number input you have to choose between "the decimal inputMode is usable or nop it's too risky let's switch to the basic keyboard"

0xean commented 5 months ago

Okay, if its gonna take a mobile PR, lets get it done because releasing is a pain and we can roll it into the current release. This essentially is a bug, so doesn't need to go through the typical grooming process.

NeOMakinG commented 5 months ago

Okay, if its gonna take a mobile PR, lets get it done because releasing is a pain and we can roll it into the current release. This essentially is a bug, so doesn't need to go through the typical grooming process.

Ok cool, jumping on it right now

NeOMakinG commented 5 months ago

Finally, thanks to @gomesalexandre we found that our library accepts allowedDecimalSeparators... It's way simpler than expected then