search

Fujicracy / fuji-v2

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

Position yields modal #406

Closed NikolaiYurchenko closed 1 year ago

github-actions[bot] commented 1 year ago

Please test this PR on: https://v2-staging-406-dot-fuji-306908.ey.r.appspot.com

ferostabio commented 1 year ago

A couple of questions @brozorec

@NikolaiYurchenko can you have a look at the position screen in a mobile device? Now that we added the "view" button, next to the "borrow more", doesn't look too good. If you have a look at the design, the button should be in a third row.

NikolaiYurchenko commented 1 year ago

A couple of questions @brozorec

  • The elements in the list appear clickable, but nothing happens. Should we remove the pointer or take the user to the position screen?
  • At the top we have estEarnings >= 0 ? 'Earnings' : 'Costs', but as a list header we have just earnings, what should we do there?

@NikolaiYurchenko can you have a look at the position screen in a mobile device? Now that we added the "view" button, next to the "borrow more", doesn't look too good. If you have a look at the design, the button should be in a third row.

@ferostar thanks for the review, I'll fix the adaptations. Regarding the header title, maybe smth like Est. USD Value Change (todo: think about it)

ferostabio commented 1 year ago

@ferostar thanks for the review, I'll fix the adaptations. Regarding the header title, maybe smth like Est. USD Value Change (todo: think about it)

Yes, I wasn't sure about it. I considered a tooltip, since we don't have any in that modal, since it's a table header and it shouldn't be long, but just not sure 🤔.

brozorec commented 1 year ago

The elements in the list appear clickable, but nothing happens. Should we remove the pointer or take the user to the position screen?

I'd suggest we remove the pointer and the rows aren't clickable.

At the top we have estEarnings >= 0 ? 'Earnings' : 'Costs', but as a list header we have just earnings, what should we do there?

What about "Est. Yield/Cost"?

Other points:

  1. Change the title of the modal to "Estimated Yield / Cost"
  2. Change the button to "Deposit and Borrow"
  3. The "Net APY" column should be calculated by depositAPY - borrowAPY
  4. Can we increase the width of the modal because I have a horizontal scroll that can be avoided?