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

Consider not defining a custom WakeLockEvent #238

Closed
reillyeon opened this issue Oct 16, 2019 · 2 comments · Fixed by #242
Closed

Consider not defining a custom WakeLockEvent #238

reillyeon opened this issue Oct 16, 2019 · 2 comments · Fixed by #242

Comments

@reillyeon
Copy link
Member

Reading the TAG design principles I noticed § 3.7. State and Event subclasses which recommends against creating custom Event subclasses and instead capturing state in the target parameter.

Example code with current API,

let lock = await navigator.wakeLock.request({ type: "script" });
lock.addEventListener('release', e => {
  console.log(`${e.lock.type} wake lock has been released`);
});

Since the event is already fired on lock the lock attribute on the event could be removed, resulting in this code,

let lock = await navigator.wakeLock.request({ type: "script" });
lock.addEventListener('release', e => {
  console.log(`${e.target.type} wake lock has been released`);
});
@reillyeon
Copy link
Member Author

CC @marcoscaceres who was there when we defined WakeLockEvent and maybe missed that target and lock would have the same value or has a good reason not to rely on it.

@marcoscaceres
Copy link
Member

Yes, great point @reillyeon. We just need a simple Event.

rakuco added a commit to rakuco/wake-lock that referenced this issue Oct 17, 2019
Per https://w3ctag.github.io/design-principles/#state-and-subclassing, we do
not really need a custom event class here.

`WakeLockEvent` has a single attribute, `lock`, that points to the event
target. That's exactly what `Event.target` also does, so we can just create
a simple `Event` instance and let the "fire an event" DOM algorithm set
`target` for us to achieve the same thing. In fact, it already does, so
before this commit we had

    ev.lock === ev.target

Fixes w3c#238.
rakuco added a commit to rakuco/wake-lock that referenced this issue Oct 18, 2019
Per https://w3ctag.github.io/design-principles/#state-and-subclassing, we do
not really need a custom event class here.

`WakeLockEvent` has a single attribute, `lock`, that points to the event
target. That's exactly what `Event.target` also does, so we can just create
a simple `Event` instance and let the "fire an event" DOM algorithm set
`target` for us to achieve the same thing. In fact, it already does, so
before this commit we had

    ev.lock === ev.target

Fixes w3c#238.
rakuco added a commit that referenced this issue Oct 18, 2019
Per https://w3ctag.github.io/design-principles/#state-and-subclassing, we do
not really need a custom event class here.

`WakeLockEvent` has a single attribute, `lock`, that points to the event
target. That's exactly what `Event.target` also does, so we can just create
a simple `Event` instance and let the "fire an event" DOM algorithm set
`target` for us to achieve the same thing. In fact, it already does, so
before this commit we had

    ev.lock === ev.target

Fixes #238.
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 a pull request may close this issue.

2 participants