Skip to content
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.

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

feat(trading): distinguish between eth and arb assets #6521

Merged
merged 31 commits into from
Jun 12, 2024

Conversation

mattrussell36
Copy link
Collaborator

Related issues 🔗

Closes #6464

Description ℹ️

  • Removes dupcliated asset field fragments so that chainId is always present
  • Adds ChainIdMapShort for 'Arb' and 'Eth'
  • Uses the short chain names in various places such as the collateral table
  • Updates the pills that swap between notional/quote values with short chain names
  • Shows short chain name in market headers

Demo 📺

Screenshot 2024-06-10 at 15 31 04

Technical 👨‍🔧

Details of technical implementation that reviewers may need to be aware of, if applicable.

@mattrussell36 mattrussell36 requested review from a team as code owners June 10, 2024 14:31
@daro-maj daro-maj self-assigned this Jun 10, 2024
@JonRay15
Copy link
Contributor

Made various new comments on the origial ticket ... can we address these before merging please.

@JonRay15
Copy link
Contributor

JonRay15 commented Jun 11, 2024

We need it on deposit and withdrawal tabs .... its only on collateral right now.... but I think it probably is best to cover this on the PR for new arb deposits?

@bwallacee
Copy link
Contributor

Should the asset details Source chain say Eth Sepolia (like it does for Arb Sepolia):
image

Should we have a space after tUSDC?:
image
Missing chain on deposits and withdraws tables in portfolio:
image

Should we have something to distinguish the chain the markets in order history are? (same for the other positions tabs)
image

@daro-maj
Copy link
Contributor

Do we need to specify chain also here ?
image

1 similar comment
@daro-maj
Copy link
Contributor

Do we need to specify chain also here ?
image

@daro-maj
Copy link
Contributor

Is the governance is in the scope ?
image

@mattrussell36 mattrussell36 merged commit 0999a49 into develop Jun 12, 2024
14 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure clarity around USDT Ethereum vs Arbitrum
4 participants