Skip to content
This repository has been archived by the owner on Apr 4, 2022. It is now read-only.

Implementation of the batch transanction view #1061

Merged
merged 36 commits into from
Apr 1, 2022
Merged

Conversation

henrypalacios
Copy link
Contributor

@henrypalacios henrypalacios commented Mar 4, 2022

Summary

Closes #1058

Implement the visualization of a batch of transactions through a node graph.

Cytoscape has been used as interaction library due to its maturity and large community. But we had to apply styles in a way outside the project format.

image

To Test:

@henrypalacios henrypalacios added app:Explorer Explorer App Protofire task to the protofire team labels Mar 4, 2022
@github-actions
Copy link

github-actions bot commented Mar 4, 2022

@alongoni
Copy link
Contributor

alongoni commented Mar 17, 2022

We are working with @henrypalacios adding some tooltips to the arrows in order to have the amounts traded by the users (Traders). Also we need to wrap the address of the AMMs.

Also, we are exploring another layout called: "cose" and it's look like this:
Default view:
image
Expected view:
image

It' ll require some testing to achieve the result.

@alfetopito
Copy link
Contributor

alfetopito commented Mar 17, 2022

Funny edge case https://pr1061--gpui.review.gnosisdev.com/tx/0x51757dd21436d695824ceb13d21ec3c3ef5e5e5454a847951b1ea91e1c89b0b7

Screen Shot 2022-03-17 at 15 30 21

EDIT: well, turns out it was my network... took a long time to load and couldn't find the token symbols. After a refresh I can see the token names loaded
Screen Shot 2022-03-17 at 15 33 01

@alfetopito
Copy link
Contributor

Well this is a giant mess https://pr1061--gpui.review.gnosisdev.com/tx/0x172bddae0015331f4b357905ff7995389597a38294a00b4b13e90c1d70884785/ 😅

For reviewers: open https://dune.xyz/queries/205786/384521, filter by barn and sort by num trades

@henrypalacios henrypalacios marked this pull request as ready for review March 18, 2022 13:19
@alongoni alongoni requested a review from ramirotw March 23, 2022 12:52
@elena-zh
Copy link

elena-zh commented Mar 30, 2022

@henrypalacios , I have verified the latest changes!
Network names, rounding, and increased token names LGTM!
Maybe another nitpick I could mention is that it would be better to fill in the whole screen with the diagram area
image

@anxolin
Copy link
Contributor

anxolin commented Mar 30, 2022

As as a good to have feature, for future reiterations, do you think we can make the arrows reflect if they are a sell or buy orders?

Normally GREEN reflects a buy, RED a sell. We could use color either in the arrow or the user or somewhere we could tell which ones are what.

@alongoni
Copy link
Contributor

@henrypalacios , I have verified the latest changes! Network names, rounding, and increased token names LGTM! Maybe another nitpick I could mention is that it would be better to fill in the whole screen with the diagram area image

Hey @elena-zh thanks! Is your zoom at 100%? Maybe that could be the cause.
https://www.loom.com/share/4c376579ba204d17a32fd3f02d4de014

@elena-zh
Copy link

Hey @alongoni , I have fund the root cause of the issue: the diagram area is not auto-scalable. I mean, if I drag and drop a browser's tab with the diagram from a smaller screen to a bigger one, I will get this issue. See the video: https://watch.screencastify.com/v/u2oEJODe2166zN7faBdz

But it is a low-low priority issue, that is why I created a separate task for this #1084

@elena-zh
Copy link

As as a good to have feature, for future reiterations, do you think we can make the arrows reflect if they are a sell or buy orders?

Normally GREEN reflects a buy, RED a sell. We could use color either in the arrow or the user or somewhere we could tell which ones are what.

I have created a separate enhancement task for this #1085

@henrypalacios
Copy link
Contributor Author

Hey @elena-zh, @ramirotw as I mentioned I submitted changes to mitigate issues #1074(pt. 1), #1084 in this first iteration.

So we will continue working on the corresponding issues on a next iteration.

@henrypalacios henrypalacios added the Auto-merge PRs with this tag will be automatically merged when approved and CI succeeds label Apr 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
app:Explorer Explorer App Auto-merge PRs with this tag will be automatically merged when approved and CI succeeds Protofire task to the protofire team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Batch Viewer to tx view
7 participants