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

report(redesign): respect prefers-color-scheme #8842

Merged
merged 8 commits into from
May 7, 2019
Merged

Conversation

connorjclark
Copy link
Collaborator

You can test this in FF 67 beta. Just need to change OS to dark mode (for osx, Appearance -> Dark)

@connorjclark connorjclark requested a review from paulirish as a code owner May 3, 2019 22:47
@brendankenny
Copy link
Member

force true incase already dark (ex: scores all 100)

really report-ui-features.js should be setting dark mode for fireworks anyways, not report-renderer.js. Not everything that runs the report renderer is showing only the report (PSI, web.dev), so it shouldn't be reaching out and adding classes to body anyways.

Can you move it in here? Not sure what's the best way to do it...e.g check for `.score100', maybe?

@connorjclark
Copy link
Collaborator Author

done. can I remove the check for devtools too?

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

@paulirish
Copy link
Member

failing on travis

image

@connorjclark
Copy link
Collaborator Author

@paulirish fixed

@paulirish
Copy link
Member

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

LGTM!

what's up with travis though, a push 2 hours ago and still pending?

@connorjclark
Copy link
Collaborator Author

FYI Chrome 76 will support prefers-color-scheme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants