0xStation / proposer

1 stars 0 forks source link

rewriting the stepper #721

Closed mcgingras closed 2 years ago

mcgingras commented 2 years ago

What's included in this pr + description

Note: sorry this PR is so spammy looking, I wanted to document each edge case I could think of so I could hold myself accountable to making sure everything was rigorously tested.

This PR includes a re-write of the stepper component since the original implementation was poorly designed to scale. Along with the re-write, it includes new steps for various types of payments, which handle both the single signer and gnosis safe cases.

Motivation & Context

The technical motivation for this PR is making the stepper easier to reason about and extend before it grows too large and out of control. The product motivation is making it easier for a user to see all of the steps they will have to take so they know where they are along the process. It also provides a convenient CTA.

Steps to test

tbd

Cases to test for

user has no roles

Screenshot 2022-10-28 at 1 58 19 PM

any condition / any signer type / saved as draft

Screenshot 2022-10-28 at 2 03 46 PM

any condition / gnosis safe (many signers) this is probably default but not always Pay upon approval / single signer

Screenshot 2022-10-28 at 3 34 12 PM Screenshot 2022-10-28 at 3 35 13 PM Screenshot 2022-10-28 at 3 38 35 PM

Pay upon approval / gnosis safe TODO: after approving gnosis tx, do a refresh so it updates the stepper. It's working, but the button persists bc the data is not refreshing after signature.

Screenshot 2022-10-28 at 3 49 33 PM Screenshot 2022-10-28 at 3 49 54 PM Screenshot 2022-10-28 at 3 50 27 PM Screenshot 2022-10-28 at 3 51 02 PM Screenshot 2022-10-28 at 3 51 53 PM Screenshot 2022-10-28 at 3 52 23 PM

Advanced payment / single signer

Screenshot 2022-10-28 at 3 45 00 PM Screenshot 2022-10-28 at 3 46 15 PM Screenshot 2022-10-28 at 3 46 45 PM Screenshot 2022-10-28 at 3 47 08 PM

Advanced payment / gnosis safe

Screenshot 2022-10-29 at 12 06 49 PM Screenshot 2022-10-29 at 12 07 19 PM Screenshot 2022-10-29 at 12 08 15 PM Screenshot 2022-10-29 at 12 10 07 PM Screenshot 2022-10-29 at 12 52 35 PM Screenshot 2022-10-29 at 12 53 09 PM

Pay upon completion / single signer note, we have not worked on deliverables yet, so this is basically the same as pay upon approval, aside from slight copy differences

Screenshot 2022-10-28 at 3 40 25 PM Screenshot 2022-10-28 at 3 41 50 PM Screenshot 2022-10-28 at 3 43 31 PM

Pay upon completion / gnosis safe note, we have not worked on deliverables yet, so this is basically the same as pay upon approval, aside from slight copy differences, also, I think I may have skipped some screenshots :0 but its working okay

Screenshot 2022-10-29 at 11 49 13 AM Screenshot 2022-10-29 at 11 49 47 AM Screenshot 2022-10-29 at 12 02 55 PM

Todo

theres a lot of duplication between the payment step and the proposalMilestonePaymentBox component. I'm thinking we could abstract the logic for determining actions into a hook, and use that hook to determine the actions on either page.

vercel[bot] commented 2 years ago

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

Name Status Preview Updated
web ✅ Ready (Inspect) Visit Preview Oct 31, 2022 at 9:26PM (UTC)
kristencheung commented 2 years ago

@frog - to fix your vercel issue, could you try

git rm -r --cached .
git add --all .
git commit -am "Fix casing discrepancies."
git push origin branch_name

I had a file casing issue and ran into this solution online^

kristencheung commented 2 years ago

Thanks for all of the screenshots btw in the pr description! This is helpful to test. Right now I have a safe with 2/3 quorum, but I'm not seeing the "queue" gnosis transaction payment post-approval on the stepper and I'm connected to a wallet that's on the safe

image
kristencheung commented 2 years ago

Somewhat unrelated to this pr, but it was kinda confusing me how sometimes we refer to signing a transaction as "approve" and sometimes as "sign" on Gnosis, I think we should just stick to sign as not to be confused with proposal approvals and I think Gnosis refers to it as signing as well. Initially, I thought there was a bug after I had signed where it then said "you have already approved". Also, I think we should also change the copy to "you have signed this transaction" because "already" implies that I did it a while ago.

image

https://www.loom.com/share/f2cc65796130410e87f14ff671f7a65f

kristencheung commented 2 years ago

Very nice work @mcgingras ! this stepper stuff is quite complex, so it's impressive that you were able to fit all this complexity into this small component 😅. I left a couple of comments + confusions that I can help dig into

Also, we're thinking of getting to the blitz migration by eod so maybe we can fix this comment and then follow up with pr comments afterwards, unless you think they're straightforward to fix!