Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

Slippage settings #385

Closed NikolaiYurchenko closed 1 year ago

NikolaiYurchenko commented 1 year ago

https://github.com/Fujicracy/fuji-v2/issues/336 Screenshot 2023-03-17 at 16 22 15 Screenshot 2023-03-17 at 16 22 39 Screenshot 2023-03-17 at 16 23 16

ferostabio commented 1 year ago

@NikolaiYurchenko please have a look at this issues:

ferostabio commented 1 year ago

Looks great now @NikolaiYurchenko, thanks! Not merging yet until updating the sdk, already discussing with Boyan.

ferostabio commented 1 year ago

@NikolaiYurchenko I'm afraid I found one more issue: if the user enters a custom input, once it leaves and opens it shows as selected the previously selected button, not the custom one. The border is only updated for the default values, not the one with the input.

NikolaiYurchenko commented 1 year ago

selected the previously selected button, not the custom one.

So, I see, you want to save the previously selected value and prefill input with it. Makes sense. Sure, I'll make this change

ferostabio commented 1 year ago

selected the previously selected button, not the custom one.

So, I see, you want to save the previously selected value and prefill input with it. Makes sense. Sure, I'll make this change

That's not the bug I described:

Haven't looked at the code to see if the issue is that the custom value wasn't saved or just that the UI fails to reflect it.

@brozorec what do you think about the other point, should the screen reset to the default each time the user enters the screen or save the last value?

NikolaiYurchenko commented 1 year ago

@brozorec what do you think about the other point, should the screen reset to the default each time the user enters the screen or save the last value?

I think resetting to 0.3 is a better UX, but I am waiting for decision about it

brozorec commented 1 year ago

@brozorec what do you think about the other point, should the screen reset to the default each time the user enters the screen or save the last value?

I checked how Uniswap and Sushi deal with it, and they persist the inputted slippage over sessions. I suggest we do the same.

you want to save the previously selected value and prefill input with it.

This is not that great. Let's not prefill the custom input with one of the selected values. Also, when the user types a custom value, it would be good to highlight it with a border.

Here is more feedback:

1. When I land on the page the slippage settings doesn't appear, I need to do an action like changing the chain in order for it to get visible.

  1. When I go to the borrow or manage page, the button is in a loading state, although there's nothing to load yet. image
  2. Design and implementation differ, icon and border-radius, also the icon is not well centered. image image
  3. When the dialog is open the settings button should be highlighted as in the designs.
ferostabio commented 1 year ago
  1. When I go to the borrow or manage page, the button is in a loading state, although there's nothing to load yet.

I noticed this as well, double-checked just in case and it's not happening on the manage-position branch.

ferostabio commented 1 year ago

@NikolaiYurchenko if I understood @brozorec correctly, there is still one point missing:

NikolaiYurchenko commented 1 year ago

Save inputted slippage across sessions (do not make it the selected value, but if the user typed 0.9, the next time he enters the webpage, the inputted value should be 0.9 instead of 0.3)

I typed 0.33, closed menu, redirected to borrow, opened it again and it is 0.33 in input and it is focused. So if it is smth wrong with it please provide reproducing steps so i could fix unpredicted behaviour. thank you in advance

brozorec commented 1 year ago

@NikolaiYurchenko if I understood @brozorec correctly, there is still one point missing:

  • Save inputted slippage across sessions (do not make it the selected value, but if the user typed 0.9, the next time he enters the webpage, the inputted value should be 0.9 instead of 0.3)

Yes, that's correct. If we want to persist this value across sessions, we need to save it in the localstorage.

NikolaiYurchenko commented 1 year ago

Yes, that's correct. If we want to persist this value across sessions, we need to save it in the localstorage. I see, ok

ferostabio commented 1 year ago

@NikolaiYurchenko now the inputted value is selected by default after reloading the screen. The custom value should be stored across sessions, but shouldn't be selected as default. @brozorec can you confirm?

NikolaiYurchenko commented 1 year ago

@NikolaiYurchenko now the inputted value is selected by default after reloading the screen. The custom value should be stored across sessions, but shouldn't be selected as default. @brozorec can you confirm?

Well in this case no reason to save it when we always select 0.3 as default? I think we should definitely prefill slippage once some selected