Skip to content

chore: adding storybook stories to review colors#25083

Merged
georgewrmarshall merged 3 commits intodevelopfrom
adding-stories
Jun 10, 2024
Merged

chore: adding storybook stories to review colors#25083
georgewrmarshall merged 3 commits intodevelopfrom
adding-stories

Conversation

@georgewrmarshall
Copy link
Copy Markdown
Contributor

@georgewrmarshall georgewrmarshall commented Jun 5, 2024 •
edited
Loading

Description

This pull request adds Storybook stories for the following components: AppLoadingSpinner, EthOverview, and LoadingScreen. The goal is to provide better visual documentation and enable easier testing of these components within Storybook. It also breaks up a PR to remove all deprecated secondary colors.

Open in GitHub Codespaces

Related issues

Part of: #24964

Manual testing steps

  1. Go to storybook build of this PR or start the Storybook server using yarn storybook.
  2. In the Storybook interface, navigate to the stories for AppLoadingSpinner, EthOverview, and LoadingScreen.
  3. Verify that the components render correctly and interact as expected

Screenshots/Recordings

Before

before.sb.mov

After

after.sb.mov

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • 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 format if applicable.
  • I’ve applied the right labels on the PR (see labeling guidelines). 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.

@georgewrmarshall georgewrmarshall self-assigned this Jun 5, 2024
@georgewrmarshall georgewrmarshall added the team-design-system All issues relating to design system in Extension label Jun 5, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 5, 2024 •
edited
Loading

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.

@georgewrmarshall georgewrmarshall marked this pull request as ready for review June 5, 2024 22:23
@georgewrmarshall georgewrmarshall requested a review from a team as a code owner June 5, 2024 22:23
@georgewrmarshall georgewrmarshall mentioned this pull request Jun 5, 2024
Merged
7 tasks
@brianacnguyen
Copy link
Copy Markdown
Contributor

brianacnguyen commented Jun 5, 2024

is app loading spinner supposed to be just gray like that?

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 5, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.64%. Comparing base (ddf0e93) to head (1a8bb37).
Report is 1 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #25083      +/-   ##
===========================================
- Coverage    65.64%   65.64%   -0.00%     
===========================================
  Files         1362     1362              
  Lines        54151    54151              
  Branches     14074    14074              
===========================================
- Hits         35546    35545       -1     
- Misses       18605    18606       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@georgewrmarshall
Copy link
Copy Markdown
Contributor Author

georgewrmarshall commented Jun 5, 2024 •
edited
Loading

is app loading spinner supposed to be just gray like that?

Good question! This PR just adds the Storybook files and does not touch any component colors. This is what is currently in production. This was part of your suggestion to break up this PR #24971 (comment)

@brianacnguyen
Copy link
Copy Markdown
Contributor

brianacnguyen commented Jun 5, 2024

is app loading spinner supposed to be just gray like that?

Good question! This PR just adds the Storybook files and does not touch any component colors. This is what is currently in production. This was part of your suggestion to break up this PR #24971 (comment)

Hmm can you double check if its indeed broken on the component side or is there anything missing from the story pov? if it's indeed broken then i can just approve it so there's at least a story for someone to come in and fix

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 5, 2024

Builds ready [fb627d2]
Page Load Metrics (101 ± 118 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint5813678157
domContentLoaded8201031
load401167101245118
domInteractive8201031
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@georgewrmarshall
Copy link
Copy Markdown
Contributor Author

georgewrmarshall commented Jun 5, 2024 •
edited
Loading

is app loading spinner supposed to be just gray like that?

Good question! This PR just adds the Storybook files and does not touch any component colors. This is what is currently in production. This was part of your suggestion to break up this PR #24971 (comment)

Hmm can you double check if its indeed broken on the component side or is there anything missing from the story pov? if it's indeed broken then i can just approve it so there's at least a story for someone to come in and fix

Yup, definitely the component. You can check by going to the Storybook build of this PR. It's using secondary-muted, which is a transparent orange. Link to the component

after720.mov

brianacnguyen
brianacnguyen previously approved these changes Jun 5, 2024
View reviewed changes
@georgewrmarshall georgewrmarshall mentioned this pull request Jun 6, 2024
Merged
7 tasks
@georgewrmarshall georgewrmarshall added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Jun 6, 2024
georgewrmarshall
georgewrmarshall commented Jun 7, 2024
View reviewed changes
brianacnguyen
brianacnguyen previously approved these changes Jun 7, 2024
View reviewed changes
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 7, 2024

Builds ready [39b8665]
Page Load Metrics (49 ± 3 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint701018194
domContentLoaded8141011
load43734973
domInteractive8131011
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 7, 2024

Builds ready [927c036]
Page Load Metrics (50 ± 4 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6810582115
domContentLoaded9301142
load41795084
domInteractive9291142
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@georgewrmarshall georgewrmarshall requested a review from a team as a code owner June 7, 2024 22:05
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 7, 2024

Builds ready [1a8bb37]
Page Load Metrics (46 ± 3 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint59927784
domContentLoaded9151011
load39634663
domInteractive9151011
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@georgewrmarshall georgewrmarshall merged commit ed5ec2d into develop Jun 10, 2024
@georgewrmarshall georgewrmarshall deleted the adding-stories branch June 10, 2024 03:29
@github-actions github-actions bot locked and limited conversation to collaborators Jun 10, 2024
@metamaskbot metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 10, 2024
@georgewrmarshall georgewrmarshall changed the title chore: adding storybook stories chore: adding storybook stories to review colors Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@brianacnguyen brianacnguyen brianacnguyen approved these changes

@garrettbear garrettbear Awaiting requested review from garrettbear

+1 more reviewer

@darkwing darkwing darkwing approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@georgewrmarshall georgewrmarshall

Labels

needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-12.1.0 Issue or pull request that will be included in release 12.1.0 team-design-system All issues relating to design system in Extension

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@georgewrmarshall @brianacnguyen @metamaskbot @darkwing