LlamaSwap / interface

Frontend for LlamaSwap
https://swap.defillama.com
85 stars 77 forks source link

UX: Number formatting, new layout for routes, smaller IP switch #44

Closed ai-slave closed 1 year ago

ai-slave commented 1 year ago
image
vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
llamaswap ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 21, 2023 at 11:16AM (UTC)
0xngmi commented 1 year ago

image smol bug when gas cost is unknown

0xngmi commented 1 year ago

unsure about adding price below each quote since it seems a bit redundant, whats the rationale behind that?

ai-slave commented 1 year ago

@0xngmi to compare what was the initial quote from the aggregator. But maybe a good place to put "~ $9,900 After fees" instead?

ai-slave commented 1 year ago

@0xngmi updated to this version

image

Price is being shown in #50, I'll maybe iterate a little later to incorporate highlighting high impact orders on the list of routes, so you can understand it's not good before you click on it.

Gas fees: Solved like this. Potentially we may want to penalty results w/o calculated gas since now they will be prioritised all the time.

image
ai-slave commented 1 year ago

removed .toFixed(3) on final amount that you're getting image

ai-slave commented 1 year ago

Anything else weird left on this one?

0xngmi commented 1 year ago

looks good, will review in a bit again and ask for more reviews

0xngmi commented 1 year ago

sorry but recently merged a PR that made a lot of changes, could you fix the merge conflicts?

ai-slave commented 1 year ago

@0xngmi done

mintdart commented 1 year ago

can you update this row color, it is a bit harder to read Screenshot 1

Please ignore this, I've changed it in my last commit

mintdart commented 1 year ago

on mobile, gas fees shrink, and aggregator name breaks line, maybe on mobile show fees, gas, and aggregator name vertically? Screenshot

ai-slave commented 1 year ago

done, also removed inner scroll on desktop since it's just hiding results w/o purpose

0xngmi commented 1 year ago

image z-index issue

ai-slave commented 1 year ago

@0xngmi can't reproduce, but topped zindex, please check

0xngmi commented 1 year ago

still seeing same issue, on prod at https://llamaswap-git-fork-ai-slave-master-llamapay.vercel.app/ (tested with brave and firefox)

ai-slave commented 1 year ago

@0xngmi should be fixed

mintdart commented 1 year ago

I still see z-index issue on https://llamaswap-git-fork-ai-slave-master-llamapay.vercel.app/?from=0x0000000000000000000000000000000000000000&to=0x4d224452801aced8b2f0aebe155379bb5d594381 Screenshot

ai-slave commented 1 year ago

Fixed, thank you guys for testing it. I'll try to do better :|

mintdart commented 1 year ago

Fixed, thank you guys for testing it. I'll try to do better :|

If z-index is -1, you can't select any route

ai-slave commented 1 year ago

fixed, layers should be fine now

mintdart commented 1 year ago

on mobile :| Screenshot

ai-slave commented 1 year ago

fixed 👼 thank you

0xngmi commented 1 year ago

your fix might not have been pushed because of mintdart's commit, after which you need to rebase

saying this because you mentioned it was fixed but there were no new commits

ai-slave commented 1 year ago

yeah sorry, just forgot to push :/

0xngmi commented 1 year ago

new ui bussin frfr