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

Show the contract receiving token allowances on the allowance screen #3692

Merged
merged 26 commits into from
Mar 14, 2022

Conversation

blackdevelopa
Copy link
Contributor

@blackdevelopa blackdevelopa commented Feb 4, 2022

Description

Contract address naming

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Resolves #2723

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2022

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@blackdevelopa blackdevelopa added the DO-NOT-MERGE Pull requests that should not be merged label Feb 4, 2022
@blackdevelopa blackdevelopa self-assigned this Feb 4, 2022
@blackdevelopa blackdevelopa added Code Impact - Medium Average task code change that can relatively safely being applied to the codebase type-enhancement New feature or request and removed DO-NOT-MERGE Pull requests that should not be merged labels Feb 11, 2022
@blackdevelopa blackdevelopa marked this pull request as ready for review February 11, 2022 14:22
@blackdevelopa blackdevelopa requested a review from a team as a code owner February 11, 2022 14:22
@blackdevelopa blackdevelopa added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Feb 13, 2022
Copy link
Contributor

@sethkfman sethkfman left a comment

Choose a reason for hiding this comment

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

Minor updates required to use locales

@sethkfman sethkfman added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Feb 15, 2022
@cortisiko cortisiko added QA in Progress QA has started on the feature. and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Feb 15, 2022
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

Issue 1
I get this warning: ERROR Warning: Failed prop type: Invalid prop protectWalletModalVisible of type function supplied to ApproveTransactionReview, expected boolean.

When I attempted to approve a token

to reproduce:
go here while on a test net (because you do not want to use real funds)
connect your wallet
scroll down on the page and tap on create token
after your txn was confirmed tap on “approve token”
notice the confirmation modal appears with the error

Issue 2
after saving the contract address to my contacts I get: Cannot update during an existing state transition (such as within render). Render methods should be a pure function of props and state

see here

to reproduce:
go here while on a test net (because you do not want to use real funds)
connect your wallet
scroll down on the page and tap on create token
after your txn was confirmed tap on “approve token”
notice the confirmation modal appears. Now tap on “Add nickname”
input a nick name then hit save

notice the error appears

Issue 3
On the edit nick name view I am unable to share my address. Tapping on the share icon does nothing. On the other hand tapping on the address does not copy it to my clipboard. I need to carefully tap on the copy icon in order to copy. Even when i do copy my address i see a toast message “shell was pasted from your clipboard”. Maybe it should be “address was copied to your clipboard” ?

see here

Issue 4
the contract i added to my contacts should not appear in recents

to reproduce:
go here while on a test net (because you do not want to use real funds)
connect your wallet
scroll down on the page and tap on create token
after your txn was confirmed tap on “approve token”
notice the confirmation modal appears. Now tap on “Add nickname”
input a nick name then hit save
return to the wallet view then go through the send flow
notice the contract address appears

@cortisiko cortisiko added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed QA in Progress QA has started on the feature. labels Feb 15, 2022
@blackdevelopa blackdevelopa added design-review Any feature that needs feedback from the design team needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) and removed design-review Any feature that needs feedback from the design team labels Feb 20, 2022
@blackdevelopa
Copy link
Contributor Author

blackdevelopa commented Feb 23, 2022

Issue 1 I get this warning: ERROR Warning: Failed prop type: Invalid prop protectWalletModalVisible of type function supplied to ApproveTransactionReview, expected boolean.

When I attempted to approve a token

to reproduce: go here while on a test net (because you do not want to use real funds) connect your wallet scroll down on the page and tap on create token after your txn was confirmed tap on “approve token” notice the confirmation modal appears with the error

Thank you for catching this, QA issue 1 is now fixed.

Issue 2 after saving the contract address to my contacts I get: Cannot update during an existing state transition (such as within render). Render methods should be a pure function of props and state

see here

to reproduce: go here while on a test net (because you do not want to use real funds) connect your wallet scroll down on the page and tap on create token after your txn was confirmed tap on “approve token” notice the confirmation modal appears. Now tap on “Add nickname” input a nick name then hit save

notice the error appears

Thank you for catching this, QA issue 2 is now fixed.

Issue 3 On the edit nick name view I am unable to share my address. Tapping on the share icon does nothing. On the other hand tapping on the address does not copy it to my clipboard. I need to carefully tap on the copy icon in order to copy. Even when i do copy my address i see a toast message “shell was pasted from your clipboard”. Maybe it should be “address was copied to your clipboard” ?

see here

Thank you for catching this, QA issue 3 is now fixed. We've got approval from Design on the share icon

Issue 4 the contract i added to my contacts should not appear in recents to reproduce: go here while on a test net (because you do not want to use real funds) connect your wallet scroll down on the page and tap on create token after your txn was confirmed tap on “approve token” notice the confirmation modal appears. Now tap on “Add nickname” input a nick name then hit save return to the wallet view then go through the send flow notice the contract address appears

Thank you for catching this, QA issue 4 is now fixed

@blackdevelopa blackdevelopa added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Mar 1, 2022
@mobularay mobularay removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 10, 2022
@cortisiko cortisiko added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels Mar 10, 2022
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

🌮 🌮 🌮

@blackdevelopa blackdevelopa merged commit d02ff68 into main Mar 14, 2022
@blackdevelopa blackdevelopa deleted the feature/2723-approve-hostname branch March 14, 2022 15:41
@github-actions github-actions bot locked and limited conversation to collaborators Mar 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Code Impact - Medium Average task code change that can relatively safely being applied to the codebase QA Passed A successful QA run through has been done type-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show the contract receiving token allowances on the allowance screen
5 participants