-
-
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
feat(3448): header update #11763
feat(3448): header update #11763
Conversation
## **Description** This is NOT being merged into `main` and is being merged into the feature branch of [feat-header-update](https://github.com/MetaMask/metamask-mobile/tree/feat-header-update) Updating the `NetworkPicker` design according to this [Figma](https://www.figma.com/design/aMYisczaJyEsYl1TYdcPUL/Portfolio-View?m=auto&node-id=5019-59597). This is just a component update only and NOT the full header redesign. The prop of `hideNetworkName` is introduced to the component and the default value is `false`. We could potentially just remove that flag altogether and remove the actual network name. Just need confirmation from DS team ## **Related issues** Feature: [#3447](MetaMask/MetaMask-planning#3447) ## **Manual testing steps** 1. Goto wallet home page 2. Goto `app/components/UI/Navbar/index.js` and scroll down to `PickerNetwork` and add prop `hideNetworkName={true}` 3. Or goto storybook ## **Screenshots/Recordings** ### Light Mode | Regular | Hidden | |:---:|:---:| |![light_mode_regular](https://github.com/user-attachments/assets/0a889689-2d2f-4066-ba8b-355736a0228d)|![light_mode_hidden](https://github.com/user-attachments/assets/d0f0bade-9094-4973-bef6-1f4915b04b4d)| ### Dark Mode | Regular | Hidden | |:---:|:---:| |![dark_mode_regular](https://github.com/user-attachments/assets/2a2a1fb3-4004-4715-b3eb-822ad3faf572)|![dark_mode_hidden](https://github.com/user-attachments/assets/efafd673-50a7-4bee-84ba-6553db55528d)| ### Storybook <img src="https://github.com/user-attachments/assets/590d5ce2-4d21-4c09-9e24-edcb7784897c" width="320px" alt="storybook"/> ### **Before** NA ### **After** NA ## **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.
## **Description** This is NOT being merged into `main` and is being merged into the feature branch of [feat-header-update](https://github.com/MetaMask/metamask-mobile/tree/feat-header-update) Updating the `AddressCopy` component to be the new design according to this [Figma](https://www.figma.com/design/aMYisczaJyEsYl1TYdcPUL/Portfolio-View?m=auto&node-id=5019-59597). This is just a component update only and NOT the full header redesign. The component has been simplified and there are no other affected screens other than `WalletAccount` component The `WalletAccount` will possibly be removed in this PR as well after some feedback from the @MetaMask/design-system team on what they think. It exists no where else and isn't needed ## **Related issues** Feature: [#3449](MetaMask/MetaMask-planning#3449) ## **Manual testing steps** 1. Goto wallet home page (ignore the fact the wallet action component is missing) ## **Screenshots/Recordings** ![address_copy](https://github.com/user-attachments/assets/64f76cbb-9e09-45a1-bbbd-c0e94ddbdf05) ### **Before** NA ### **After** NA ## **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.
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: 8a0b55b Note
Tip
|
Bitrise❌❌❌ Commit hash: bf53cb4 Note
Tip
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11763 +/- ##
==========================================
+ Coverage 55.35% 55.57% +0.22%
==========================================
Files 1767 1781 +14
Lines 39841 40083 +242
Branches 4965 4995 +30
==========================================
+ Hits 22052 22275 +223
- Misses 16274 16286 +12
- Partials 1515 1522 +7 ☔ View full report in Codecov by Sentry. |
…uest for easier review
…ile into feat-header-update
Bitrise❌❌❌ Commit hash: fc63b0a Note
Tip
|
Bitrise❌❌❌ Commit hash: 9ea8a5e Note
Tip
|
Bitrise🔄🔄🔄 Commit hash: 9ea8a5e Note
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, good work!
Bitrise❌❌❌ Commit hash: 9ea8a5e Note
Tip
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Bitrise🔄🔄🔄 Commit hash: 9ea8a5e Note
|
Bitrise❌❌❌ Commit hash: 9ea8a5e Note
Tip
|
2 similar comments
Bitrise❌❌❌ Commit hash: 9ea8a5e Note
Tip
|
Bitrise❌❌❌ Commit hash: 9ea8a5e Note
Tip
|
Bitrise🔄🔄🔄 Commit hash: 0132dfb Note
|
Quality Gate passedIssues Measures |
Bitrise✅✅✅ Commit hash: 0132dfb Note
|
Description
The header is being updated according to this Figma design (second to last screen).
Because
WalletAccount
is being removed, the more icon or overflow icon will be moved to theAccountSelectorList
. Here is the PR for more infoRegression Tests Passing (11/04/24): https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6224a4f4-1c55-43cf-a622-ec6d91f42110?tab=workflows
Related issues
Ticket: #3448
Manual testing steps
Wallet Home Page
Copy Address
Network Switcher
Overflow (Edit Account): This is on feat(3481): overflow update #11823
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist