chore: removing warning alternative color from codebase by georgewrmarshall · Pull Request #25010 · MetaMask/metamask-extension

georgewrmarshall · 2024-06-03T21:29:41Z

Description

This pull request removes the deprecated warning colors(warning-alternative and warning-disabled) in preparation for upgrading to design token v4.

Related issues

Fixes: #24966

Manual testing steps

Go to the storybook build of this PR
Go to all stories that did use the warning alternative:
See color updates are working as expected
Ensure not instances of warning-alternative, warning-disabled exists

Screenshots/Recordings

All before/after screenshots and screencasts have been moved to comments for easier review. This screenshot shows no instances of warning-alternative CSS variables or enums left in the codebase

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-03T21:29:54Z

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-03T21:38:16Z

ui/components/app/confirm/info/row/value-double.tsx

  [ConfirmInfoRowVariant.Critical]: Color.errorAlternative,
-  [ConfirmInfoRowVariant.Warning]: Color.warningAlternative,
+  [ConfirmInfoRowVariant.Warning]: Color.warningDefault,
 };


Before

After

georgewrmarshall · 2024-06-03T21:38:17Z

ui/components/app/confirm/info/row/row.tsx

  [ConfirmInfoRowVariant.Critical]: Color.errorAlternative,
-  [ConfirmInfoRowVariant.Warning]: Color.warningAlternative,
+  [ConfirmInfoRowVariant.Warning]: Color.warningDefault,
 };


Before

After

can you show an example with an icon?

georgewrmarshall · 2024-06-03T21:38:21Z

.storybook/3.COLORS.stories.mdx

-var(--color-warning-alternative) [DEPRECATED]
 var(--color-warning-muted)
 var(--color-warning-inverse)
-var(--color-warning-disabled) [DEPRECATED]


Before

before.colordocs720.mov

After

colordocs720.mov

georgewrmarshall · 2024-06-03T21:39:22Z

ui/components/component-library/header-base/header-base.stories.tsx

+    <Box backgroundColor={BackgroundColor.warningDefault}>
      <HeaderBase
        marginBottom={4}
        startAccessory={


Before

After

unblocking: i feel like these should use brand colors instead

georgewrmarshall · 2024-06-03T21:40:44Z

ui/components/multichain/pages/page/page.stories.tsx

+      <Footer backgroundColor={BackgroundColor.warningDefault}>
        <ButtonPrimary block disabled>
          Cancel
        </ButtonPrimary>


Before

After

georgewrmarshall · 2024-06-03T21:42:34Z

ui/css/design-system/_colors.scss

  'error-inverse': --color-error-inverse,
  'error-disabled': --color-error-disabled,
  'warning-default': --color-warning-default,
-  'warning-alternative': --color-warning-alternative,


No instances of warning-alternative remain in codebase

georgewrmarshall · 2024-06-03T21:42:44Z

ui/css/design-system/_colors.scss

-  'warning-alternative': --color-warning-alternative,
  'warning-muted': --color-warning-muted,
  'warning-inverse': --color-warning-inverse,
-  'warning-disabled': --color-warning-disabled,


No instances of warning-disabled remain in codebase

georgewrmarshall · 2024-06-03T21:42:56Z

ui/helpers/constants/design-system.ts

  errorInverse = 'error-inverse',
  errorDisabled = 'error-disabled',
  warningDefault = 'warning-default',
-  warningAlternative = 'warning-alternative',


No instances of warningAlternative remain in codebase

georgewrmarshall · 2024-06-03T21:43:03Z

ui/helpers/constants/design-system.ts

-  warningAlternative = 'warning-alternative',
  warningMuted = 'warning-muted',
  warningInverse = 'warning-inverse',
-  warningDisabled = 'warning-disabled',


No instances of warningDisabled remain in codebase

georgewrmarshall · 2024-06-03T21:43:10Z

ui/components/ui/button/buttons.scss

    &:active {
-      border-color: var(--color-warning-alternative);
+      border-color: var(--color-warning-default);
    }


Before

before.button.mov

After

after.buttonwarning720.mov

shouldn't the text be colored as well?

Good question. This is from a deprecated button that I believe isn't used anywhere, so I'm just keeping it consistent with what is currently there. See Before screencast

Screenshot shows no usage of type warning for a button component

georgewrmarshall · 2024-06-03T21:54:48Z

ui/pages/onboarding-flow/create-password/index.scss


  &__average {
-    color: var(--color-warning-alternative);
+    color: var(--color-warning-default);


Before

After

Small improvement to accessibility color contrasts here

georgewrmarshall · 2024-06-03T22:10:50Z

ui/helpers/constants/design-system.ts

  errorAlternative = 'error-alternative',
  errorMuted = 'error-muted',
  warningDefault = 'warning-default',
-  warningAlternative = 'warning-alternative',


Same as above

georgewrmarshall · 2024-06-03T22:10:54Z

ui/helpers/constants/design-system.ts

  errorAlternative = 'error-alternative',
  errorMuted = 'error-muted',
  warningDefault = 'warning-default',
-  warningAlternative = 'warning-alternative',


Same as above

metamaskbot · 2024-06-03T22:33:03Z

Builds ready [25bb804]

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 (54 ± 5 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	101	85	11	5
		domContentLoaded	9	27	12	5	2
		load	42	76	54	10	5
		domInteractive	9	27	12	5	2

codecov · 2024-06-03T22:33:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.71%. Comparing base (63851c9) to head (25bb804).
Report is 54 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #25010      +/-   ##
===========================================
- Coverage    65.71%   65.71%   -0.00%     
===========================================
  Files         1368     1368              
  Lines        54388    54384       -4     
  Branches     14150    14150              
===========================================
- Hits         35739    35735       -4     
  Misses       18649    18649

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

brianacnguyen

a few small comments

chore: removing warning alternative color from codebase

ab4a6d1

georgewrmarshall self-assigned this Jun 3, 2024

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

chore: removing warning disabled color from codebase

25bb804

georgewrmarshall commented Jun 3, 2024

View reviewed changes

georgewrmarshall requested a review from brianacnguyen June 3, 2024 22:11

georgewrmarshall marked this pull request as ready for review June 3, 2024 22:11

georgewrmarshall requested review from a team as code owners June 3, 2024 22:11

brianacnguyen reviewed Jun 4, 2024

View reviewed changes

georgewrmarshall requested a review from brianacnguyen June 5, 2024 22:12

brianacnguyen approved these changes Jun 5, 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 29db397 into develop Jun 7, 2024

georgewrmarshall deleted the remove-warning-alternative 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

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

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Jun 3, 2024

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Before

After

Uh oh!

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

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!

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

Uh oh!

Choose a reason for hiding this comment

Before

After

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Before

After

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Before

After

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

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!

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

Uh oh!

Choose a reason for hiding this comment

Before

georgewrmarshall commented Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

georgewrmarshall Jun 3, 2024 •

edited

Loading

codecov bot commented Jun 3, 2024 •

edited

Loading