Skip to content

Implement transaction route component #648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 55 commits into
base: dev
Choose a base branch
from
Draft

Implement transaction route component #648

wants to merge 55 commits into from

Conversation

kuba80-02
Copy link
Contributor

No description provided.

@kuba80-02 kuba80-02 marked this pull request as draft March 13, 2025 19:29
@kuba80-02
Merge branch 'dev' of https://github.com/invariant-labs/webapp-eclipse …
64525a7 
…into implement-transaction-route-component
wojciech-cichocki
wojciech-cichocki requested changes Mar 14, 2025
View reviewed changes
Copy link
Member

@wojciech-cichocki wojciech-cichocki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try experimenting more with the alignment of margins/amounts/card size. This example doesn't look good:
image

@Sniezka1927
Copy link
Contributor

It might be worth to check:

  1. Use same margins and paddings from every side so the transaction is centered horizontally and vertically

  2. Use same width for parent container no matter of route steps.

  3. Use common path style not depending on routes. For me this style looks the best
    image

  4. Try to merge the Dex card with token. IMO currently there are too much information displayed in separated boxes, each swap step (dex information + output token) should be within single container.

@kuba80-02
Merge branch 'dev' of https://github.com/invariant-labs/webapp-eclipse …
5f68695 
…into implement-transaction-route-component
wojciech-cichocki
wojciech-cichocki requested changes Apr 9, 2025
View reviewed changes
Copy link
Member

@wojciech-cichocki wojciech-cichocki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that these three values are still coming from the local simulation instead of aggregator API:
image

@kuba80-02
Merge branch 'dev' of https://github.com/invariant-labs/webapp-eclipse …
06df049 
…into implement-transaction-route-component
@github-actions GitHub Actions
Copy link

This pull request has been deployed to Vercel.

Latest commit: edfdc9e
✅ Preview: https://webapp-eclipse-qyqmd30ja-invariantlabs.vercel.app
🔍 Inspect: https://vercel.com/invariantlabs/webapp-eclipse/6TN7fgK9gCC8WbTDTJdz2Tpd7uDP

View Workflow Logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wojciech-cichocki wojciech-cichocki wojciech-cichocki requested changes

Assignees
No one assigned
Labels
deployed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kuba80-02 @Sniezka1927 @wojciech-cichocki