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(permit): modals LIMIT #3164

Merged
merged 14 commits into from
Oct 5, 2023
Merged

feat(permit): modals LIMIT #3164

merged 14 commits into from
Oct 5, 2023

Conversation

alfetopito
Copy link
Collaborator

Summary

Fixes #3145

Follow up to #3158

Screen.Recording.2023-09-28.at.17.48.36.mov

Permit for LIMIT turned on for testing.
Partial fills might not work.

To Test

  1. Pick a permittable token to sell, make sure there's no allowance
  2. On LIMIT, using non-expert mode, confirm the order
  • Should show the new modal in the step 1 while the permit signature is requested in the wallet
  1. Sign the approval
  • Should show the new modal in the step 2 while the order signature is requested in the wallet
  1. Sign the order
  • Should show the regular modal
  1. Place an order without permit
  • Should show the old modal
  1. Use a non-permit token that needs approval
  • Approval button should be visible
  1. Start the approval
  • Should show the old modal

@vercel
Copy link

vercel bot commented Sep 28, 2023 •
edited by github-actions bot
Loading

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

Name Status Preview Updated (UTC)
swap-dev ✅ Ready (Inspect) Visit Preview Oct 5, 2023 9:28am

🌃 Cosmos ↗︎

@alfetopito alfetopito self-assigned this Sep 28, 2023
@alfetopito alfetopito requested a review from a team September 28, 2023 16:51
@shoom3301
Copy link
Collaborator

image

The modal got "ears" around :)

@shoom3301
Copy link
Collaborator

Should we change the text?
"Confirm Swap" => "Confirm Limit Order"

@alfetopito alfetopito mentioned this pull request Oct 3, 2023
Merged
@alfetopito alfetopito mentioned this pull request Oct 3, 2023
Merged
@alfetopito
Copy link
Collaborator Author

Should we change the text? "Confirm Swap" => "Confirm Limit Order"

Both fixed!

@alfetopito
Copy link
Collaborator Author

FYI, backend changes have been merged cowprotocol/services#1892
Partial fills should be supported now (I have not tested it yet)

@alfetopito
Copy link
Collaborator Author

Partial fills are working \o/ https://dev.explorer.cow.fi/goerli/orders/0x577dd963b0759ac76eba045684ffc17669e15eb746ffb28c6eb5730d0b4e09c45b0abe214ab7875562adee331deff0fe1912fe426525532c?tab=overview

@shoom3301
Copy link
Collaborator

image

Something wrong with X alignment

@alfetopito alfetopito mentioned this pull request Oct 4, 2023
Merged
@fairlighteth
Copy link
Contributor

fairlighteth commented Oct 4, 2023 •
edited
Loading

@shoom3301 It's not what I see:

Screen.Recording.2023-10-04.at.17.25.25.mov
Screenshot 2023-10-04 at 17 25 56

Base automatically changed from permit/3145_modals to develop October 5, 2023 09:18
@alfetopito alfetopito merged commit 70e5653 into develop Oct 5, 2023
7 checks passed
@alfetopito alfetopito deleted the permit/3145_modals-limit branch October 5, 2023 09:36
@github-actions github-actions bot locked and limited conversation to collaborators Oct 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@anxolin anxolin anxolin approved these changes

@fairlighteth fairlighteth fairlighteth approved these changes

Assignees

@alfetopito alfetopito

Labels
permit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Permit: Add modals to signing step
4 participants
@alfetopito @shoom3301 @fairlighteth @anxolin