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

refactor: split up refund page #785

Merged
merged 11 commits into from
Jan 9, 2025
Merged

refactor: split up refund page #785

merged 11 commits into from
Jan 9, 2025

Conversation

jackstar12
Copy link
Member

@jackstar12 jackstar12 commented Jan 3, 2025

closes: #747

Also removes the link to the existing swap upon refund file upload if its found within browser history - its simply handled the same as all other refund files.

Copy link

vercel bot commented Jan 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
webapp ✅ Ready (Inspect) Visit Preview Jan 9, 2025 4:26pm
webapp-mainnet ✅ Ready (Inspect) Visit Preview Jan 9, 2025 4:26pm

Copy link
Member

@kilrau kilrau left a comment

Choose a reason for hiding this comment

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

Minor wording discrepancies with Figma in many places:

  • Landing title should be "Refund Swap" not "Refund a swap", please check all figma screens. Add fullstops etc.

Also:

  • Bitcoin/Liquid should have address field and refund button on empty state already and not have it appear automagically after selecting refund file (as we have it rn)
  • I forgot to include this error state in the wireframe, but here the sentence should say "Please switch your wallet network to Rootstock"
    image

Other than that awesome improvement, I really like this:

image

@jackstar12 jackstar12 requested a review from kilrau January 8, 2025 11:02
@jackstar12
Copy link
Member Author

@kilrau addressed all except the last one - will be in a follow up pr.

Copy link
Member

@kilrau kilrau left a comment

Choose a reason for hiding this comment

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

  1. Let me enter an address in this state, disable address validation and enable it as soon as a valid refund file was parsed
    image
  2. The address field shouldn't disappear in this state
    image

EDIT: this error state should be there too (just added it to figma)
image

@kilrau
Copy link
Member

kilrau commented Jan 8, 2025

and let's fix this CI 💪

@jackstar12
Copy link
Member Author

Let me enter an address in this state!!!

What about address validation in that case?

@kilrau
Copy link
Member

kilrau commented Jan 9, 2025

Let me enter an address in this state!!!

What about address validation in that case?

disable it, enable it once a valid refund file was parsed

@kilrau
Copy link
Member

kilrau commented Jan 9, 2025

Created follow up issue for my review: #793 So here only CI is missing.

Copy link
Member

@michael1011 michael1011 left a comment

Choose a reason for hiding this comment

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

tACK

<Match
when={
lockupTransaction.state === "ready" ||
lockupTransaction.state == "unresolved"
Copy link
Member

Choose a reason for hiding this comment

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

What does unresolved mean in this context?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its the initial state before it has been fetched for the first time.

Since we know pass the swap to the resource as a source signal it only fetches once populated (and refetches when it changes)

@michael1011 michael1011 merged commit 88b969c into main Jan 9, 2025
6 checks passed
@michael1011 michael1011 deleted the refactor/refund-page branch January 9, 2025 21:04
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.

Refund page: Split up into two sections (UTXO/EVM)
3 participants