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

pull out CSS color validator and validate <meta> theme-color #117

Merged
merged 2 commits into from
Apr 4, 2016
Merged

pull out CSS color validator and validate <meta> theme-color #117

merged 2 commits into from
Apr 4, 2016

Conversation

brendankenny
Copy link
Member

also puts all of the current code to get chrome-devtools-frontend able to be imported into lighthouse (the other place was network-recorder.js) into one file for easier maintenance.

fixes #92

@@ -26,7 +27,10 @@ class ThemeColor extends Gather {
return driver.querySelector('head meta[name="theme-color"]')
.then(node => node && node.getAttribute('content'))
.then(themeColorMeta => {
return {themeColorMeta};
if (themeColorMeta && validColor(themeColorMeta)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should test this behavior :) (I know it is not complex, but think of the test as ensuring the contract is held up)

Copy link
Member Author

Choose a reason for hiding this comment

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

so, setting up a test for this made it clear it was a bit weird to have in the gather phase. I've pushed it into the audit itself, and added tests for it there :)

@brendankenny
Copy link
Member Author

moved the valid meta theme-color check into the audit, added debugStrings for missing meta theme-color tag, missing color on the tag, and invalid color in the tag, and added testing for an invalid color and the existence of warnings when the audit fails as above.

Since the audits are type checked, also added externs for WebInspector.Color.parse.

PTAL

@paulirish
Copy link
Member

v nice. wfm

@samccone samccone added the +1 label Apr 3, 2016
@addyosmani addyosmani added the +2 label Apr 3, 2016
@brendankenny brendankenny merged commit beab048 into GoogleChrome:master Apr 4, 2016
@brendankenny brendankenny deleted the inspector branch April 4, 2016 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a helper for checking if a CSS color is valid
4 participants