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

#605 | Revoke / Edit Allowances UI #711

Merged
merged 19 commits into from
Dec 15, 2023

Conversation

ezra-sg
Copy link
Contributor

@ezra-sg ezra-sg commented Dec 7, 2023

Fixes #605

Description

This PR adds the UI (non-functional) for editing and revoking allowances

Note: we cannot currently test single ERC721 tokens as the indexer returns slightly malformed data for them; we will need to test those in a later PR, which I will include in the testing notes

Test scenarios

  • run the code locally on mainnet: git fetch --all && git checkout 605-revoke-allowances-ui && echo 'NETWORK="mainnet"' > .env && yarn dev
  • go to http://localhost:8081
  • log in with the team account (0x...eE37)
  • go to the approvals page
    • you should see checkboxes in the first column of the table
    • you should see edit icons next to the allowance values in the table
  • try various combinations of toggling individual rows and using the master toggle at the top
    • the checkboxes should behave as your intuition would expect
  • go to another page of allowance results
    • the checkboxes, including the master one at the top, should reset
  • click the edit allowance icon on an erc20 token row with an allowance which is neither unlimited nor 0
    • the allowance modal should show the right token information
    • the "Custom" radio should be selected
    • the allowance amount should be pre-populated in the currency input
    • the Confirm button should be disabled
  • open dev tools
  • change the allowance amount either by clicking a different radio button or typing a custom value
    • the Confirm button should become enabled
  • click the confirm button
    • you should see the new allowance amount printed in the console
  • the Cancel button should close the modal
  • click the edit allowance icon on an erc20 token row which has an Unlimited allowance
    • the Unlimited radio should be selected when the edit modal opens up
  • repeat the steps from the Custom allowance notes above
    • the modal should work the same
  • click the edit allowance button on a row which has an NFT
    • the text in the modal should change slightly to say "Collection"
    • the "allowed" radio button should be selected
  • click "not allowed"
    • false should print in the console

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have cleaned up the code in the areas my change touches
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings
  • I have removed any unnecessary console messages
  • I have included all english text to the translation file and/or created a new issue with the required translations for the currently supported languages
  • I have tested for mobile functionality and responsiveness
  • I have added appropriate test coverage

@ezra-sg ezra-sg changed the base branch from develop to epic-allowances December 7, 2023 00:32
@telosnetwork telosnetwork deleted a comment from netlify bot Dec 7, 2023
@telosnetwork telosnetwork deleted a comment from netlify bot Dec 7, 2023
@ezra-sg ezra-sg marked this pull request as ready for review December 7, 2023 22:02
@ezra-sg ezra-sg self-assigned this Dec 7, 2023
@ezra-sg ezra-sg added the 🚫 Blocked Can't proceed with this issue or pull request label Dec 7, 2023
@donnyquixotic donnyquixotic removed the 🚫 Blocked Can't proceed with this issue or pull request label Dec 8, 2023
@donnyquixotic
Copy link
Contributor

donnyquixotic commented Dec 14, 2023

@ezra-sg I don't think the unlimited tool tip is necessary. It's always going to be the entire balance, users don't care about the technical detail for that. If we want to update the language and keep it, let's move it to the info icon only, it's too distracting and easy to trigger when selecting radio buttons. For reference, revoke.cash only mentions in a general info blurb separate from the revoke UI that 'unlimited' refers only to specific tokens and not the wallet as whole.

Copy link
Collaborator

@Viterbo Viterbo left a comment

Choose a reason for hiding this comment

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

nailed it! 💯

@donnyquixotic donnyquixotic merged commit a79c5c9 into epic-allowances Dec 15, 2023
1 check passed
@donnyquixotic donnyquixotic deleted the 605-revoke-allowances-ui branch December 15, 2023 22:49
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.

3 participants