-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
chore(js-ts): Convert app/core/Engine.test.js to TypeScript #11799
Closed
devin-ai-integration
wants to merge
16
commits into
main
from
devin/convert-ts-app-core-Engine-test.js-5504
Closed
chore(js-ts): Convert app/core/Engine.test.js to TypeScript #11799
devin-ai-integration
wants to merge
16
commits into
main
from
devin/convert-ts-app-core-Engine-test.js-5504
+7,358
−1,139
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
devin-ai-integration
bot
added
needs-dev-review
PR needs reviews from other engineers (in order to receive required approvals)
team-mobile-platform
Run Smoke E2E
Triggers smoke e2e on Bitrise
skip-sonar-cloud
Only used for bypassing sonar cloud when failures are not relevant to the changes.
labels
Oct 15, 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. |
Bitrise✅✅✅ Commit hash: ff535d1 Note
|
## **Description** Resurrecting an older PR of mine to provide support for custom network images: #10448 ## **Related issues** Fixes: Support custom network icons on mobile. ## **Manual testing steps** 1. Add custom network (In this case Flare Mainnet or Songbird Testnet) 2. Icon should appear wherever Network Icon should appear ## **Screenshots/Recordings** https://github.com/user-attachments/assets/6ea5b310-4b6a-4b6c-9656-d892bae83fc3 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). 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.
## **Description** This PR reduces the sampling rate by 50% Sentry Performance. We are currently using too much of our allocation. ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). 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.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> <img width="1052" alt="Screenshot 2024-10-11 at 20 29 04" src="https://github.com/user-attachments/assets/a5680804-7bc8-45c4-a27a-d790bd5f9a5d"> <img width="766" alt="Screenshot 2024-10-16 at 00 01 02" src="https://github.com/user-attachments/assets/aa4689f2-9ce0-4dc7-9b99-f24fd4db1b78"> <img width="699" alt="Screenshot 2024-10-15 at 17 57 12" src="https://github.com/user-attachments/assets/dc31684a-4dea-4f26-8b18-c5497679a0ce"> This task is for adding custom spans to track activities that happen between app start and wallet UI load. The screenshot below is an example of a trace for Wallet UI load that takes about a minute to load. During that time, we can see a large gap between app start spans and the initial http requests. The goal here is to isolate these areas and track them with custom spans. Once implemented, we can expect to see the custom spans appearing within the gap, which would inform us of the areas to optimize Issue: MetaMask/mobile-planning#1940 Technical Details * Added custom span for when the Login screen is mounted to when the login button is tapped * Added span for when the login button is tapped to when the wallet view is mounted * Added custom span for Engine initialization process * Added custom span for Store creation * Added custom span Storage rehydration * Added custom span fro Create New Wallet to Choose Password * Added custom span for Biometrics authentication ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). 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.
## **Description** Persisting back again token list and phishing list. From 11KBs to 8MB to an wallet with: * 2 accounts * 7 Networks added plus Ethereum and LInea Mainnet Tokens imported by network * - Ethereum: 16 * - Linea: 6 * - Avalanche: 9 * - Binance: 10 * - Base: 1 * - Optimism: 3 * - Polygon: 3 * - Palm:0 * - ZkSync: 1 * - Arbitrum: 5 <img src="https://github.com/user-attachments/assets/a61cc6a3-44ec-4c93-b2d0-5fdc22157c64" width:200 /> App launch times e2e: 1- https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b1ae0ba0-cee8-472d-978e-17882f132740 2- https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d8fda877-0c9a-4448-a75a-cc4921551e16 3- https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/38dc5140-e18e-494c-a86a-22492554e837 ------After fixing e2e------ 1- https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/797b8317-6116-4bb5-8a12-e60a8e1b7aeb ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** Exploring app (tokens): https://github.com/user-attachments/assets/638e42de-2219-423f-a9ee-6b18ada57751 Phishing detector in app browser: https://github.com/user-attachments/assets/62f63451-fa7e-445f-b875-21155b13a8bc ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --------- Co-authored-by: Aslau Mario-Daniel <marioaslau@gmail.com>
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> This PR adds the staking confirmation screen with some mock data being used temporarily. ### Change List - Add staking confirmation screen. - Connect existing `<StakeInputView />` to staking confirmation screen when user enters valid amount to stake. ## **Related issues** Ticket: [FE] Build staking input confirmation screen - ([link](https://consensyssoftware.atlassian.net/browse/STAKE-824)) Figma Designs - [link](https://www.figma.com/design/1c0Y9jDJe6p0j82jydJDcs/Mobile-Staking?node-id=2979-22435&m=dev) ## **Manual testing steps** 1. Add `export MM_POOLED_STAKING_UI_ENABLED=true` to your `.js.env` file. 2. Click on Ethereum In the token list page 3. Scroll down a bit and click "Stake more". This should open the stake input view (not related to this PR) 4. Enter a valid amount to stake and click "Confirm" 5. You should be redirected to a staking confirmation screen. The screen should display the amount to stake in `wETH` and Fiat. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> Nothing would happen after clicking "Confirm" on the stake input view. This screen is new. ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/84ea4c52-50c5-48c3-8077-2c2e8a92bf21 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). 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.
…nd resolve conflicts in Engine.test.ts
Closing this PR as requested. |
github-actions
bot
removed
the
needs-dev-review
PR needs reviews from other engineers (in order to receive required approvals)
label
Oct 16, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
Run Smoke E2E
Triggers smoke e2e on Bitrise
skip-sonar-cloud
Only used for bypassing sonar cloud when failures are not relevant to the changes.
team-ai
AI team (for the Devin AI bot)
team-mobile-platform
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TypeScript Conversion: app/core/Engine.test.js (Work in Progress)
Description
This PR is a work in progress to convert the file 'app/core/Engine.test.js' from JavaScript to TypeScript. The conversion has not been completed yet, but this PR serves as a placeholder for the upcoming changes.
Current Status
Next Steps
Notes
Testing (To be completed)
Link to Devin run: https://preview.devin.ai/devin/48f3b260585d4beda93f99698235dcda
Please review this placeholder PR and provide any feedback or suggestions for the upcoming conversion process.