search

fudgebucket27 / Lexplorer

Loopring explorer alternative
14 stars 10 forks source link

Add token overview and details pages #205 #218

Closed modersohn closed 2 years ago

modersohn commented 2 years ago

Will close #205

modersohn commented 2 years ago

@danielsolistensvik just in case you want to give your "whale-query" a go, here's my first take on token overview and details page. If you want to, feel free to take it from here.

Since I couldn't add you as a reviewer (probably only after you interacted here?), I'll added you as a collaborator to my fork - the end result being hopefully the same, i.e. you should be able to push to my fork so the code automatically ends up in the PR here.

daniel-soli commented 2 years ago

I'll give it a go :)

modersohn commented 2 years ago

OK great - I am away for today, so won't be able to look at anything before later tomorrow.

daniel-soli commented 2 years ago

Added a simple table with pager which sorts balances by desc. Thought about only showing the top 25, but don't know if thats interesting only knowing those. So better to sort all by balance I think. Have a look :)

daniel-soli commented 2 years ago

I have added some more:

If this is not wanted or necessary at all, just remove it. I think it's a nice feature to see the ens. I tried with also getting L1 ens (abcde.eth), but with both resolvers it was kinda slow. This is all a "nice to have" so feel free to remove. Only thing I forgot in the previous commits was appcache, so if you decide to revert to only whales list without ens, just add cache :)

Edit: May have missed some more in the go, but as allways just feel free to add/edit/delete :)

fudgebucket27 commented 2 years ago

Oh this is pretty cool so far! EDIT: Try to keep the PRs small and relevant to each other again Daniel please, there's way to much going on right now

daniel-soli commented 2 years ago

I agree with you on the subject of keeping things relevant to eachother in the PR. I think in this specific case it's all relevant and not that much of a big PR. But with that said, it's easy to split this up into two seperate PR's. The original content of @modersohn and then my commits. :) That way it would be in smaller fractions. What do you think?

fudgebucket27 commented 2 years ago

Yes that would be better thank you

On Tue, 5 July 2022, 4:06 pm Daniel Soli Stensvik, @.***> wrote:

I agree with you on the subject of keeping things relevant to eachother in the PR. I think in this specific case it's all relevant and not that much of a big PR. But with that said, it's easy to split this up into two seperate PR's. The original content of @modersohn https://github.com/modersohn and then my commits. :) That way it would be in smaller fractions. What do you think?

— Reply to this email directly, view it on GitHub https://github.com/fudgebucket27/Lexplorer/pull/218#issuecomment-1174652834, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIDWT6NM6NLZL2DQGYZ653VSPGFNANCNFSM52TQEWDQ . You are receiving this because you commented.Message ID: @.***>

modersohn commented 2 years ago

See, I went out of my way to lay all the groundwork so you could do the whales query you seemingly were keen to do.

And the one thing I asked you not to do (in #205), is exactly what you did.

This PR is about the token overview and details pages. Reverse lookup really has nothing to do with it.

daniel-soli commented 2 years ago

It's now back to the "original" with only the whales list. As I clearly stated it was just a suggestion which could be reverted, and it could also be put in a seperate PR as @fudgebucket27 suggested.

fudgebucket27 commented 2 years ago

Looks fine to me. Thanks for adding the new test cases too. I'll merge into dev