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

Editorial review: Add moveBefore() docs #39002

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

chrisdavidmills
Copy link
Contributor

Description

Chrome 133 supports the moveBefore() method; see https://chromestatus.com/feature/5135990159835136.

This PR documents the new method on Document, DocumentFragment, and Element.

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested a review from a team as a code owner April 6, 2025 13:59
@chrisdavidmills chrisdavidmills requested review from sideshowbarker and removed request for a team April 6, 2025 13:59
@github-actions github-actions bot added Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed labels Apr 6, 2025
Copy link
Contributor

github-actions bot commented Apr 6, 2025

Preview URLs (7 pages)
Flaws (14)

Note! 4 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/Document
Title: Document
Flaw count: 10

  • macros:
    • Macro produces link /en-US/docs/Web/SVG/Element/svg which is a redirect
    • Can't resolve /en-US/docs/Web/API/Document/xmlStandalone
    • Can't resolve /en-US/docs/Web/API/Document/captureEvents
    • Can't resolve /en-US/docs/Web/API/Document/getBoxQuads
    • Can't resolve /en-US/docs/Web/API/Document/releaseEvents
    • and 5 more flaws omitted

URL: /en-US/docs/Web/API/Element
Title: Element
Flaw count: 1

  • macros:
    • Can't resolve /en-US/docs/Web/API/Element/getBoxQuads

URL: /en-US/docs/Web/API/Web_components/Using_custom_elements
Title: Using custom elements
Flaw count: 3

  • macros:
    • Macro produces link /en-US/docs/Web/Web_Components/Using_custom_elements which is a redirect
    • Macro produces link /en-US/docs/Web/Web_Components/Using_shadow_DOM which is a redirect
    • Macro produces link /en-US/docs/Web/Web_Components/Using_templates_and_slots which is a redirect

(comment last updated: 2025-04-09 13:47:52)

@chrisdavidmills chrisdavidmills marked this pull request as draft April 7, 2025 07:59
@github-actions github-actions bot added size/l [PR only] 501-1000 LoC changed and removed size/m [PR only] 51-500 LoC changed labels Apr 7, 2025
@chrisdavidmills chrisdavidmills marked this pull request as ready for review April 7, 2025 09:42
@chrisdavidmills chrisdavidmills changed the title Add moveBefore() docs Technical review: Add moveBefore() docs Apr 7, 2025
@chrisdavidmills chrisdavidmills removed the request for review from sideshowbarker April 7, 2025 09:44
@noamr
Copy link
Contributor

noamr commented Apr 7, 2025

Thanks! A few technical comments:

  • What is preserved for <dialog> is its modal state. See https://github.com/web-platform-tests/wpt/blob/e3e6eee31d98d5b5dea1a3159febe139280c10ad/dom/nodes/moveBefore/modal-dialog.html

  • Selection is a bit of an odd one; It only preserves selection in certain situations - namely when the anchor node of the selection is the target of the moveBefore.

  • This is missing a section about custom elements. By default, a custom elements that get moved using moveBefore, receive their disconnectedCallback and connectedCallback invoked. To have custom support for state-preserving move, an author can add a connectedMoveCallback that would be called instead of connected/disconnected. It can be empty to avoid the disconnection/connection side-effects, or contain logic that responds to a move.

  • We should say something about the constraints: moveBefore only works within the same document, and it cannot work as a way to connect or disconnect an element. Moving a disconnected element to a connected parent or vice versa, would result in an exception. Users of this API that want to be resilient to this need to know in advance whether they're acting across disconnected/connected state, across documents, and use insertBefore instead in these cases (or try/catch the moveBefore).

  • As far as mutation observers go, there is no particular change, but it might be worthwhile to note that the moved element would appear as removed/re-inserted in mutation observers.

@chrisdavidmills
Copy link
Contributor Author

Thanks! A few technical comments:

* What is preserved for `<dialog>` is its modal state. See https://github.com/web-platform-tests/wpt/blob/e3e6eee31d98d5b5dea1a3159febe139280c10ad/dom/nodes/moveBefore/modal-dialog.html

Cool, thanks for the tip; I've updated the list of preserved state things to account for this.

Note that, in light of your comments, I've moved this content to a new "Description" section on the page and added the other bits you wanted me to include there.

* Selection is a bit of an odd one; It only preserves selection in certain situations - namely when the anchor node of the selection is the target of the `moveBefore`.

I did a bit of testing on this but couldn't get the selection to preserve. I've just removed the bullet about preserving selection, as this doesn't seem to me to be a case that will come up often or behavior that can be particularly relied upon. What do you think?

* This is missing a section about custom elements. By default, a custom elements that get moved using `moveBefore`, receive their `disconnectedCallback` and `connectedCallback` invoked. To have custom support for state-preserving move, an author can add a `connectedMoveCallback` that would be called instead of connected/disconnected. It can be empty to avoid the disconnection/connection side-effects, or contain logic that responds to a move.

I've added a section to cover this inside the "Description" section, and added some info about connectedMoveCallback() to the custom elements page. I didn't add it to the Document.moveBefore() page as it didn't seem so relevant there, just DocumentFragment... and Element...

* We should say something about the constraints: `moveBefore` only works within the same document, and it cannot work as a way to connect or disconnect an element. Moving a disconnected element to a connected parent or vice versa, would result in an exception. Users of this API that want to be resilient to this need to know in advance whether they're acting across disconnected/connected state, across documents, and use `insertBefore` instead in these cases (or try/catch the `moveBefore`).

I've added a section about constraints to the "Description" section, and also tried to move sure the relevant exceptions are represented in the "Exceptions" section.

* As far as mutation observers go, there is no particular change, but it might be worthwhile to note that the moved element would appear as removed/re-inserted in mutation observers.

Good call; I've added a note about this.

@chrisdavidmills
Copy link
Contributor Author

@noamr ^


- [Animation](/en-US/docs/Web/CSS/CSS_animations) and [transition](/en-US/docs/Web/CSS/CSS_transitions) state.
- {{htmlelement("iframe")}} loading state.
- Interactive node {{cssxref(":focus")}} and {{cssxref(":active")}} states.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps reword to "interactivity"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to

  • Interactivity states (for example, {{cssxref(":focus")}} and {{cssxref(":active")}}).

> [!NOTE]
> When observing changes to the DOM using a {{domxref("MutationObserver")}}, nodes moved with `moveBefore()` will be recorded with a [removed node](/en-US/docs/Web/API/MutationRecord/removedNodes) and an [added node](/en-US/docs/Web/API/MutationRecord/addedNodes).

### `moveBefore()` constaints
Copy link
Contributor

Choose a reason for hiding this comment

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

constraints

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops! Fixed.


### Moving custom elements

When moving a [custom element](/en-US/docs/Web/API/Web_components/Using_custom_elements), care needs to be taken to handle the `connectedCallback()` and `disconnectedCallback()` [lifecycle callbacks](/en-US/docs/Web/API/Web_components/Using_custom_elements#custom_element_lifecycle_callbacks) appropriately, if any are defined in the custom element's constructor.
Copy link
Contributor

Choose a reason for hiding this comment

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

"care needs to be taken" is not the right framing here.
By default, custom elements don't support state-preserving move, and their disconnected and connected callbacks are fired.

Authors of web components can opt in to state-preserving move behavior, by implementing the connectedMoveCallback, which would be called instead of the disconnected+connected default.

Note: this default behavior is introduced in order to maintain backwards compatibility with existing custom elements who might rely on being disconnected and reconnected when moved around the DOM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I get this now; thanks for the explanation. I've reworded the section as follows:


Moving custom elements

Each time a custom element is moved via moveBefore() (or similar methods such as {{domxref("Node.insertBefore()")}}), the disconnectedCallback() and connectedCallback() lifecycle callbacks are fired.

Consider this minimal example:

class MyCustomElement extends HTMLElement {
  constructor() {
    super();
  }

  connectedCallback() {
    console.log("Custom element added to page.");
  }

  disconnectedCallback() {
    console.log("Custom element removed from page.");
  }
}

customElements.define("my-custom-element", MyCustomElement);

In this case, "Custom element removed from page." and "Custom element added to page." are logged to the console with each move. This might be your intended behavior. However, if you use the callbacks to run initialization and cleanup code, it might cause problems with the state of the moved element.

Custom elements can be opted in to state-preserving moves via the connectedMoveCallback() lifecycle callback. If defined in the constructor, this will run instead of connectedCallback() and disconnectedCallback() when an element instance is moved via moveBefore(). You could add an empty connectedMoveCallback() to stop the other two callbacks running, or include some custom logic to handle the move:

connectedMoveCallback() {
  console.log("Custom element moved with moveBefore()");
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect thanks!

@@ -57,6 +57,7 @@ Custom element lifecycle callbacks include:

- `connectedCallback()`: called each time the element is added to the document. The specification recommends that, as far as possible, developers should implement custom element setup in this callback rather than the constructor.
- `disconnectedCallback()`: called each time the element is removed from the document.
- `connectedMoveCallback()`: called _instead_ of `connectedCallback()` and `disconnectedCallback()` each time the element is moved to a different place in the DOM via {{domxref("Element.moveBefore()")}}. This can be used to overcome functionality problems encountered as a result of initialization/cleanup code being run multiple times when the element is not actually being added to or removed from the DOM. See [Moving custom elements](/en-US/docs/Web/API/Element/moveBefore#moving_custom_elements) for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

  • When defined, called _instead etc..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@chrisdavidmills chrisdavidmills changed the title Technical review: Add moveBefore() docs Editorial review: Add moveBefore() docs Apr 9, 2025
@chrisdavidmills
Copy link
Contributor Author

Thanks a lot for the review, @noamr. I think this is ready for editorial review now, so I will move it on to that stage, but feel free to chime in if you spot anything else.

@wbamberg wbamberg self-requested a review April 9, 2025 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/l [PR only] 501-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants