Draft
Conversation
The `build:admin` command is the one to run when modifying admin JavaScript source files, not `build:js` (which bundles vendored dependencies like sortablejs, shoelace, and tinymce).
The constructor read the src attribute and called start() which mutated the element's subtree. This breaks the Web Components spec requirement that constructors must not inspect attributes or touch children, and caused cloneNode(true) to produce a different subtree than the source. When SortableJS uses jQuery's clone(true) to build a drag ghost, the node count mismatch makes jQuery crash while copying events. The bootstrap now lives in connectedCallback, which fires after the element is inserted into the document and attributes are available.
Querying the document for the preload link in the constructor violates the Web Components spec guidance that constructors must not depend on external document state. The sprite URL is now resolved lazily via a getter, which makes the component safe to construct in contexts where the preload link is not yet present (e.g. during cloneNode inside a template fragment).
Web Components spec requires constructors to avoid inspecting attributes or children, because custom element constructors run during cloneNode(true). Reading innerHTML and attributes at construction time leaves clones in an inconsistent state and trips up libraries like jQuery's clone(true), which SortableJS uses for drag clones. Instead of capturing the original innerHTML and rewriting it, prepend the icon via insertAdjacentHTML inside connectedCallback. A guard skips the insertion when an icon is already present, keeping the operation idempotent across reconnections without needing to store the original content.
Web Components spec requires constructors to avoid mutating the DOM, because custom element constructors run during cloneNode(true). Writing innerHTML in the constructor leaves clones in an inconsistent state and can trip up libraries like jQuery's clone(true), which SortableJS uses for drag clones. Prepend the icon via insertAdjacentHTML inside connectedCallback with a guard that makes the insertion idempotent. Initialize the Clipboard instance on connect so it is paired with the disconnectedCallback teardown and survives reconnection.
Web Components spec requires constructors to avoid mutating the DOM, because custom element constructors run during cloneNode(true). Writing innerHTML and setting attributes in the constructor leaves clones in an inconsistent state and can trip up libraries like jQuery's clone(true), which SortableJS uses for drag clones. Move the setup into connectedCallback. The operations are either idempotent (classList.add, setAttribute, addEventListener with the same listener reference) or always overwrite a known state (innerHTML is always the icon since this button's content is never rendered server-side), so no explicit guard is needed.
Web Components spec requires constructors to avoid inspecting attributes or mutating the DOM, because custom element constructors run during cloneNode(true). Reading the linked state from classList, setting attributes, and writing innerHTML at construction time leaves clones in an inconsistent state and can trip up libraries like jQuery's clone(true), which SortableJS uses for drag clones. Move the setup into connectedCallback. The operations are idempotent on reconnection (classList.add, setAttribute, addEventListener with the same listener reference) and innerHTML always resolves to the icon since this button's content is never rendered server-side.
Web Components spec requires constructors to avoid inspecting children, because custom element constructors run during cloneNode(true). Running querySelector and attaching listeners to children at construction time can trip up libraries like jQuery's clone(true), which SortableJS uses for drag clones, when the subtree has not yet been populated on the clone. Move the child lookups and the delete link listener registration into connectedCallback, where the element's subtree is guaranteed to be in its final shape.
Web Components spec requires constructors to avoid inspecting children, because custom element constructors run during cloneNode(true). Running querySelector, reading dataset values, and attaching listeners to children at construction time can trip up libraries like jQuery's clone(true), which SortableJS uses for drag clones, when the subtree has not yet been populated on the clone. Move the setup into connectedCallback. Convert removeImage and mutationCallback to arrow function class fields so each yields a stable reference, which keeps addEventListener's same-listener deduplication working if the element is ever reconnected, and avoids the need for an explicit init guard.
Web Components spec requires constructors to avoid inspecting children or attaching listeners, because custom element constructors run during cloneNode(true). Attaching listeners to window, document.body, and child elements at construction time can leave clones in an inconsistent state and also leaks window/document listeners when the element is garbage collected. Move all listener registration into connectedCallback and add the matching removeEventListener calls in disconnectedCallback. Convert the inline arrow handlers to stable arrow function class fields so the same reference can be used for both add and remove.
Drop the AlchemyHTMLElement base class in favor of a plain HTMLElement. The base class reads innerHTML in its constructor, which violates the Web Components spec and leaves cloned elements in an inconsistent state when jQuery's clone(true) walks the subtree. Spinner does not need any of the base class's abstractions: its size and color attributes are set once at element creation (via the JS Spinner wrapper or inline HTML) and never mutated afterwards, so rendering only needs to run on connection.
Drop the AlchemyHTMLElement base class in favor of a plain HTMLElement. The base class reads innerHTML in its constructor, which violates the Web Components spec and leaves cloned elements in an inconsistent state when jQuery's clone(true) walks the subtree. Overlay only needs to render once on connection. Guard the text interpolation with a nullish coalescing fallback so a missing text attribute no longer renders the literal string "null".
Drop the AlchemyHTMLElement base class in favor of a plain HTMLElement. The base class reads innerHTML in its constructor, which violates the Web Components spec and leaves cloned elements in an inconsistent state when jQuery's clone(true) walks the subtree. Replace the maxChars static property with a simple getter that falls back to 60 when the max-chars attribute is not set. The downstream comparison with charLength still works because JavaScript's relational operators coerce numeric strings to numbers.
Drop the AlchemyHTMLElement base class in favor of a plain HTMLElement. The base class reads innerHTML in its constructor, which violates the Web Components spec and leaves cloned elements in an inconsistent state when jQuery's clone(true) walks the subtree. Bail out of connectedCallback after the async locale import if the element was disconnected in the meantime. Without this guard, a fast drag-drop would leak a flatpickr calendar onto a detached input because the drop ghost gets disconnected before the import resolves. Also guard disconnectedCallback with optional chaining so a teardown before initialization does not throw.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3841 +/- ##
=======================================
Coverage 98.06% 98.06%
=======================================
Files 322 322
Lines 8530 8530
=======================================
Hits 8365 8365
Misses 165 165 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this pull request for?
Describe you pull request here...
Closes # (Remove if not related to any issue)
Notable changes (remove if none)
Explain any changes (maybe breaking?) that have been made.
Screenshots
Remove if no visual changes have been made.
Checklist