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.

Sign up for GitHub

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

Jump to bottom

Avoid repeating CSS color vars #13464

Merged
merged 1 commit into from
Jan 31, 2022
Merged

Avoid repeating CSS color vars #13464

merged 1 commit into from
Jan 31, 2022

Conversation

darkwing
Copy link
Contributor

@darkwing darkwing commented Jan 28, 2022 •
edited
Loading

@georgewrmarshall and I noticed that the CSS variables were being repeated in the eventual output, likely because the design system is included in multiple places. This wasn't a problem in the past because colors.scss only contained variables.

To test:

  1. Load the extension, ensure all the colors are as they should be.
  2. Manually open the generated index.css file and ensure the vars are only declared once (you can search for :root)

@darkwing darkwing requested a review from a team as a code owner January 28, 2022 23:12
@darkwing darkwing requested a review from danjm January 28, 2022 23:12
@github-actions
Copy link
Contributor

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.

@metamaskbot
Copy link
Collaborator

Builds ready [e412474]
Page Load Metrics (1204 ± 53 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint691258400429206
domContentLoaded10731503120411153
load10731503120411153
domInteractive10731503120411153

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [73c7480]
Page Load Metrics (1250 ± 130 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6451412411354
domContentLoaded102818121250270130
load102818121250270130
domInteractive102818121250270130

highlights:

storybook

digiwand
digiwand approved these changes Jan 31, 2022
View reviewed changes
Copy link
Contributor

@digiwand digiwand left a comment

Choose a reason for hiding this comment

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

LGTM! 🌈

adonesky1
adonesky1 approved these changes Jan 31, 2022
View reviewed changes
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

Nice!

@darkwing darkwing merged commit c1b5e18 into develop Jan 31, 2022
@darkwing darkwing deleted the optimize-css-vars branch January 31, 2022 19:14
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@adonesky1 adonesky1 adonesky1 approved these changes

@digiwand digiwand digiwand approved these changes

@danjm danjm Awaiting requested review from danjm danjm was automatically assigned from MetaMask/extension-devs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@darkwing @metamaskbot @digiwand @adonesky1