Re: [whatwg/dom] add signal to addEventListener (#919)

@annevk commented on this pull request.

Thanks for working on this, found a final set of nits, but it seems this is ready to land soon.

> @@ -1074,12 +1083,14 @@ steps:
  <li><p>Let <var>capture</var> be the result of <a>flattening</a> <var>options</var>.
 
  <li><p>Let <var>once</var> and <var>passive</var> be false.
+ <li><p>Let <var>signal</var> be null.

Newline before this. At least it seems we put each `<li>` on its own island from context.

> @@ -1105,6 +1116,9 @@ participate in a tree structure.</p>
  <a>service worker events</a>, then <a>report a warning to the console</a> that this might not give
  the expected results. [[!SERVICE-WORKERS]]
 
+ <li><p>If <a for="event listener">signal</a> is not null and its [=AbortSignal/aborted flag=] is
+ set then return.

comma before then

> @@ -1113,6 +1127,12 @@ participate in a tree structure.</p>
  <a for="event listener">callback</a>, and <a for="event listener">capture</a> is
  <var>listener</var>'s <a for="event listener">capture</a>, then <a for=list>append</a>
  <var>listener</var> to <var>eventTarget</var>'s <a>event listener list</a>.
+
+  <li><p>If <a for="event listener">signal</a> is not null then <a for=AbortSignal lt=add>add the following</a>
+  abort steps to it:
+    <ol>
+     <li><a>remove an event listener</a> with <var>eventTarget</var> and <var>listener</var>.

Uppercase "r" here.

> @@ -982,6 +987,7 @@ when something has occurred.
  <li><dfn for="event listener">passive</dfn> (a boolean, initially false)
  <li><dfn for="event listener">once</dfn> (a boolean, initially false)
  <li><dfn for="event listener">removed</dfn> (a boolean for bookkeeping purposes, initially false)
+ <li><dfn for="event listener">signal</dfn> (null or an {{AbortSignal}} object)

Let's put this before the bookkeeping "removed"?

>  
  <li><p>If <var>options</var> is a dictionary, then set <var>passive</var> to <var>options</var>'s
- <code>{{AddEventListenerOptions/passive}}</code> and <var>once</var> to <var>options</var>'s
- <code>{{AddEventListenerOptions/once}}</code>.
+ <code>{{AddEventListenerOptions/passive}}</code>, <var>once</var> to <var>options</var>'s
+ <code>{{AddEventListenerOptions/once}}</code> and <var>signal</var> to
+ {{AddEventListenerOptions/signal}}.

It seems this still needs to be done, right? Could also consider nested steps for when options is a dictionary.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/pull/919#pullrequestreview-540934138

Received on Monday, 30 November 2020 14:06:51 UTC