chore: consolidating spinner color and refactoring institutional qr code modal by georgewrmarshall · Pull Request #25011 · MetaMask/metamask-extension

georgewrmarshall · 2024-06-03T22:41:35Z

Description

Breaking out parts of #24971 for easier review. This PR replaces deprecated colors in favor of the most updated design system colors. I also noticed that this particular component could benefit from some layout and overflow updates.

Related issues

Part of: #24964

Manual testing steps

Go to the storybook build of this page
Search for Spinner and QRCodeModal
Check colors have been updated properly in light and dark mode

Screenshots/Recordings

Before

before720.mov

After

after.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.

github-actions · 2024-06-03T22:41:48Z

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 · 2024-06-03T23:02:17Z

ui/components/institutional/institutional-components.scss

 **/
 @import 'jwt-dropdown/jwt-dropdown';
 @import 'note-to-trader/index';
+@import 'qr-code-modal/qr-code-modal';


importing stylesheet to main index styles file

georgewrmarshall · 2024-06-03T23:02:27Z

ui/components/institutional/qr-code-modal/qr-code-modal.scss

@@ -0,0 +1,5 @@
+.institutional-qr-code-modal {
+  &__qr-code {
+    background-color: var(--qr-code-white-background);


Moving inline styles to custom classname

georgewrmarshall · 2024-06-03T23:03:04Z

ui/components/institutional/qr-code-modal/qr-code-modal.stories.tsx


 const meta: Meta<typeof QRCodeModal> = {
-  title: 'Components/QRCodeModal',
+  title: 'Components/Institutional/QRCodeModal',


Fixing storybook title

Before

After

georgewrmarshall · 2024-06-03T23:03:38Z

ui/components/institutional/qr-code-modal/qr-code-modal.tsx

-            style={{
-              padding: 20,
-              backgroundColor: 'var(--qr-code-white-background)',
-              display: 'flex',
-              alignItems: 'center',
-              flexDirection: 'column',
-            }}


Removing inlines styles in favor of style utility props and custom classname

georgewrmarshall · 2024-06-03T23:03:54Z

ui/components/institutional/qr-code-modal/qr-code-modal.tsx

-              flexDirection: 'column',
-            }}
+
+        <ModalBody>


ModalBody handles overflow using a scroll container

georgewrmarshall · 2024-06-03T23:04:17Z

ui/components/institutional/qr-code-modal/qr-code-modal.tsx

+              <QRCode value={qrCodeValue} size={270} />
+            </Box>
+          ) : (
+            <Spinner />


We can remove the color as it now defaults to the correct icon-muted color

georgewrmarshall · 2024-06-03T23:05:44Z

ui/components/ui/spinner/spinner.component.js


-const Spinner = ({ className = '', color = 'var(--color-text-default)' }) => {
+const Spinner = ({ className = '', color = 'var(--color-icon-muted)' }) => {
  return (


Before

before.mov

After

after.mov

georgewrmarshall · 2024-06-03T23:05:53Z

ui/components/ui/spinner/spinner.stories.js

  },
  args: {
-    color: 'var(--color-warning-default)',
+    color: 'var(--color-icon-muted)',


Same as above

brianacnguyen

approved on ds side

albertolive · 2024-06-04T10:43:27Z

Hi @georgewrmarshall,

The refactoring looks great! I really appreciate your work on this.

I have one small request: Could you please reduce the size of the loader? It currently appears quite large, and adjusting it to a smaller size would be perfect.

metamaskbot · 2024-06-04T21:16:22Z

Builds ready [3a9c51c]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (339 ± 320 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	70	159	102	25	12
		domContentLoaded	8	53	16	12	6
		load	42	2174	339	667	320
		domInteractive	8	53	16	12	6

Bundle size diffs

background: 0 Bytes (0.00%)
ui: -2 Bytes (-0.00%)
common: 0 Bytes (0.00%)

codecov · 2024-06-04T21:16:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.70%. Comparing base (88b0fdb) to head (7fa0895).
Report is 45 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #25011   +/-   ##
========================================
  Coverage    65.70%   65.70%           
========================================
  Files         1369     1369           
  Lines        54364    54365    +1     
  Branches     14149    14150    +1     
========================================
+ Hits         35716    35717    +1     
  Misses       18648    18648

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

georgewrmarshall · 2024-06-05T20:13:03Z

ui/components/institutional/qr-code-modal/qr-code-modal.scss

+  &__spinner {
+    width: 100px;
+  }


Reducing the size of the spinner

georgewrmarshall · 2024-06-05T20:14:58Z

Hi @georgewrmarshall,

The refactoring looks great! I really appreciate your work on this.

I have one small request: Could you please reduce the size of the loader? It currently appears quite large, and adjusting it to a smaller size would be perfect.

Not a problem! Updated and updated screencasts in description @albertolive

metamaskbot · 2024-06-05T20:42:37Z

Builds ready [7fa0895]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (52 ± 3 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	68	105	83	7	3
		domContentLoaded	9	20	11	3	1
		load	44	70	52	7	3
		domInteractive	9	20	11	3	1

Bundle size diffs

background: 0 Bytes (0.00%)
ui: -2 Bytes (-0.00%)
common: 0 Bytes (0.00%)

georgewrmarshall added the team-design-system All issues relating to design system in Extension label Jun 3, 2024

georgewrmarshall self-assigned this Jun 3, 2024

chore: refactoring institutional qr code modal

f38b38d

georgewrmarshall force-pushed the update-qr-code-modal branch from 065d980 to f38b38d Compare June 3, 2024 23:01

georgewrmarshall commented Jun 3, 2024

View reviewed changes

georgewrmarshall requested a review from brianacnguyen June 3, 2024 23:08

georgewrmarshall marked this pull request as ready for review June 3, 2024 23:08

georgewrmarshall requested review from a team as code owners June 3, 2024 23:08

brianacnguyen previously approved these changes Jun 3, 2024

View reviewed changes

albertolive previously approved these changes Jun 4, 2024

View reviewed changes

Merge branch 'develop' into update-qr-code-modal

3a9c51c

chore: updating size of the spinner

7fa0895

georgewrmarshall dismissed stale reviews from brianacnguyen and albertolive via 7fa0895 June 5, 2024 20:11

georgewrmarshall commented Jun 5, 2024

View reviewed changes

georgewrmarshall requested review from albertolive and brianacnguyen June 5, 2024 20:13

brianacnguyen approved these changes Jun 5, 2024

View reviewed changes

georgewrmarshall mentioned this pull request Jun 5, 2024

chore: removing deprecated secondary colors from codebase #24971

Merged

7 tasks

albertolive approved these changes Jun 6, 2024

View reviewed changes

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 mentioned this pull request Jun 6, 2024

chore: upgrading to design tokens v4 #24953

Merged

7 tasks

darkwing approved these changes Jun 7, 2024

View reviewed changes

georgewrmarshall merged commit c1a5959 into develop Jun 7, 2024

georgewrmarshall deleted the update-qr-code-modal branch June 7, 2024 19:53

github-actions bot locked and limited conversation to collaborators Jun 7, 2024

metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 7, 2024

georgewrmarshall changed the title ~~chore: refactoring institutional qr code modal~~ chore: consolidating spinner color and refactoring institutional qr code modal Jun 10, 2024

Conversation

georgewrmarshall commented Jun 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Jun 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Before

After

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Before

After

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brianacnguyen left a comment

Choose a reason for hiding this comment

Uh oh!

albertolive commented Jun 4, 2024

Uh oh!

metamaskbot commented Jun 4, 2024

Uh oh!

codecov bot commented Jun 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

georgewrmarshall commented Jun 5, 2024

Uh oh!

metamaskbot commented Jun 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

georgewrmarshall commented Jun 3, 2024 •

edited

Loading

codecov bot commented Jun 4, 2024 •

edited

Loading