Fujicracy / fuji-v2

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

Markets: populate data #181

Closed Chris-CHEVALIER closed 1 year ago

Chris-CHEVALIER commented 1 year ago

Close #155

github-actions[bot] commented 1 year ago

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

brozorec commented 1 year ago

Can't test because of CORS errors. @brozorec this may be in SDK scope, can you add the header ?

Capture d’écran, le 2022-12-13 à 13 51 28

F*ck, I didn't check this when implementing. This header should come from the server. I think with Next we can make API calls on the server before delivering to the client. Can we do something like this on our app? https://github.com/vercel/next.js/tree/canary/examples/api-routes-cors

doliG commented 1 year ago

@brozorec problem solved. I loved how next make that easy, its just a little bit of config 🤓

doliG commented 1 year ago

Ok so I think this is ready for review, just need help of either @DaigaroCota or @brozorec about navigation after clicking on a Market row.

Markoyw commented 1 year ago

Hey @doliG,

I have gone through the implementation on staging.

Here's the link to the design feedback: https://www.figma.com/file/v53wiSwcL5DYynVL0z6L4s/Fuji---Working-File-(NEW)?node-id=3041%3A51856&t=MY9dru8TXFFHXShG-1

brozorec commented 1 year ago

Good job :+1:

  1. When one hovers over the providers' logos, can we display a tooltip with their names?

Wdyt @Markoyw ?

  1. I was a bit confused about having the order of assets' names and the assets' rates inversed.
brozorec commented 1 year ago

Ok so I think this is ready for review, just need help of either @DaigaroCota or @brozorec about navigation after clicking on a Market row.

It has to navigate to "Manage position", ref https://github.com/Fujicracy/fuji-v2/issues/41

Markoyw commented 1 year ago

Hey @brozorec @doliG

  1. That's a good idea. I am attaching an example of the tooltip design here https://www.figma.com/file/v53wiSwcL5DYynVL0z6L4s/Fuji---Working-File-(NEW)?node-id=3042%3A51874&t=bFPZHGdDkTO3x5i9-1

Also, I'm not sure if we are able to stack the tooltip as such:

Screenshot 2023-01-02 at 8 00 49 PM
  1. Ah, it was a comment on the sorting functionality. I was trying to test it - when I click on the header, the arrow changes up to down. However, the data is not sorted.
brozorec commented 1 year ago

Awesome, thank you @Markoyw :+1: Regarding "2." I meant something different: it was about the order of the columns. For the assets' names, we start with the borrowed one while for the rates the supply rate is first.

Markoyw commented 1 year ago

@brozorec

Good point. What do you think if we were to switch the assets column starting from - 'collateral' and then 'borrow'?

doliG commented 1 year ago

@Markoyw Design feedbacks will be fixed asap ⏳

  1. I don't think nested tooltip are a good idea in terms of UX (not mobile friendly) + it's difficult to handle technically, but it's doable. I suggest instead to display one big tooltip with the logo of all protocols and their name.

  2. Regarding the column order, I had the same remark as @brozorec and it's an easy change. @Markoyw if you're okay with it I'll do it.

  3. Regarding sorting, it's because sorting the data will cause everything to change (even chain with the best rate). @brozorec can we discuss it through a call ? I think it'll be easier.

brozorec commented 1 year ago

@brozorec

Good point. What do you think if we were to switch the assets column starting from - 'collateral' and then 'borrow'?

@Markoyw Yep, I think it's safe to do so :+1:

brozorec commented 1 year ago
  1. Initially, we won't have vaults with more than 4 providers, so I suggest we just put a simple tooltip with the names.
  2. Regarding the sorting, I'm creating a ticket for it so that we deal with it later on: #218

@doliG

doliG commented 1 year ago

@Markoyw one last thing, what should I display if we fail to fetch the markets ? (I'm asking because we use defillama API and sometimes it fails)

doliG commented 1 year ago
  1. Tooltip done
  2. Ok, I commented the sorting code
  3. [NEW] I created #219 that can be done once we have a design. Thanks :)

@brozorec @Markoyw this PR is done, I'm merging it. Thanks for your contributions guys 🔥

doliG commented 1 year ago
  1. Ok done
  2. .
  3. I commented the code that sort the table, we'll handle it later
  4. [NEW] #219 (waiting for design)

@Markoyw @brozorec I'm merginf this one, thanks you guys for your contributions.