Re: [whatwg/dom] Introduce `moveBefore()` state-preserving atomic move API (PR #1307)

@annevk commented on this pull request.

I think the mutation record needs some more design work. I would expect it to capture the information of a remove and an insert at the same time. Perhaps it needs to be a new object, though we could further overload the existing `MutationRecord` as well I guess. At least I think you need:

- old target
- target
- moved node (I'm not sure you can ever move multiple at this point, but maybe we should allow for it in the mutation record design?)
- old previous sibling
- old next sibling
- previous sibling
- next sibling

Would be good to know what @smaug---- thinks and maybe @ajklein even wants to chime in.

> @@ -2729,6 +2729,9 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
  <li><p>Let <var>nodes</var> be <var>node</var>'s <a for=tree>children</a>, if <var>node</var> is a
  {{DocumentFragment}} <a for=/>node</a>; otherwise « <var>node</var> ».
 
+ <li><p>Let <var>state-preserving atomic move in progress</var> be <var>parent</var>'s

camelCase for variables please.

> @@ -2729,6 +2729,9 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
  <li><p>Let <var>nodes</var> be <var>node</var>'s <a for=tree>children</a>, if <var>node</var> is a
  {{DocumentFragment}} <a for=/>node</a>; otherwise « <var>node</var> ».
 
+ <li><p>Let <var>state-preserving atomic move in progress</var> be <var>parent</var>'s
+ <a for=Node>node document</a> <span>state-preserving atomic move in progress</span>.

node document's? Also `<a>` instead of `<span>`.

> + <li><p>Let <var>return node</var> be the result of <a>pre-inserting</a> <var>node</var> into
+ <a>this</a> before <var>child</var>.</p></li>

I think it would be cleaner if we introduced "move" instead of overloading "insert", though I'm willing to be convinced. At least I always viewed this as introducing "move" as the third primitive following "insert" and "remove".

> @@ -4986,6 +5033,27 @@ within <a>this</a>.
 <p>The <dfn method for=Node><code>removeChild(<var>child</var>)</code></dfn> method steps are to
 return the result of <a>pre-removing</a> <var>child</var> from <a>this</a>.
 
+<p>The <dfn method for=Node><code>moveBefore(<var>node</var>, <var>child</var>)</code></dfn>
+method steps are:
+
+<ol>
+ <li><p>Let <var>perform state-preserving atomic move</var> be true if <a>this</a> is
+ <a>connected</a>, <var>node</var> is <a>connected</a>, <a>this</a>'s <a for=Node>node
+ document</a> is <a>fully active</a>, and <a>this</a>'s <a for=/>root</a> is the same as
+ <var>node</var>'s <a for=/>root</a>.</p></li>.

Hmm, I think this method should throw if the conditions aren't met. That seems much better for extensibility than implicit fallback to insert.

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

Message ID: <whatwg/dom/pull/1307/review/2263388887@github.com>