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

UX: Multichain: Move PickerNetwork to Wallet Tabs #21220

Closed
wants to merge 10 commits into from

Conversation

darkwing
Copy link
Contributor

@darkwing darkwing commented Oct 6, 2023

Description

The multichain effort requires that we move the PickerNetwork from the header to each home tab. This PR moves the PickerNetwork to each of those tabs. This PR does not account for the formatting of the header.

Manual testing steps

  1. If the MULTICHAIN flag is not enabled, nothing should change.
  2. With the MULTICHAIN flag enabled, see no PickerNetwork in the unlocked state, regardless of full screen or popup
  3. Clicking the PickerNetwork should launch the Network menu, and clicking an item should change the network
  4. Test changing networks on each Wallet screen tab.

Screenshots/Recordings

SCR-20231005-tjfg

Related issues

Fixes: https://app.zenhub.com/workspaces/metamask-extension-63529dce65cbb100265a3842/issues/gh/metamask/metamask-planning/683

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained:
    • What problem this PR is solving.
    • How this problem was solved.
    • How reviewers can test my changes.
  • I’ve indicated what issue this PR is linked to: Fixes #???
  • I’ve included tests if applicable.
  • I’ve documented any added code.
  • I’ve applied the right labels on the PR (see labeling guidelines).
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

@darkwing darkwing requested a review from a team as a code owner October 6, 2023 02:54
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

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.

@darkwing darkwing added team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead needs-ux-ds-review labels Oct 6, 2023
@darkwing darkwing force-pushed the 683-multichain-network-picker branch from ea7be75 to a14bdd3 Compare October 6, 2023 03:44
vthomas13
vthomas13 previously approved these changes Oct 6, 2023
darkwing added a commit that referenced this pull request Oct 10, 2023
## **Description**
This PR adds a separator to the Multichain global menu under View on
Explorer

## **Manual testing steps**

1.  Open the Global Menu
2. See the separator between View on Explorer and its next menu item

## **Screenshots/Recordings**

<img width="290" alt="SCR-20231005-txgc"
src="https://github.com/MetaMask/metamask-extension/assets/46655/5e8dbbd5-45dc-4eda-999a-117580569190">


## **Related issues**

Fixes: #21220

This issue also references the position of the Account Picker but that
is addressed in
#21220

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained:
  - [ ] What problem this PR is solving.
  - [ ] How this problem was solved.
  - [ ] How reviewers can test my changes.
- [ ] I’ve indicated what issue this PR is linked to: Fixes #???
- [ ] I’ve included tests if applicable.
- [ ] I’ve documented any added code.
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 10, 2023
owencraston pushed a commit that referenced this pull request Oct 10, 2023
## **Description**
This PR adds a separator to the Multichain global menu under View on
Explorer

## **Manual testing steps**

1.  Open the Global Menu
2. See the separator between View on Explorer and its next menu item

## **Screenshots/Recordings**

<img width="290" alt="SCR-20231005-txgc"
src="https://github.com/MetaMask/metamask-extension/assets/46655/5e8dbbd5-45dc-4eda-999a-117580569190">


## **Related issues**

Fixes: #21220

This issue also references the position of the Account Picker but that
is addressed in
#21220

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained:
  - [ ] What problem this PR is solving.
  - [ ] How this problem was solved.
  - [ ] How reviewers can test my changes.
- [ ] I’ve indicated what issue this PR is linked to: Fixes #???
- [ ] I’ve included tests if applicable.
- [ ] I’ve documented any added code.
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
Woodpile37 added a commit to Woodpile37/metamask-extension that referenced this pull request Oct 11, 2023
commit f987a9d
Merge: 5522af7 1e5578e
Author: Gustavo Antunes <[email protected]>
Date:   Tue Oct 10 21:46:06 2023 -0300

    Merge branch 'develop' into feat/accounts-feedback-link

commit 1e5578e
Author: Daniel Rocha <[email protected]>
Date:   Tue Oct 10 23:32:20 2023 +0200

    Add `getAllowedKeyringMethods` hook for Account Snaps (MetaMask#21246)

    This PR adds the `getAllowedKeyringMethods` hook. It is used in the `rpc-methods` packed to get the list of allowed Keyring methods for a given origin.

    ---------

    Co-authored-by: Frederik Bolding <[email protected]>
    Co-authored-by: Maarten Zuidhoorn <[email protected]>
    Co-authored-by: MetaMask Bot <[email protected]>

commit b8b3dd6
Author: Brad Decker <[email protected]>
Date:   Tue Oct 10 15:17:54 2023 -0500

    ci(codeowners): add privacy reviewers to codeowners (MetaMask#21241)

    ## **Description**
    Further locks down the privacy snapshot by adding codeowners for the
    snapshot file. A new team for this was created by @danjm
    @MetaMask/extension-privacy-reviewers

    ## **Manual testing steps**
    1. In theory if you branch off this PR and change the codeowners, then
    push up your branch you'll see that the team has been added to the PR as
    a codeowner.

    ## **Pre-merge author checklist**

    - [x] I’ve followed [MetaMask Coding
    Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
    - [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] I’ve indicated what issue this PR is linked to: Fixes #???
    - [x] I’ve included tests if applicable.
    - [x] I’ve documented any added code.
    - [x] I’ve applied the right labels on the PR (see [labeling
    guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
    - [x] I’ve properly set the pull request status:
      - [ ] In case it's not yet "ready for review", I've set it to "draft".
    - [x] In case it's "ready for review", I've changed it from "draft" to
    "non-draft".

    ## **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.

commit a6f058f
Author: David Walsh <[email protected]>
Date:   Tue Oct 10 12:12:37 2023 -0500

    UX: Multichain: Add separator under 'View on Explorer' (MetaMask#21223)

    ## **Description**
    This PR adds a separator to the Multichain global menu under View on
    Explorer

    ## **Manual testing steps**

    1.  Open the Global Menu
    2. See the separator between View on Explorer and its next menu item

    ## **Screenshots/Recordings**

    <img width="290" alt="SCR-20231005-txgc"
    src="https://github.com/MetaMask/metamask-extension/assets/46655/5e8dbbd5-45dc-4eda-999a-117580569190">

    ## **Related issues**

    Fixes: MetaMask#21220

    This issue also references the position of the Account Picker but that
    is addressed in
    MetaMask#21220

    ## **Pre-merge author checklist**

    - [ ] I’ve followed [MetaMask Coding
    Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
    - [ ] I've clearly explained:
      - [ ] What problem this PR is solving.
      - [ ] How this problem was solved.
      - [ ] How reviewers can test my changes.
    - [ ] I’ve indicated what issue this PR is linked to: Fixes #???
    - [ ] I’ve included tests if applicable.
    - [ ] I’ve documented any added code.
    - [ ] I’ve applied the right labels on the PR (see [labeling
    guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
    - [ ] I’ve properly set the pull request status:
      - [ ] In case it's not yet "ready for review", I've set it to "draft".
    - [ ] In case it's "ready for review", I've changed it from "draft" to
    "non-draft".

    ## **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.
@darkwing darkwing reopened this Oct 11, 2023
@darkwing darkwing force-pushed the 683-multichain-network-picker branch from c27bb61 to 200eabd Compare October 11, 2023 18:55
@metamaskbot
Copy link
Collaborator

Builds ready [8a1e6c2]
Page Load Metrics (748 ± 351 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8111491105
domContentLoaded6511584157
load761601748731351
domInteractive6511584157
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 753 Bytes (0.01%)
  • common: 0 Bytes (0.00%)

Comment on lines +756 to +757
e.stopPropagation();
e.preventDefault();
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: can we move these 2 events related actions to component PickerNetwork level?

@darkwing
Copy link
Contributor Author

This is blocked by #21301

@darkwing darkwing force-pushed the 683-multichain-network-picker branch from 8a1e6c2 to 68b400c Compare October 13, 2023 13:18
@metamaskbot
Copy link
Collaborator

Builds ready [30dad92]
Page Load Metrics (1126 ± 401 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint871471132010
domContentLoaded701441062311
load8120151126836401
domInteractive701441062311
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 698 Bytes (0.01%)
  • common: 0 Bytes (0.00%)

@darkwing darkwing added the DO-NOT-MERGE Pull requests that should not be merged label Oct 17, 2023
k-g-j pushed a commit that referenced this pull request Nov 1, 2023
## **Description**
This PR adds a separator to the Multichain global menu under View on
Explorer

## **Manual testing steps**

1.  Open the Global Menu
2. See the separator between View on Explorer and its next menu item

## **Screenshots/Recordings**

<img width="290" alt="SCR-20231005-txgc"
src="https://github.com/MetaMask/metamask-extension/assets/46655/5e8dbbd5-45dc-4eda-999a-117580569190">


## **Related issues**

Fixes: #21220

This issue also references the position of the Account Picker but that
is addressed in
#21220

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained:
  - [ ] What problem this PR is solving.
  - [ ] How this problem was solved.
  - [ ] How reviewers can test my changes.
- [ ] I’ve indicated what issue this PR is linked to: Fixes #???
- [ ] I’ve included tests if applicable.
- [ ] I’ve documented any added code.
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
@darkwing darkwing closed this Dec 4, 2023
Woodpile37 added a commit to Woodpile37/metamask-extension that referenced this pull request Jan 9, 2024
commit f987a9d
Merge: 5522af7 1e5578e
Author: Gustavo Antunes <[email protected]>
Date:   Tue Oct 10 21:46:06 2023 -0300

    Merge branch 'develop' into feat/accounts-feedback-link

commit 1e5578e
Author: Daniel Rocha <[email protected]>
Date:   Tue Oct 10 23:32:20 2023 +0200

    Add `getAllowedKeyringMethods` hook for Account Snaps (MetaMask#21246)

    This PR adds the `getAllowedKeyringMethods` hook. It is used in the `rpc-methods` packed to get the list of allowed Keyring methods for a given origin.

    ---------

    Co-authored-by: Frederik Bolding <[email protected]>
    Co-authored-by: Maarten Zuidhoorn <[email protected]>
    Co-authored-by: MetaMask Bot <[email protected]>

commit b8b3dd6
Author: Brad Decker <[email protected]>
Date:   Tue Oct 10 15:17:54 2023 -0500

    ci(codeowners): add privacy reviewers to codeowners (MetaMask#21241)

    ## **Description**
    Further locks down the privacy snapshot by adding codeowners for the
    snapshot file. A new team for this was created by @danjm
    @MetaMask/extension-privacy-reviewers

    ## **Manual testing steps**
    1. In theory if you branch off this PR and change the codeowners, then
    push up your branch you'll see that the team has been added to the PR as
    a codeowner.

    ## **Pre-merge author checklist**

    - [x] I’ve followed [MetaMask Coding
    Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
    - [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] I’ve indicated what issue this PR is linked to: Fixes #???
    - [x] I’ve included tests if applicable.
    - [x] I’ve documented any added code.
    - [x] I’ve applied the right labels on the PR (see [labeling
    guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
    - [x] I’ve properly set the pull request status:
      - [ ] In case it's not yet "ready for review", I've set it to "draft".
    - [x] In case it's "ready for review", I've changed it from "draft" to
    "non-draft".

    ## **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.

commit a6f058f
Author: David Walsh <[email protected]>
Date:   Tue Oct 10 12:12:37 2023 -0500

    UX: Multichain: Add separator under 'View on Explorer' (MetaMask#21223)

    ## **Description**
    This PR adds a separator to the Multichain global menu under View on
    Explorer

    ## **Manual testing steps**

    1.  Open the Global Menu
    2. See the separator between View on Explorer and its next menu item

    ## **Screenshots/Recordings**

    <img width="290" alt="SCR-20231005-txgc"
    src="https://github.com/MetaMask/metamask-extension/assets/46655/5e8dbbd5-45dc-4eda-999a-117580569190">

    ## **Related issues**

    Fixes: MetaMask#21220

    This issue also references the position of the Account Picker but that
    is addressed in
    MetaMask#21220

    ## **Pre-merge author checklist**

    - [ ] I’ve followed [MetaMask Coding
    Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
    - [ ] I've clearly explained:
      - [ ] What problem this PR is solving.
      - [ ] How this problem was solved.
      - [ ] How reviewers can test my changes.
    - [ ] I’ve indicated what issue this PR is linked to: Fixes #???
    - [ ] I’ve included tests if applicable.
    - [ ] I’ve documented any added code.
    - [ ] I’ve applied the right labels on the PR (see [labeling
    guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
    - [ ] I’ve properly set the pull request status:
      - [ ] In case it's not yet "ready for review", I've set it to "draft".
    - [ ] In case it's "ready for review", I've changed it from "draft" to
    "non-draft".

    ## **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.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DO-NOT-MERGE Pull requests that should not be merged team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants