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 test for Proximity API #1373

Merged
merged 1 commit into from
Nov 27, 2014
Merged

Add test for Proximity API #1373

merged 1 commit into from
Nov 27, 2014

Conversation

alrra
Copy link
Contributor

@alrra alrra commented Jun 14, 2014

There is one detail that is currently included in the MDN link, namely:

"Obviously, the API requires the device to have a proximity sensor, which are mostly available only on mobile devices. Devices without such a sensor may support those events but will never fire them."

that I think is important, and which I don't know if should be included somewhere inline or not. (Cc: @patrickkettner)

@ryanseddon
Copy link
Member

@alrra it'd be good to add a deep link if possible to that information in the meta data

@alrra
Copy link
Contributor Author

alrra commented Jun 16, 2014

deep link if possible

@ryanseddon it's not.

@patrickkettner
Copy link
Member

As is, this is a 👎 from me - firefox desktop triggers a false positive. Personally, I think that "Devices without such a sensor may support those events but will never fire them" is a broken API, for the same reason we don't say that desktop browsers have touchscreens.

But, thats beside the point.

could as addAsyncTest this after checking for the actual deviceproximity event for values?

@patrickkettner
Copy link
Member

ping @alrra

@alrra alrra changed the title Add test for Proximity Events Add test for Proximity API Sep 24, 2014
@alrra
Copy link
Contributor Author

alrra commented Sep 24, 2014

@patrickkettner Updated! Let me know if I need to change anything else.

Modernizr.addAsyncTest(function () {

var timeout;
var timeoutTime = 300;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@patrickkettner 300 because I've seen it used before.

patrickkettner added a commit that referenced this pull request Nov 27, 2014
Add test for `Proximity API`
@patrickkettner patrickkettner merged commit 1d4d5f3 into Modernizr:master Nov 27, 2014
patrickkettner added a commit to patrickkettner/Modernizr that referenced this pull request Feb 22, 2015
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.

None yet

3 participants