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

chore: change default network avatar style #28976

Merged
merged 13 commits into from
Dec 11, 2024
Merged

Conversation

micaelae
Copy link
Member

@micaelae micaelae commented Dec 5, 2024

Description

This PR changes the default position of the network avatar to the bottomRight of the token. It also changes the default shape from a circle to a rounded square

Figma: https://www.figma.com/design/IuOIRmU3wI0IdJIfol0ESu/Cross-Chain-Swaps?node-id=7-24563&node-type=canvas&m=dev

Open in GitHub Codespaces

Related issues

Fixes: N/A

Manual testing steps

  1. Visually inspect AvatarNetwork instances (token list, network picker, activity, etc

Screenshots/Recordings

Before

Screenshot 2024-12-09 at 11 20 35 AM
Screenshot 2024-12-09 at 11 20 40 AM

After

Screenshot 2024-12-09 at 11 18 59 AM
Screenshot 2024-12-09 at 11 18 45 AM

Pre-merge author checklist

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.

@metamaskbot
Copy link
Collaborator

Builds ready [1eb299c]
Page Load Metrics (2058 ± 49 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint26222271783617296
domContentLoaded18392213202210048
load18662262205810249
domInteractive289441168
backgroundConnect778332512
firstReactRender179626178
getState1023131514522
initialActions01000
loadScripts1419175715529043
setupStore77716209
uiStartup22173007241319694
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -88 Bytes (-0.00%)
  • common: 48 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [7b5b184]
Page Load Metrics (2004 ± 61 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint18312363201513967
domContentLoaded18212296198012862
load18322312200412761
domInteractive267340147
backgroundConnect107525189
firstReactRender15412163
getState104158132136
initialActions01000
loadScripts13391766155411053
setupStore712921
uiStartup20622706228816278
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -88 Bytes (-0.00%)
  • common: 48 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [cf1d1e8]
Page Load Metrics (2135 ± 98 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint18352613213720297
domContentLoaded18182534208919192
load18402621213520498
domInteractive26117482512
backgroundConnect10168464320
firstReactRender1796272210
getState993901546632
initialActions01000
loadScripts13941938163415775
setupStore713921
uiStartup216132062493271130
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -88 Bytes (-0.00%)
  • common: 51 Bytes (0.00%)

@micaelae micaelae marked this pull request as ready for review December 9, 2024 19:21
@micaelae micaelae requested review from a team as code owners December 9, 2024 19:21
@metamaskbot
Copy link
Collaborator

Builds ready [0d0fbbe]
Page Load Metrics (2233 ± 55 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint34224952055556267
domContentLoaded20142401220211053
load20302491223311455
domInteractive279749199
backgroundConnect11106312612
firstReactRender16176333617
getState1203791636330
initialActions01000
loadScripts1567185517068842
setupStore7331153
uiStartup22513109261420096
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -88 Bytes (-0.00%)
  • common: 51 Bytes (0.00%)

@micaelae micaelae force-pushed the mb894-network-design-updates branch from 0d0fbbe to f62f604 Compare December 10, 2024 02:34
@metamaskbot
Copy link
Collaborator

Builds ready [f62f604]
Page Load Metrics (2288 ± 94 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint35726372101588282
domContentLoaded19852565223219091
load19992632228819794
domInteractive267342126
backgroundConnect987552412
firstReactRender159427168
getState1103901495828
initialActions01000
loadScripts15122048174516780
setupStore77417199
uiStartup222331452677250120
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -88 Bytes (-0.00%)
  • common: 51 Bytes (0.00%)

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.

This is looking great! I’ve tried to check most instances of BadgeWrapper that don’t use AvatarNetwork to ensure that the changes to the defaults haven’t been overridden for those use cases, and so far, so good. I left one small question about the border color in the CSS; otherwise, I think it’s good to go.

I’d also like to understand how this change will be rolled out or planned for mobile and the portfolio. Ideally, they should be released at the same time or very close together to avoid inconsistencies across platforms.

@@ -52,6 +53,8 @@ export const AvatarNetwork: AvatarNetworkComponent = React.forwardRef(
showHalo ? 'mm-avatar-network--with-halo' : '',
className,
)}
borderRadius={BorderRadius.MD}
borderWidth={1}
Copy link
Contributor

@georgewrmarshall georgewrmarshall Dec 10, 2024

Choose a reason for hiding this comment

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

non-blocking: Because of how borders work in the DOM, adding this will effectively reduce the size of the AvatarNetwork by 2px since the border will be included within the avatar size. I believe this has been adjusted in a few places, so it might not be an issue. However, we might need to account for this by applying box-sizing: content-box.

box-sizing.720.mov

Copy link
Member Author

Choose a reason for hiding this comment

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

Good callout. The designs actually call for including the border within the avatar's size so I have removed the content-box sizing from the AvatarNetwork

@micaelae micaelae force-pushed the mb894-network-design-updates branch from f62f604 to 06226c7 Compare December 10, 2024 22:08
@metamaskbot
Copy link
Collaborator

Builds ready [06226c7]
Page Load Metrics (1938 ± 74 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint43723291871364175
domContentLoaded16832270190814972
load16942321193815474
domInteractive25573494
backgroundConnect879342110
firstReactRender1593302613
getState743631507837
initialActions01000
loadScripts12931854149814067
setupStore7251052
uiStartup192729182284273131
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -88 Bytes (-0.00%)
  • common: 57 Bytes (0.00%)

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! 🚀 This is a massive improvement for network icons 💯

  • checked extension build ✅
  • checked storybook ✅
Screenshot 2024-12-11 at 1 16 00 PM Screenshot 2024-12-11 at 1 16 06 PM Screenshot 2024-12-11 at 1 16 28 PM

@georgewrmarshall
Copy link
Contributor

georgewrmarshall commented Dec 11, 2024

I noticed some AvatarNetwork instances that need to be shifted to the bottom-right in NFTs. We might want to address this in a subsequent PR.

Screenshot 2024-12-11 at 1 20 32 PM

@micaelae micaelae added this pull request to the merge queue Dec 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 11, 2024
@micaelae micaelae added this pull request to the merge queue Dec 11, 2024
Merged via the queue into main with commit d758bf9 Dec 11, 2024
75 checks passed
@micaelae micaelae deleted the mb894-network-design-updates branch December 11, 2024 18:54
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2024
@metamaskbot metamaskbot added the release-12.10.1 Issue or pull request that will be included in release 12.10.1 label Dec 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.10.1 Issue or pull request that will be included in release 12.10.1 team-bridge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants