Re: [whatwg/dom] Pass better parent/root information to (re)moving steps. (PR #1450)

@annevk commented on this pull request.



> @@ -2994,8 +2994,9 @@ null.
 <div algorithm>
 <p><a lt="Other applicable specifications">Specifications</a> may define
 <dfn export id=concept-node-move-ext>moving steps</dfn> for all or some <a for=/>nodes</a>. The
-algorithm is passed a <a for=/>node</a> <var>movedNode</var>, and a <a for=/>node</a>-or-null
-<var ignore>oldParent</var> as indicated in the <a for=/>move</a> algorithm below. Like the
+algorithm is passed a <a for=/>node</a> <var>movedNode</var>, a <a for=/>node</a>
+<var ignore>oldAncestor</var>, and a boolean <var ignore>isSubtreeRoot</var>
+as indicated in the <a for=/>move</a> algorithm below. Like the

Shall we make _isSubtreeRoot_ the second argument for clarity?

> @@ -3108,9 +3109,10 @@ to do these things asynchronously, however.
 
   <ol>
    <li>
-    <p>If <var>inclusiveDescendant</var> is <var>node</var>, then run the <a>moving steps</a> with
-    <var>inclusiveDescendant</var> and <var>oldParent</var>. Otherwise, run the <a>moving steps</a>
-    with <var>inclusiveDescendant</var> and null.
+    <p>Let <var>isSubtreeRoot</var> be whether <var>inclusiveDescendant</var> is <var>node</var>.

We are always very explicit with booleans in standards. So I'd prefer

> Let _isSubtreeRoot_ be true if _inclusiveDescendant_ is _node_; otherwise false.

> @@ -3108,9 +3109,10 @@ to do these things asynchronously, however.
 
   <ol>
    <li>
-    <p>If <var>inclusiveDescendant</var> is <var>node</var>, then run the <a>moving steps</a> with
-    <var>inclusiveDescendant</var> and <var>oldParent</var>. Otherwise, run the <a>moving steps</a>
-    with <var>inclusiveDescendant</var> and null.
+    <p>Let <var>isSubtreeRoot</var> be whether <var>inclusiveDescendant</var> is <var>node</var>.
+
+    <p>Run the <a>moving steps</a> with <var>inclusiveDescendant</var>, <var>oldParent</var>, and
+    <var>isSubtreeRoot</var>.
 
     <p class="note">Because the <a>move</a> algorithm is a separate primitive from
     <a for=/>insert</a> and <a for=/>remove</a>, it does not invoke the traditional

Aside: I think we should remove "traditional" here.

> @@ -3108,9 +3109,10 @@ to do these things asynchronously, however.
 
   <ol>
    <li>
-    <p>If <var>inclusiveDescendant</var> is <var>node</var>, then run the <a>moving steps</a> with
-    <var>inclusiveDescendant</var> and <var>oldParent</var>. Otherwise, run the <a>moving steps</a>
-    with <var>inclusiveDescendant</var> and null.
+    <p>Let <var>isSubtreeRoot</var> be whether <var>inclusiveDescendant</var> is <var>node</var>.

It should also be its own step, not just its own paragraph.

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

Message ID: <whatwg/dom/pull/1450/review/3777581289@github.com>