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

Show token symbol in verify contract details #5956

Merged
merged 7 commits into from
Apr 18, 2023
Merged

Show token symbol in verify contract details #5956

merged 7 commits into from
Apr 18, 2023

Conversation

blackdevelopa
Copy link
Contributor

@blackdevelopa blackdevelopa commented Mar 13, 2023 •
edited

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description
Should show the token symbol when verifying contract details.

Screenshots/Recordings
Before:
http://recordit.co/qNXHWrukFz

After
http://recordit.co/Xv4CQeEOeg

Issue

Progresses #5872

Checklist

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

@github-actions
Copy link
Contributor

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 marked this pull request as ready for review March 13, 2023 13:36
@blackdevelopa blackdevelopa requested a review from a team as a code owner March 13, 2023 13:36
@blackdevelopa blackdevelopa self-assigned this Mar 13, 2023
@blackdevelopa blackdevelopa added team-confirmations-secure-ux-PR PR from the confirmations team needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Mar 13, 2023
@blackdevelopa blackdevelopa changed the title show token symbol in verify contract details Show token symbol in verify contract details Mar 13, 2023
@jpuri
Copy link
Contributor

jpuri commented Mar 13, 2023

Hey code change is perfect, again can you plz check about test coverage.

jpuri
jpuri previously approved these changes Mar 20, 2023
View reviewed changes
@blackdevelopa blackdevelopa 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 Mar 22, 2023
@seaona seaona 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 Apr 6, 2023
@seaona
Copy link
Contributor

seaona commented Apr 13, 2023 •
edited

@blackdevelopa
I've seen that whenever I click on Verify Contract details MM crashes with the error Can't find variable: tokenSymbol.
I am on Android.

Screencast.from.13-04-23.15.16.48.webm

Repro Steps

  1. Go to https://etherscan.io/token/0x6b175474e89094c44da98b954eedeac495271d0f#writeContract
  2. Connect MM
  3. Click on Approve (add any address and any number)
  4. See MM wallet opens
  5. Click Verify Contract Details
  6. See how MM crashes

@seaona seaona 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 Apr 13, 2023
@blackdevelopa blackdevelopa dismissed jpuri’s stale review April 17, 2023 12:55

Requesting for a second review

@seaona
Copy link
Contributor

seaona commented Apr 17, 2023

@blackdevelopa I see the issue fixed. The behaviour looks good, I can see the token symbol on Verify Contract details:

mobile-token-contract-details.mp4

We need x2 dev re-approvals and we can merge it

@seaona seaona added QA Passed A successful QA run through has been done and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Apr 18, 2023
@bschorchit bschorchit merged commit 374b395 into main Apr 18, 2023
13 checks passed
@bschorchit bschorchit deleted the verify_token_symbol branch April 18, 2023 23:59
@github-actions github-actions bot locked and limited conversation to collaborators Apr 18, 2023
@bschorchit
Copy link

I've went ahead and merged it and added the next release label as it had the ✅ from Mariona, Jyoti and Olu 😊

@blackdevelopa
Copy link
Contributor Author

Thank you @bschorchit

@seaona
Copy link
Contributor

seaona commented Apr 19, 2023

thank you @bschorchit !!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@segun segun segun approved these changes

@jpuri jpuri jpuri approved these changes

Assignees

@blackdevelopa blackdevelopa

Labels
QA Passed A successful QA run through has been done release-6.5.0 team-confirmations-secure-ux-PR PR from the confirmations team unit test coverage confirmed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@blackdevelopa @jpuri @seaona @bschorchit @segun @hilvmason