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

Add hairline test. #1769

Merged
merged 1 commit into from Nov 26, 2015

Conversation

Projects
None yet
2 participants
@strarsis
Copy link
Contributor

strarsis commented Nov 24, 2015

This PR adds a test for CSS hairlines support in the browser.
I also found this blog post very useful for explaining CSS hairlines
and how to test for support (test is derived from it): http://dieulot.net/css-retina-hairline

@@ -65,6 +65,7 @@ window.caniusecb = function(caniuse) {
geolocation: 'geolocation',
getrandomvalues: 'getrandomvalues',
getusermedia: 'stream',
hairline: 'hairline',

This comment has been minimized.

@patrickkettner

patrickkettner Nov 26, 2015

Member

I am not seeing this on caniuse - can you remove unlessI am missing something

"name": "Derived from",
"href": "https://gist.github.com/dieulot/520a49463f6058fbc8d1"
}]
}

This comment has been minimized.

@patrickkettner

patrickkettner Nov 26, 2015

Member

can you add a description as to what hairline is? I don't think the naming is super well known

}
!*/ define(['Modernizr', 'testStyles'], function(Modernizr, testStyles) {
Modernizr.addTest('hairline', function() {
return testStyles('#modernizr {border:.5px solid transparent;}', function(elem) {

This comment has been minimized.

@patrickkettner

patrickkettner Nov 26, 2015

Member

no need for a trailing semicolon

!*/ define(['Modernizr', 'testStyles'], function(Modernizr, testStyles) {
Modernizr.addTest('hairline', function() {
return testStyles('#modernizr {border:.5px solid transparent;}', function(elem) {
return elem.offsetHeight == 1;

This comment has been minimized.

@patrickkettner

patrickkettner Nov 26, 2015

Member

triple equals please

@patrickkettner

This comment has been minimized.

Copy link
Member

patrickkettner commented Nov 26, 2015

couple of small nits, then lgtm. After you are done, could you rebase down to a single commit? Thanks!

@strarsis strarsis force-pushed the strarsis:master branch from 76a869f to 17f451e Nov 26, 2015

@strarsis

This comment has been minimized.

Copy link
Contributor Author

strarsis commented Nov 26, 2015

Fixed + squashed.

patrickkettner added a commit that referenced this pull request Nov 26, 2015

@patrickkettner patrickkettner merged commit ece4566 into Modernizr:master Nov 26, 2015

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.