| LC-2738
   Anne van Kesteren <annevk@annevk.nl>(archived comment) | On Tue, Dec 18, 2012 at 1:27 PM, Anne van Kesteren <annevk@annevk.nl> wrote:> [...]
 
 I just noticed another thing. The default values for the event members
 are not defined. You need to do that, otherwise (new
 DeviceLightEvent("trololol").value) is not defined.
 
 See http://dom.spec.whatwg.org/#defining-event-interfaces for advice
 on defining new event interfaces.
 | https://dvcs.w3.org/hg/dap/diff/2f675457d430/light/Overview.html 
 https://dvcs.w3.org/hg/dap/diff/51296676e203/light/Overview.html
 
 reply sent to Anne: http://lists.w3.org/Archives/Public/public-device-apis/2013Jan/0021.html
 | tocheck | 
|---|
| LC-2736
   Tab Atkins Jr. <jackalmage@gmail.com>(archived comment) | Heya!  I saw the LC announcement in public-webapps, and have some feedback.
 * The introduction isn't very well written - it jumps straight into
 technical details (and too much of them), without adequate explanation
 of the purpose of the spec and the defined interfaces. I suggest
 replacing it with something like:
 
 This specification defines events that provide information about the
 ambient light level, as measured by a device's light sensor.  A
 LightLevelEvent describes the light level as one of three simple
 categories - "dim", "normal", and "bright" - while a DeviceLightEvent
 provides a more granular answer by describing the light level in terms
 of lux units.
 
 * Chapter 5 should start with an intro that describes the event.  It's
 fine for this to be somewhat repetitive with the spec's own intro -
 the point is to make it sensical for someone to jump straight into
 this section.  I suggest something like:
 
 The DeviceLightEvent interface provides information about the ambient
 light levels, as detected by the device's light detector, in terms of
 lux units.
 
 * Chapter 5 should have a note (for authors) that the precise lux
 value reported by different devices in the same light may be
 different, due to differences in detection method, sensor
 construction, etc.
 
 * 5.1 and 5.2.3 seem to be duplicate information.  I suspect this is a
 result of ReSpec, but this should be avoided if possible.  Same for
 6.1 and 6.2.3.
 
 * Similar, Chapter 6 should have an intro.  I suggest something like:
 
 The LightLevelEvent interface provides information about the ambient
 light levels, as detected by the device's light detector, in terms of
 three general range: "dim", "normal", or "bright".
 
 * The LightLevelEvent interface should use a WebIDL enum, rather than
 describing the values solely in prose.  Here's the IDL necessary:
 
 enum LightLevel { "dim", "normal", "bright" };
 [Constructor (DOMString type, optional LightLevelEventInit eventInitDict)]
 interface LightLevelEvent : Event {
 readonly attribute LightLevel? value;
 };
 dictionary LightLevelEventInit : EventInit {
 LightLevel? value;
 };
 
 
 * In 6.2.1 (the description of the allowed values for the
 LightLevelEvent.value attribute), "may" is used.  Either "must" should
 be used, or it should be made non-normative and switched to "can" or
 something.  I think the latter is appropriate, once the enum (above)
 is adopted, as the WebIDL constitutes sufficient normative
 requirements already.
 
 * In 6.2.2 (the definition of how to fire the event), if the light
 level can't be determined, the .value attribute of the event should be
 null, not the empty string.  I've already handled the nullability in
 the WebIDL above.
 
 * In 6.2.2, the prose is a little unclear about when to fire
 lightlevel events.  I recommend replacing the sentence starting with
 "When the current light changes..." and the following note with
 something like:
 
 This specification does not define the exact lux ranges that the
 LightLevel values map to, as devices with different sensitivities may
 want to map them slightly differently.  However, it recommends that
 "dim" correspond to ambient light below 50lux, "normal" correspond to
 light between 50lux and 10000lux, and "bright" correspond to light
 above 10000lux.  User agents must fire a light level event when the
 current light level changes.  User agents may fire a light level event
 at any other time, for example if they have reason to believe that the
 page does not have sufficiently fresh data.
 
 ~TJ
 | The WebIDL was changed to use enum as you suggested, thank you. 
 Changes were also made to address editorial comments, see the diff https://dvcs.w3.org/hg/dap/rev/1c5fc74bd529
 | yes | 
|---|