[Claim] Revoke Approval hook #2249
Conversation
W3stside
changed the base branch from
develop
to
claim-approve-optional-param
W3stside
force-pushed
the
claim-revoke-approve-hook
branch
from
b5553b6 to
0faa470
Compare
|
|
@W3stside , now Approved! is displayed even for an account that has not approved this token before It is displayed right after I change an account after claim. it might be displayed up to 30 sec (or so) Can we do something with this? |
|
Another question: when I press on the Revoke button, I see 'Approving..' modal. The same with the sent request to the connected wallet. |
I think this pr is more for testing the base pr of approvals, if we decide we want this we can fix that for sure |
|
Agree, if we implement it, modals should reflect that. I understand this is for testing purposes with the option to actually expose it at all times? In the investment flow, the only scenario I can see someone In general I like giving this control to the user. But feel a more central place (approvals management?) better serves this functionality. That being said, doesn't hurt to keep it the way you showed it. |
There was a problem hiding this comment.
We can merge, use it for testing, but after i think is ok to delete the button (the hook could be left around in case we need it, or users ask for this).
Not strongly opposed to it, but i wouldn't do it.
Reason are same as @biocom.
Since this is there, if users ask for it, we can add it back.
APPROVING with comments
| @@ -233,6 +292,11 @@ export default function InvestOption({ claim, optionIndex, openModal, closeModal | |||
| )} | |||
| </ButtonConfirmed> | |||
| )} | |||
| {isAlreadyApproved && ( | |||
| <UnderlineButton disabled={approving || !isAlreadyApproved} onClick={handleRevokeApproval}> | |||
| Revoke approval | |||
There was a problem hiding this comment.
Should we add a (Test only) if we want to merge this?
There was a problem hiding this comment.
I'll remove this if we merge
W3stside
force-pushed
the
claim-revoke-approve-hook
branch
from
6ab360c to
d4c96e9
Compare
W3stside
force-pushed
the
claim-approve-optional-param
branch
from
5bfd28e to
433afe4
Compare
W3stside
force-pushed
the
claim-revoke-approve-hook
branch
from
d4c96e9 to
37644ce
Compare
|
|
Works fine to me now! |
W3stside
force-pushed
the
claim-approve-optional-param
branch
from
433afe4 to
8956e62
Compare
W3stside
force-pushed
the
claim-revoke-approve-hook
branch
from
37644ce to
eece0ba
Compare
W3stside
force-pushed
the
claim-revoke-approve-hook
branch
from
eece0ba to
63a9031
Compare
Summary
This exists purely as a positive side-effect of having to create this function when testing approve logic in #2175 - doesn't have to stay but think it's useful or at least almost at a useful state.
High-level description of what your changes are accomplishing
Allows "revoking" approval of a token for a spender. Sets approve to "0" - maybe there's a better way or even a native unapprove method but this is what I have for now.
To Test