-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Move "View on Etherscan" to the top of the three dot menu #12345
Milestone
Comments
Closed by: #12384 |
6 tasks
darkwing
added a commit
that referenced
this issue
Aug 3, 2022
darkwing
added a commit
that referenced
this issue
Aug 3, 2022
darkwing
added a commit
that referenced
this issue
Aug 3, 2022
darkwing
added a commit
that referenced
this issue
Aug 4, 2022
darkwing
added a commit
that referenced
this issue
Aug 5, 2022
darkwing
added a commit
that referenced
this issue
Aug 15, 2022
16 tasks
8 tasks
8 tasks
8 tasks
racitores
added a commit
that referenced
this issue
Sep 22, 2023
## Explanation Currently MMI extension is assuming that all general/core MM extension features should work under MMI but this is not been tested in the pipeline. This PR adds Metamask e2e tests run in the circle ci pipeline for a MMI build generated extension. There are a set failing tests marked as `@no-mmi` so that when running the tests with `yarn test:e2e:chrome:mmi` will do `SELENIUM_BROWSER=chrome node test/e2e/run-all.js --mmi` passing --mmi as param. This is a first step to improve MMI test coverage and regression. Next steps will be reviewed and updated tests if required <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [x] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [ ] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required.
DDDDDanica
pushed a commit
that referenced
this issue
Sep 22, 2023
Deleting line "The recipient's hexadecimal address is shown in the activity list item." as the hexadecimal address is not supposed to be shown in the activity list item unless you open the activity list item. ## Explanation <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [ ] I've clearly explained: - [ ] What problem this PR is solving - [ ] How this problem was solved - [ ] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required.
benjisclowder
added a commit
that referenced
this issue
Sep 25, 2023
Deleting line "The recipient's hexadecimal address is shown in the activity list item." as the hexadecimal address is not supposed to be shown in the activity list item unless you open the activity list item. ## Explanation <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [ ] I've clearly explained: - [ ] What problem this PR is solving - [ ] How this problem was solved - [ ] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required.
DDDDDanica
pushed a commit
that referenced
this issue
Sep 25, 2023
Deleting line "The recipient's hexadecimal address is shown in the activity list item." as the hexadecimal address is not supposed to be shown in the activity list item unless you open the activity list item, in the next step. ## Explanation <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [ ] I've clearly explained: - [ ] What problem this PR is solving - [ ] How this problem was solved - [ ] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required.
DDDDDanica
pushed a commit
that referenced
this issue
Sep 25, 2023
## Deleting line "The recipient's hexadecimal address is shown in the activity list item." as the hexadecimal address is not supposed to be shown in the activity list item unless you open the activity list item, in the next step. <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [ ] I've clearly explained: - [ ] What problem this PR is solving - [ ] How this problem was solved - [ ] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required.
mikesposito
added a commit
that referenced
this issue
Sep 26, 2023
## Explanation With this PR the extension starts to use the `QRKeyring` provided by `KeyringController`, instead of declaring and managing it directly in the client. <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> * Fixes #18776 ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> - QR Hardware Wallets should work as before. ## Pre-merge author checklist - [ ] I've clearly explained: - [ ] What problem this PR is solving - [ ] How this problem was solved - [ ] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required. --------- Co-authored-by: Salah <salah-eddine.saakoun@consensys.net> Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
blackdevelopa
added a commit
that referenced
this issue
Oct 4, 2023
## Explanation We should allow users and developers to report false positives for the security provider warnings. Tasks - [x] Add option on UI to report false design. [See deign](https://www.figma.com/file/Q69vHaA7Or7Rxz4vdzjVHC/Support-for-tx-security-providers-(MVP)?type=design&node-id=6065%3A12121&mode=design&t=oYcxRuTnTvDrzqAM-1). The support url might change in the future. - [x] Add new property to transactions and signature events <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> <img width="438" alt="Screenshot 2023-09-15 at 16 53 46" src="/~https://github.com/MetaMask/metamask-extension/assets/29962968/5c4ab365-7979-4df3-9842-77a9a906956e"> ### After <!-- How does it look now? Drag your file(s) below this line: --> <img width="434" alt="Screenshot 2023-09-21 at 11 33 07" src="/~https://github.com/MetaMask/metamask-extension/assets/29962968/774920c7-afb1-483b-8a05-c24b9bd26843"> <img width="432" alt="Screenshot 2023-09-21 at 11 33 24" src="/~https://github.com/MetaMask/metamask-extension/assets/29962968/dc890014-827b-4029-897d-b9df47edcc6f"> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [ ] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [x] How reviewers can test my changes - [x] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [x] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [x] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required.
garrettbear
pushed a commit
that referenced
this issue
Oct 5, 2023
## Explanation - This PR fixes part of #19528 - Replaces instances of `ActionableMessage` with `BannerAlert` in `ui/pages/settings/advanced-tab/advanced-tab.component.js` <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ![before_success](/~https://github.com/MetaMask/metamask-extension/assets/46365255/624b4da9-0b59-4b83-b564-593b9c1328a8) ![before_danger](/~https://github.com/MetaMask/metamask-extension/assets/46365255/1549b1b7-88e9-4459-b1c1-7d39b2ddfa70) ### After <!-- How does it look now? Drag your file(s) below this line: --> ![after_success](/~https://github.com/MetaMask/metamask-extension/assets/46365255/41079132-ae02-48e5-afec-98b2fd22925b) ![after_danger](/~https://github.com/MetaMask/metamask-extension/assets/46365255/aeaad47a-9461-4942-bdd1-e9018167c6e5) ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> - Create build from this PR - From Menu go to Settings -> Advanced - Scroll Down to find **Restore user data** section. - Try Restoring with proper/corrupted file. ## Pre-merge author checklist - [x] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [x] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required. --------- Co-authored-by: georgewrmarshall <george.marshall@consensys.net>
jiexi
added a commit
that referenced
this issue
Oct 18, 2023
## Explanation Wallets shouldn't be directly concerned about the network ID as this more of a p2p concept for gossip. What wallets really care about is chain ID as that is the correct value to use to identify a chain, build transactions, etc. Although these two values usually match (ignoring hex/dec formatting), there are [exceptions](https://medium.com/@pedrouid/chainid-vs-networkid-how-do-they-differ-on-ethereum-eec2ed41635b). We want to remove usage of networkId and replace it with chainId where appropriate and necessary. This was a generally straight forward change to make, except for the following cases: * `networkVersion` on the `window.ethereum` provider object * `metamaskNetworkId` on TransactionMeta objects ~~In both cases, we now use static mappings to assist in covering chainId <-> networkId edge cases.~~ See comments in this PR for more elaboration and the core PR linked below. * Fixes [mmp-1068](MetaMask/MetaMask-planning#1068) * See: [core-1633](MetaMask/core#1633) * See: /~https://github.com/MetaMask/smart-transactions-controller <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [x] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [x] How reviewers can test my changes - [x] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [x] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [x] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required. --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com> Co-authored-by: Alex Donesky <adonesky@gmail.com>
chloeYue
added a commit
that referenced
this issue
Oct 26, 2023
## Explanation Test scenarios for eth sign, personal sign, sign typed data, sign in with ethereum, encrypt & decrypt message, and connecting and disconnecting from dapp Closes #18941 Closes #18940 Closes #18929 <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [x] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [x] How reviewers can test my changes - [x] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required. --------- Co-authored-by: Marina Boboc <120041701+benjisclowder@users.noreply.github.com> Co-authored-by: Chloe Gao <chloe.gao@consensys.net> Co-authored-by: chloeYue <105063779+chloeYue@users.noreply.github.com>
k-g-j
pushed a commit
that referenced
this issue
Nov 1, 2023
## Explanation We should allow users and developers to report false positives for the security provider warnings. Tasks - [x] Add option on UI to report false design. [See deign](https://www.figma.com/file/Q69vHaA7Or7Rxz4vdzjVHC/Support-for-tx-security-providers-(MVP)?type=design&node-id=6065%3A12121&mode=design&t=oYcxRuTnTvDrzqAM-1). The support url might change in the future. - [x] Add new property to transactions and signature events <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> <img width="438" alt="Screenshot 2023-09-15 at 16 53 46" src="/~https://github.com/MetaMask/metamask-extension/assets/29962968/5c4ab365-7979-4df3-9842-77a9a906956e"> ### After <!-- How does it look now? Drag your file(s) below this line: --> <img width="434" alt="Screenshot 2023-09-21 at 11 33 07" src="/~https://github.com/MetaMask/metamask-extension/assets/29962968/774920c7-afb1-483b-8a05-c24b9bd26843"> <img width="432" alt="Screenshot 2023-09-21 at 11 33 24" src="/~https://github.com/MetaMask/metamask-extension/assets/29962968/dc890014-827b-4029-897d-b9df47edcc6f"> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [ ] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [x] How reviewers can test my changes - [x] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [x] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [x] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required.
k-g-j
pushed a commit
that referenced
this issue
Nov 1, 2023
## Explanation - This PR fixes part of #19528 - Replaces instances of `ActionableMessage` with `BannerAlert` in `ui/pages/settings/advanced-tab/advanced-tab.component.js` <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ![before_success](/~https://github.com/MetaMask/metamask-extension/assets/46365255/624b4da9-0b59-4b83-b564-593b9c1328a8) ![before_danger](/~https://github.com/MetaMask/metamask-extension/assets/46365255/1549b1b7-88e9-4459-b1c1-7d39b2ddfa70) ### After <!-- How does it look now? Drag your file(s) below this line: --> ![after_success](/~https://github.com/MetaMask/metamask-extension/assets/46365255/41079132-ae02-48e5-afec-98b2fd22925b) ![after_danger](/~https://github.com/MetaMask/metamask-extension/assets/46365255/aeaad47a-9461-4942-bdd1-e9018167c6e5) ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> - Create build from this PR - From Menu go to Settings -> Advanced - Scroll Down to find **Restore user data** section. - Try Restoring with proper/corrupted file. ## Pre-merge author checklist - [x] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [x] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required. --------- Co-authored-by: georgewrmarshall <george.marshall@consensys.net>
k-g-j
pushed a commit
that referenced
this issue
Nov 1, 2023
## Explanation Wallets shouldn't be directly concerned about the network ID as this more of a p2p concept for gossip. What wallets really care about is chain ID as that is the correct value to use to identify a chain, build transactions, etc. Although these two values usually match (ignoring hex/dec formatting), there are [exceptions](https://medium.com/@pedrouid/chainid-vs-networkid-how-do-they-differ-on-ethereum-eec2ed41635b). We want to remove usage of networkId and replace it with chainId where appropriate and necessary. This was a generally straight forward change to make, except for the following cases: * `networkVersion` on the `window.ethereum` provider object * `metamaskNetworkId` on TransactionMeta objects ~~In both cases, we now use static mappings to assist in covering chainId <-> networkId edge cases.~~ See comments in this PR for more elaboration and the core PR linked below. * Fixes [mmp-1068](/~https://github.com/MetaMask/MetaMask-planning/issues/1068) * See: [core-1633](MetaMask/core#1633) * See: /~https://github.com/MetaMask/smart-transactions-controller <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [x] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [x] How reviewers can test my changes - [x] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [x] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [x] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required. --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com> Co-authored-by: Alex Donesky <adonesky@gmail.com>
garrettbear
added a commit
that referenced
this issue
Nov 20, 2023
### Please note that `SelectButton` and `SelectOption` are WIP files but are needed to make working with SelectWrapper possible. ## Explanation `SelectWrapper` is the context api/data wrapper for what will be used with `SelectButton`, `SelectOption`, and other future components. _This component doesn't have any visual aspects to it, so anything with see from the UI is temporary_ and will be updated in future PRs ( #20609 and #20610 ) Fixes #20607 - There is a lot to digest with the `SelectWrapper` component as it does have ties to `SelectButton` and `SelectOption`. Something worth pointing out is there is the option to have **controlled and uncontrolled for both value and the open state**. - Controlled open uses props `isOpen` and `onOpenChange` - Controlled value uses props `defaultValue`, `value`, and `onValueChange` - The SelectWrapper is in itself a context api and has a `useSelectContext` hook that can be utilized when creating custom features and use controlled props. See the `SelectContextType` to understand what may be used with this hook. To Do / WIP Issues: _It's a bit unclear if it really makes sense to have these at the `SelectWrapper` level._ - onBlur - onFocus - onChange <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> <img width="1290" alt="Screenshot 2023-10-11 at 11 08 28 AM" src="/~https://github.com/MetaMask/metamask-extension/assets/26469696/3a60a891-53e9-49b6-bc2e-7fed8679ee07"> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [x] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [x] How reviewers can test my changes - [x] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [x] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [x] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required. --------- Co-authored-by: George Marshall <george.marshall@consensys.net>
garrettbear
pushed a commit
that referenced
this issue
Jan 24, 2024
## Explanation - This PR fixes Part of #19528 and #18896 - Replaces `ActionableMessage` with `BannerAlert` in `ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.js` - Replaces old `<Button />` component (ui/components/component-library) with new `<Button />` component (ui/components/component-library/button/button.js) inside the `BannerAlert` component. <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before **Token With Same Symbol But Different Address (Before)** ![token-with-same-symbol-but-different-address_Before](/~https://github.com/MetaMask/metamask-extension/assets/46365255/cf97ac7f-53e4-409e-ae35-e692a677f282) **Token With Duplicate Contract Address (Before)** ![Same-address-different-symbol_Before](/~https://github.com/MetaMask/metamask-extension/assets/46365255/35291d3f-4262-44cb-a166-da2358381ba1) <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After **Token With Same Symbol But Different Address (After)** ![after_with_diff_addrs](/~https://github.com/MetaMask/metamask-extension/assets/46365255/97f8ed17-8880-45e2-be86-a83f86aedda5) **Token With Duplicate Contract Address (After)** ![after_with_same_address](/~https://github.com/MetaMask/metamask-extension/assets/46365255/f5d75432-7499-4133-bc08-136acafedc98) <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps 1. Create and Install the new build with this PR. 2. Visit and Connect the Wallet on [Test Dapp Site](https://metamask.github.io/test-dapp/) . 3. Input token address, symbol and decimal under **_EIP 747_** section and click **_Add Token_**. 4. Test with a token that is already added to the wallet. - Input same address but different token name. - Input different contract address and same token name. <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [x] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [x] How reviewers can test my changes - [x] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required. --------- Co-authored-by: George Marshall <george.marshall@consensys.net> Co-authored-by: David Walsh <davidwalsh83@gmail.com>
VGau
added a commit
that referenced
this issue
Mar 21, 2024
## Explanation Removed Linea mainnet feature toggle and all related functions and conditions. <!-- Thanks for the pull request. Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues, Slack conversations, Zendesk issues, user stories, etc. reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Screenshots/Screencaps <!-- If you're making a change to the UI, make sure to capture a screenshot or a short video showing off your work! --> ### Before <!-- How did the UI you changed look before your changes? Drag your file(s) below this line: --> ### After <!-- How does it look now? Drag your file(s) below this line: --> ## Manual Testing Steps <!-- How should reviewers and QA manually test your changes? For instance: - Go to this screen - Do this - Then do this --> ## Pre-merge author checklist - [X] I've clearly explained: - [x] What problem this PR is solving - [x] How this problem was solved - [ ] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone If further QA is required (e.g. new feature, complex testing steps, large refactor), add the `Extension QA Board` label. In this case, a QA Engineer approval will be be required.
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Click the three dot menu icon to see this:
Move the "View on Etherscan" option to the top.
The text was updated successfully, but these errors were encountered: