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

Remove WakeLockEvent and WakeLockEventInit. #242

Merged
merged 1 commit into from
Oct 18, 2019

Conversation

rakuco
Copy link
Member

@rakuco rakuco commented 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 #238.


The following tasks have been completed:

  • Modified Web platform tests (link to pull request)

Implementation commitment:


Preview | Diff

@rakuco rakuco requested review from reillyeon, kenchris and marcoscaceres and removed request for reillyeon October 17, 2019 10:01
@rakuco
Copy link
Member Author

rakuco commented Oct 17, 2019

Testing this in WPT depends on web-platform-tests/wpt#5671 for permissions support in testdriver.js. I've added a test to Blink in the meantime in https://chromium-review.googlesource.com/c/chromium/src/+/1865347

Copy link
Member

@reillyeon reillyeon left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@marcoscaceres marcoscaceres left a comment

Choose a reason for hiding this comment

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

Just a small thing, but otherwise all good 👍

index.html Outdated Show resolved Hide resolved
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.
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.

Consider not defining a custom WakeLockEvent
4 participants