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.

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

feat(app): make recipient editable with reset #2867

Closed
wants to merge 5 commits into from
Closed

feat(app): make recipient editable with reset #2867

wants to merge 5 commits into from

Conversation

Swepool
Copy link
Contributor

@Swepool Swepool commented Aug 29, 2024 •
edited
Loading

closes #2770

@Swepool Swepool requested a review from cor as a code owner August 29, 2024 14:17
@netlify Netlify
Copy link

netlify bot commented Aug 29, 2024 •
edited
Loading

✅ Deploy Preview for unionlabs ready!

Name Link
🔨 Latest commit 966854b
🔍 Latest deploy log https://app.netlify.com/sites/unionlabs/deploys/66d5882d851cf5000867700d
😎 Deploy Preview https://deploy-preview-2867--unionlabs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 1 paths audited
Performance: 50 (🟢 up 31 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (🔴 down 8 from production)
SEO: 93 (no change from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 29, 2024 •
edited
Loading

App 🤌

🌎 Deploying...
✨ Deployment complete! Take a peek over at https://ab9f222c.app-1b1.pages.dev

@Swepool Swepool self-assigned this Aug 30, 2024
@Swepool Swepool added the A-app Area: app.union.build label Aug 30, 2024
cor
cor requested changes Sep 1, 2024
View reviewed changes
Copy link
Contributor

@cor cor left a comment

Choose a reason for hiding this comment

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

Some comments, in addition: selecting a destination chain should no longer require a wallet to be connected, as you can fill in any address for the destination chain. Only require wallets to be connected for the source chain. Because of this, it's important that we do full verification of validity of the destination address

app/src/routes/transfer/(components)/transfer-form.svelte Outdated Show resolved Hide resolved
app/src/routes/transfer/(components)/transfer-form.svelte Outdated Show resolved Hide resolved
app/src/routes/transfer/(components)/transfer-form.svelte Outdated Show resolved Hide resolved
app/src/routes/transfer/(components)/transfer-form.svelte Show resolved Hide resolved
cor
cor requested changes Sep 2, 2024
View reviewed changes
Copy link
Contributor

@cor cor left a comment

Choose a reason for hiding this comment

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

Doesn't work, transferring from stride -> arbitrum with a manual receiver put in yields the following incorrect transfer:

https://app.union.build/explorer/transfers/015096DB79FF0D2D019306E2CF0856A622C346765E250E89B42BB1C227080DF0

whereas it does work when transferring from the current staging UI:

https://staging.app.union.build/explorer/transfers/F61C997FD42FAC9B68A9F54BC2FC5041286B77E49029F5781AE429526292CDE1

Perhaps there is an issue with the way in which you normalize the custom receiver address?

@Swepool Swepool closed this Oct 1, 2024
@Swepool Swepool deleted the address-edit branch October 15, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cor cor cor requested changes

Assignees

@Swepool Swepool

Labels
A-app Area: app.union.build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make receiver editable on transfer page
2 participants
@Swepool @cor