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: bring trade success screen to new trade flow #8441

Merged
merged 2 commits into from
Dec 30, 2024

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Dec 24, 2024

Description

Does what it says on the box - bring back the scree to the new trade flow.

Issue (if applicable)

closes #8027

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

None - only change in prod code paths is removal of negative x margin for trade summary content.

Testing

Engineering

  • Trade summary looks good with the new trade flow flag on
  • Trade summary looks good with the new trade flow flag off

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)
  • Trade summary looks good

Screenshots (if applicable)

  • Flag on
Screenshot 2024-12-24 at 12 30 26
  • Flag off
Screenshot 2024-12-24 at 12 32 58

@gomesalexandre gomesalexandre requested a review from a team as a code owner December 24, 2024 09:34
@0xApotheosis
Copy link
Member

This is awesome @gomesalexandre, thank you ser! 🙏

We probably want to update the trade summary to show the updated "ExpandedStepperSteps" content to match the style of the new trade flow (when expanded).

Copy link
Collaborator

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

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

https://jam.dev/c/1b474d0c-4250-49ca-abbd-fbf687646d96

Amazing!

One feedback not blocking this PR but could be done as a follow up:
image

This part looks very weird with the new trade flow, We might be able to kick the Trade Summary wording and keep the Trade Complete/error accordion by itself

@shapeshift/product wdyt?

@NeOMakinG NeOMakinG enabled auto-merge (squash) December 30, 2024 10:11
@NeOMakinG NeOMakinG merged commit 40fa274 into develop Dec 30, 2024
3 checks passed
@NeOMakinG NeOMakinG deleted the feat_trade_summary_new_trade_flow branch December 30, 2024 10:13
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.

Update trade complete screen
3 participants