Skip to content

Fix ResizeMirror missing mirror:destroy listener in attach()#674

Open
LeSingh1 wants to merge 1 commit into
Shopify:mainfrom
LeSingh1:fix-resizemirror-destroy-listener
Open

Fix ResizeMirror missing mirror:destroy listener in attach()#674
LeSingh1 wants to merge 1 commit into
Shopify:mainfrom
LeSingh1:fix-resizemirror-destroy-listener

Conversation

@LeSingh1
Copy link
Copy Markdown

The ResizeMirror plugin's attach() method registers mirror:created, drag:over, and drag:over:container listeners but does not register a mirror:destroy listener. The corresponding detach() method does call .off('mirror:destroy', this.onMirrorDestroy), so the asymmetry looks like an oversight from the original implementation.

The practical effect is that this.mirror is never reset to null between drags via the destroy event. Today the plugin still works because resize() early-returns when this.mirror.parentNode == null, but the plugin holds a reference to a detached mirror element until the next mirror:created fires, and the onMirrorDestroy method is effectively dead code without this binding.

The companion Snappable plugin already binds both mirror:created and mirror:destroy in its attach(). This change makes ResizeMirror consistent with that pattern and with its own detach().

A changeset is included. Existing tests pass (yarn test, yarn type-check, yarn lint all clean).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant