Skip to content

Refactor closeable directives #2661

Description

@gerteck

Please confirm that you have searched existing issues in the repo

Yes, I have searched the existing issues

Any related issues?

No response

Tell us about your environment

Win 11

MarkBind version

5.6.0

Describe the bug and the steps to reproduce it

The v-closeable directive currently attaches mouseover and mouseout event listeners directly to the DOM element without any cleanup logic. This leads to memory leaks and duplicate event listener bindings when the directive element is toggled with v-if or when the component is unmounted and remounted. (Edge case if the directive is used in other vue components).

module.exports = {
bind(el) {
function onClose() {
el.dataset.isShown = 'false';
showLabel.style.display = '';
closeButton.style.display = 'none';
content.style.display = 'none';
}
function onShow() {
el.dataset.isShown = 'true';
showLabel.style.display = 'none';
content.style.display = '';
}
function onMouseOver() {
if (el.dataset.isShown === 'false') {
return;
}
closeButton.style.display = '';
}
function onMouseOut() {
if (el.dataset.isShown === 'false') {
return;
}
closeButton.style.display = 'none';
}
function createCloseButton() {
const closeButton = document.createElement('span');
closeButton.classList.add('closeable-button', 'label', 'label-default', 'hidden-print');
closeButton.style.cssText += 'display: none; position: absolute; top: 0; '
+ 'left: 0; cursor: pointer;background: #d9534f';
closeButton.innerHTML = '<span class="glyphicon glyphicon-remove" aria-hidden="true"></span>';
closeButton.addEventListener('click', onClose);
return closeButton;
}
function createShowLabel(message) {
const showLabel = document.createElement('a');
showLabel.classList.add('closeable-show', 'hidden-print');
showLabel.style.cssText += 'display: none; cursor: pointer;text-decoration: underline';
showLabel.innerHTML = message;
showLabel.addEventListener('click', onShow);
return showLabel;
}
el.dataset.isShown = 'true';
el.style.position = 'relative';
const content = document.createElement('div');
content.classList.add('content');
Array.from(el.children).forEach(child => content.append(child));
el.replaceChildren();
el.append(content);
el.classList.add('closeable-wrapper');
const closeButton = createCloseButton();
el.append(closeButton);
const message = el.getAttribute('alt') || 'Expand Content';
const showLabel = createShowLabel(message);
el.append(showLabel);
el.addEventListener('mouseover', onMouseOver);
el.addEventListener('mouseout', onMouseOut);
}
};

Suggested Fix

  • Store the event listener functions (e.g., in el._closeable) when adding them
  • On unmounting, we remove the event listeners
el.removeEventListener('mouseover', el._closeable.onMouseOver);
el.removeEventListener('mouseout', el._closeable.onMouseOut);
delete el._closeable;

Expected behavior

The directive should clean up event listeners on unmount to avoid memory leaks and ensure no duplicate bindings occur.

Anything else?

No response

Metadata

Metadata

Assignees

Type

No type

Projects

Status
Discussion

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions