-
Notifications
You must be signed in to change notification settings - Fork 59
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
Introduce Default Sensor concept #279
Introduce Default Sensor concept #279
Conversation
This solves an inline issue. PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few comments.
index.bs
Outdated
<pre highlight="js"> | ||
let sensor = new GeolocationSensor({ accuracy: "high" }); | ||
|
||
sensor.onreading = function(event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can remove event? Wdyt?
index.bs
Outdated
while still enabling more complex use cases. | ||
|
||
Most of devices deployed today do not carry more than one | ||
[=device sensor=] corresponding to the same [=sensor type|sensor types=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of the same sensor type?
index.bs
Outdated
[=device sensor=] corresponding to the same [=sensor type|sensor types=]. | ||
The use cases which require a set of similar [=device sensors=] are rare | ||
and generally limited to specific [=sensor types=], | ||
such as proximity sensors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not really, accelerometers are better example, especially for 2-in-1 or convertible devices. Maybe change proximity to accelerometer?
index.bs
Outdated
the <dfn export>default sensor</dfn> is chosen. | ||
|
||
<div class="example"> | ||
Listening to geolocation changes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to use sensors that are actually specified, implemented.
index.bs
Outdated
Note: extension to this specification may choose not to define a default sensor | ||
when doing so wouldn't make sense. | ||
For example, it might be difficult to agree on an obvious default [=device sensor|sensor=] for | ||
proximity [=device sensor|sensors=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change proximity to geolocation.
396b6c2
to
e0e88e7
Compare
@alexshalamov ptal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks much better, thanks!
@anssiko could you PTAL? |
pr-preview did not work for some reason, so here's the HTML diff: |
- Move the dedicated text from the 'Background' to the 'Concepts' section - Cleanup
e0e88e7
to
b918749
Compare
Preview | Diff