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

Dark Theme setup and button theming #13651

Merged
merged 11 commits into from
Feb 25, 2022
Merged

Dark Theme setup and button theming #13651

merged 11 commits into from
Feb 25, 2022

Conversation

GuillaumeRx
Copy link
Contributor

Explanation: Add @metamask/design-tokens that includes dark theme color variables and apply it on the button component

@GuillaumeRx GuillaumeRx requested a review from a team as a code owner February 16, 2022 17:38
@GuillaumeRx GuillaumeRx added the DO-NOT-MERGE Pull requests that should not be merged label Feb 16, 2022
@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.

@GuillaumeRx GuillaumeRx self-assigned this Feb 16, 2022
@GuillaumeRx
Copy link
Contributor Author

Waiting on MetaMask/design-tokens#22 to be merged and deployed to continue on button theming

@GuillaumeRx GuillaumeRx requested a review from darkwing February 16, 2022 17:39
@darkwing darkwing marked this pull request as draft February 16, 2022 17:43
@darkwing darkwing changed the title [WIP] Dark Theme setup and button theming Dark Theme setup and button theming Feb 16, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [216fdd6]
Page Load Metrics (1230 ± 46 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint711355469456219
domContentLoaded1070139912309646
load1070139912309646
domInteractive1070139912309646

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [d48ceb5]
Page Load Metrics (1174 ± 40 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint661231531454218
domContentLoaded1041139311748441
load1046139311748440
domInteractive1041139311748441

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [09f4237]
Page Load Metrics (1195 ± 59 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint661455763572275
domContentLoaded10461460119312460
load10461460119512459
domInteractive10461460119312460

highlights:

storybook

@GuillaumeRx GuillaumeRx removed the DO-NOT-MERGE Pull requests that should not be merged label Feb 22, 2022
@GuillaumeRx GuillaumeRx marked this pull request as ready for review February 22, 2022 17:07
@brad-decker
Copy link
Contributor

omg i love this so much.

brad-decker
brad-decker previously approved these changes Feb 22, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [80bec7d]
Page Load Metrics (1206 ± 134 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint661119455442212
domContentLoaded100817991203279134
load101417991206279134
domInteractive100817991203279134

highlights:

storybook

@darkwing
Copy link
Contributor

Seeing this on the home screen:

Hover.mp4

@GuillaumeRx
Copy link
Contributor Author

Seeing this on the home screen:

Hover.mp4

@darkwing oh yes good catch, we don't have a darker in between that matches the original hover color. @georgewrmarshall what's you thoughts about this ?

@darkwing
Copy link
Contributor

This issue is also happening on the "Settings" > "About" screen on btn-link

@metamaskbot
Copy link
Collaborator

Builds ready [8f30b3a]
Page Load Metrics (1144 ± 47 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint651361510482231
domContentLoaded1049136811429747
load1049136811449747
domInteractive1049136811429747

highlights:

storybook

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.

Looking good. I've added some of my suggestions to a branch that I've created a PR into this branch for #13742

.storybook/preview.js Outdated Show resolved Hide resolved
@metamaskbot
Copy link
Collaborator

Builds ready [06b6e58]
Page Load Metrics (1141 ± 28 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint701193541482231
domContentLoaded1046132611415828
load1046132711415828
domInteractive1046132611415828

highlights:

storybook

@GuillaumeRx GuillaumeRx linked an issue Feb 24, 2022 that may be closed by this pull request
georgewrmarshall added a commit that referenced this pull request Feb 24, 2022
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.

Looks great! Love the dark theme / background implementation in storybook 💯 I'll update the Box PR to revert to develop so this implementation can be used.

darkwing
darkwing previously approved these changes Feb 25, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [405bcff]
Page Load Metrics (1286 ± 69 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint721407653569273
domContentLoaded10541608128414570
load10541608128614369
domInteractive10541608128414570

highlights:

storybook

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!

@georgewrmarshall georgewrmarshall merged commit 23e6c07 into develop Feb 25, 2022
@georgewrmarshall georgewrmarshall deleted the dark-theme-setup branch February 25, 2022 22:11
@github-actions github-actions bot locked and limited conversation to collaborators Feb 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Dark Mode
5 participants