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

feat: add eth stake button #9136

Merged
merged 13 commits into from
Apr 10, 2024
Merged

feat: add eth stake button #9136

merged 13 commits into from
Apr 10, 2024

Conversation

ameliejyc
Copy link
Contributor

@ameliejyc ameliejyc commented Apr 4, 2024

Description

This change adds a 'Stake' button link to the Portfolio stake page next to the ETH token on mainnet. The reason for this change is to have parity with the extension, give users easier access to MM Staking, and drive revenue.

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/STAKE-629

Request template: https://www.notion.so/metamask-consensys/Add-Stake-Button-to-Mobile-f160ebc0f6b6437c9b3d0f943851771a

Manual testing steps

  • Go to Token Listing
  • Verify that the new staking icon shows on for ETH mainnet
  • Verify that the new staking icon doesn't show on any other token than ETH, and not on any other chain than mainnet
  • Verify that onPress, the MM browser opens up the Portfolio stake page with the correct query params
  • Verify that if a browser tab is already open on the stake page, the MM browser picks this tab rather than opening a new one

Screenshots/Recordings

stake.button.ios2.mov

Before

image

After

image

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've completed the PR template to the best of my ability
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Copy link
Contributor

github-actions bot commented Apr 4, 2024

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.

@ameliejyc ameliejyc changed the title Add eth stake button feat: add eth stake button Apr 4, 2024
@ameliejyc
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@ameliejyc ameliejyc added team-stake design-review Any feature that needs feedback from the design team labels Apr 4, 2024
@ameliejyc ameliejyc force-pushed the add-eth-stake-button branch 2 times, most recently from 21032ed to d50ea0f Compare April 4, 2024 14:00
@ameliejyc
Copy link
Contributor Author

ameliejyc commented Apr 4, 2024

Two questions:

  1. I have verified that MetaMetrics trackEvent is called with the expected params, however is there anything else I need to do to get this new event updated in Segment? I don't see a Segment mobile tracking plan.
  2. I'm struggling to test this on Android due to build issues. I'm assuming the UI will work the same given it's RN but would like to sanity check regardless. Can someone please help me get it setup or test using my checked out branch?

UPDATE: I fixed my android issues and tested the UI:

image

@ameliejyc ameliejyc force-pushed the add-eth-stake-button branch 2 times, most recently from bc8adc5 to 8bc449c Compare April 5, 2024 09:09
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 68.75000% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 46.00%. Comparing base (4be64ff) to head (8bc449c).

Files Patch % Lines
app/components/UI/Tokens/index.tsx 68.75% 2 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9136      +/-   ##
==========================================
+ Coverage   45.98%   46.00%   +0.01%     
==========================================
  Files        1273     1273              
  Lines       31342    31357      +15     
  Branches     3213     3217       +4     
==========================================
+ Hits        14414    14425      +11     
- Misses      16079    16081       +2     
- Partials      849      851       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ameliejyc ameliejyc marked this pull request as ready for review April 5, 2024 09:34
@ameliejyc ameliejyc requested review from a team as code owners April 5, 2024 09:34
@gauthierpetetin gauthierpetetin added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Apr 5, 2024
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

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

LGTM! I have once question for DS designers but I imagine it will be a change on the design side

app/component-library/components/Icons/Icon/Icon.types.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@brianacnguyen brianacnguyen left a comment

Choose a reason for hiding this comment

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

Looks good. Just 1 comment regarding not needing the Stake Icon

Copy link
Contributor

@NicolasMassart NicolasMassart left a comment

Choose a reason for hiding this comment

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

Tested and code approved.
Please run e2e smoke tests.
Nice work!

Simulator.Screen.Recording.-.iPhone.13.Pro.-.2024-04-08.at.18.48.21.mp4

@ameliejyc ameliejyc added the Run Smoke E2E Triggers smoke e2e on Bitrise label Apr 8, 2024
Copy link
Contributor

github-actions bot commented Apr 8, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: e0ae61d
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/cdbf9bc6-9557-4447-b34f-2c160dcedd74

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@ameliejyc ameliejyc removed the request for review from georgewrmarshall April 9, 2024 07:46
Copy link

sonarcloud bot commented Apr 9, 2024

@ameliejyc ameliejyc merged commit 226d82d into main Apr 10, 2024
31 checks passed
@ameliejyc ameliejyc deleted the add-eth-stake-button branch April 10, 2024 14:38
@github-actions github-actions bot locked and limited conversation to collaborators Apr 10, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Apr 10, 2024
@metamaskbot metamaskbot added the release-7.21.0 Issue or pull request that will be included in release 7.21.0 label Apr 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
design-review Any feature that needs feedback from the design team release-7.21.0 Issue or pull request that will be included in release 7.21.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-stake
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants