mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-03-21 21:22:11 -03:00
Merge pull request #703 from willmiao/fix/showcase-listener-leaks
fix(showcase): tear down modal listeners
This commit is contained in:
@@ -454,6 +454,8 @@ export async function showModelModal(model, modelType) {
|
|||||||
</div>
|
</div>
|
||||||
`;
|
`;
|
||||||
|
|
||||||
|
let showcaseCleanup;
|
||||||
|
|
||||||
const onCloseCallback = function() {
|
const onCloseCallback = function() {
|
||||||
// Clean up all handlers when modal closes for LoRA
|
// Clean up all handlers when modal closes for LoRA
|
||||||
const modalElement = document.getElementById(modalId);
|
const modalElement = document.getElementById(modalId);
|
||||||
@@ -461,6 +463,10 @@ export async function showModelModal(model, modelType) {
|
|||||||
modalElement.removeEventListener('click', modalElement._clickHandler);
|
modalElement.removeEventListener('click', modalElement._clickHandler);
|
||||||
delete modalElement._clickHandler;
|
delete modalElement._clickHandler;
|
||||||
}
|
}
|
||||||
|
if (showcaseCleanup) {
|
||||||
|
showcaseCleanup();
|
||||||
|
showcaseCleanup = null;
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
modalManager.showModal(modalId, content, null, onCloseCallback);
|
modalManager.showModal(modalId, content, null, onCloseCallback);
|
||||||
@@ -475,7 +481,7 @@ export async function showModelModal(model, modelType) {
|
|||||||
currentVersionId: civitaiVersionId,
|
currentVersionId: civitaiVersionId,
|
||||||
});
|
});
|
||||||
setupEditableFields(modelWithFullData.file_path, modelType);
|
setupEditableFields(modelWithFullData.file_path, modelType);
|
||||||
setupShowcaseScroll(modalId);
|
showcaseCleanup = setupShowcaseScroll(modalId);
|
||||||
setupTabSwitching({
|
setupTabSwitching({
|
||||||
onTabChange: async (tab) => {
|
onTabChange: async (tab) => {
|
||||||
if (tab === 'versions') {
|
if (tab === 'versions') {
|
||||||
|
|||||||
@@ -15,6 +15,18 @@ import {
|
|||||||
import { generateMetadataPanel } from './MetadataPanel.js';
|
import { generateMetadataPanel } from './MetadataPanel.js';
|
||||||
import { generateImageWrapper, generateVideoWrapper } from './MediaRenderers.js';
|
import { generateImageWrapper, generateVideoWrapper } from './MediaRenderers.js';
|
||||||
|
|
||||||
|
export const showcaseListenerMetrics = {
|
||||||
|
wheelListeners: 0,
|
||||||
|
mutationObservers: 0,
|
||||||
|
backToTopHandlers: 0,
|
||||||
|
};
|
||||||
|
|
||||||
|
export function resetShowcaseListenerMetrics() {
|
||||||
|
showcaseListenerMetrics.wheelListeners = 0;
|
||||||
|
showcaseListenerMetrics.mutationObservers = 0;
|
||||||
|
showcaseListenerMetrics.backToTopHandlers = 0;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Load example images asynchronously
|
* Load example images asynchronously
|
||||||
* @param {Array} images - Array of image objects (both regular and custom)
|
* @param {Array} images - Array of image objects (both regular and custom)
|
||||||
@@ -524,8 +536,8 @@ export function scrollToTop(button) {
|
|||||||
* @param {string} modalId - ID of the modal element
|
* @param {string} modalId - ID of the modal element
|
||||||
*/
|
*/
|
||||||
export function setupShowcaseScroll(modalId) {
|
export function setupShowcaseScroll(modalId) {
|
||||||
// Listen for wheel events
|
const wheelOptions = { passive: false };
|
||||||
document.addEventListener('wheel', (event) => {
|
const wheelHandler = (event) => {
|
||||||
const modalContent = document.querySelector(`#${modalId} .modal-content`);
|
const modalContent = document.querySelector(`#${modalId} .modal-content`);
|
||||||
if (!modalContent) return;
|
if (!modalContent) return;
|
||||||
|
|
||||||
@@ -543,7 +555,9 @@ export function setupShowcaseScroll(modalId) {
|
|||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}, { passive: false });
|
};
|
||||||
|
document.addEventListener('wheel', wheelHandler, wheelOptions);
|
||||||
|
showcaseListenerMetrics.wheelListeners += 1;
|
||||||
|
|
||||||
// Use MutationObserver to set up back-to-top button when modal content is added
|
// Use MutationObserver to set up back-to-top button when modal content is added
|
||||||
const observer = new MutationObserver((mutations) => {
|
const observer = new MutationObserver((mutations) => {
|
||||||
@@ -558,12 +572,28 @@ export function setupShowcaseScroll(modalId) {
|
|||||||
});
|
});
|
||||||
|
|
||||||
observer.observe(document.body, { childList: true, subtree: true });
|
observer.observe(document.body, { childList: true, subtree: true });
|
||||||
|
showcaseListenerMetrics.mutationObservers += 1;
|
||||||
|
|
||||||
// Try to set up the button immediately in case the modal is already open
|
// Try to set up the button immediately in case the modal is already open
|
||||||
const modalContent = document.querySelector(`#${modalId} .modal-content`);
|
const modalContent = document.querySelector(`#${modalId} .modal-content`);
|
||||||
if (modalContent) {
|
if (modalContent) {
|
||||||
setupBackToTopButton(modalContent);
|
setupBackToTopButton(modalContent);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let cleanedUp = false;
|
||||||
|
|
||||||
|
return () => {
|
||||||
|
if (cleanedUp) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
cleanedUp = true;
|
||||||
|
document.removeEventListener('wheel', wheelHandler, wheelOptions);
|
||||||
|
showcaseListenerMetrics.wheelListeners -= 1;
|
||||||
|
observer.disconnect();
|
||||||
|
showcaseListenerMetrics.mutationObservers -= 1;
|
||||||
|
const modalContent = document.querySelector(`#${modalId} .modal-content`);
|
||||||
|
teardownBackToTopButton(modalContent);
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -571,11 +601,9 @@ export function setupShowcaseScroll(modalId) {
|
|||||||
* @param {HTMLElement} modalContent - Modal content element
|
* @param {HTMLElement} modalContent - Modal content element
|
||||||
*/
|
*/
|
||||||
function setupBackToTopButton(modalContent) {
|
function setupBackToTopButton(modalContent) {
|
||||||
// Remove any existing scroll listeners to avoid duplicates
|
teardownBackToTopButton(modalContent);
|
||||||
modalContent.onscroll = null;
|
|
||||||
|
const handler = () => {
|
||||||
// Add new scroll listener
|
|
||||||
modalContent.addEventListener('scroll', () => {
|
|
||||||
const backToTopBtn = modalContent.querySelector('.back-to-top');
|
const backToTopBtn = modalContent.querySelector('.back-to-top');
|
||||||
if (backToTopBtn) {
|
if (backToTopBtn) {
|
||||||
if (modalContent.scrollTop > 300) {
|
if (modalContent.scrollTop > 300) {
|
||||||
@@ -584,8 +612,23 @@ function setupBackToTopButton(modalContent) {
|
|||||||
backToTopBtn.classList.remove('visible');
|
backToTopBtn.classList.remove('visible');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
};
|
||||||
|
|
||||||
// Trigger a scroll event to check initial position
|
modalContent._backToTopScrollHandler = handler;
|
||||||
modalContent.dispatchEvent(new Event('scroll'));
|
modalContent.addEventListener('scroll', handler);
|
||||||
}
|
showcaseListenerMetrics.backToTopHandlers += 1;
|
||||||
|
handler();
|
||||||
|
}
|
||||||
|
|
||||||
|
function teardownBackToTopButton(modalContent) {
|
||||||
|
if (!modalContent) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const existingHandler = modalContent._backToTopScrollHandler;
|
||||||
|
if (existingHandler) {
|
||||||
|
modalContent.removeEventListener('scroll', existingHandler);
|
||||||
|
delete modalContent._backToTopScrollHandler;
|
||||||
|
showcaseListenerMetrics.backToTopHandlers -= 1;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
72
tests/frontend/components/showcase.listenerMetrics.test.js
Normal file
72
tests/frontend/components/showcase.listenerMetrics.test.js
Normal file
@@ -0,0 +1,72 @@
|
|||||||
|
import { describe, it, beforeEach, afterEach, expect } from 'vitest';
|
||||||
|
|
||||||
|
const { SHOWCASE_MODULE } = vi.hoisted(() => ({
|
||||||
|
SHOWCASE_MODULE: new URL('../../../static/js/components/shared/showcase/ShowcaseView.js', import.meta.url).pathname,
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe('Showcase listener metrics', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
document.body.innerHTML = `
|
||||||
|
<div id="modelModal">
|
||||||
|
<div class="modal-content">
|
||||||
|
<div class="showcase-section">
|
||||||
|
<div class="carousel collapsed">
|
||||||
|
<div class="scroll-indicator"></div>
|
||||||
|
</div>
|
||||||
|
<button class="back-to-top"></button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
`;
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
document.body.innerHTML = '';
|
||||||
|
});
|
||||||
|
|
||||||
|
it('tracks wheel/mutation/back-to-top listeners and resets after cleanup', async () => {
|
||||||
|
const {
|
||||||
|
setupShowcaseScroll,
|
||||||
|
resetShowcaseListenerMetrics,
|
||||||
|
showcaseListenerMetrics,
|
||||||
|
} = await import(SHOWCASE_MODULE);
|
||||||
|
|
||||||
|
resetShowcaseListenerMetrics();
|
||||||
|
|
||||||
|
expect(showcaseListenerMetrics.wheelListeners).toBe(0);
|
||||||
|
expect(showcaseListenerMetrics.mutationObservers).toBe(0);
|
||||||
|
expect(showcaseListenerMetrics.backToTopHandlers).toBe(0);
|
||||||
|
|
||||||
|
const cleanup = setupShowcaseScroll('modelModal');
|
||||||
|
|
||||||
|
expect(showcaseListenerMetrics.wheelListeners).toBe(1);
|
||||||
|
expect(showcaseListenerMetrics.mutationObservers).toBe(1);
|
||||||
|
expect(showcaseListenerMetrics.backToTopHandlers).toBe(1);
|
||||||
|
|
||||||
|
cleanup();
|
||||||
|
|
||||||
|
expect(showcaseListenerMetrics.wheelListeners).toBe(0);
|
||||||
|
expect(showcaseListenerMetrics.mutationObservers).toBe(0);
|
||||||
|
expect(showcaseListenerMetrics.backToTopHandlers).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('remains stable after repeated setup/cleanup cycles', async () => {
|
||||||
|
const {
|
||||||
|
setupShowcaseScroll,
|
||||||
|
resetShowcaseListenerMetrics,
|
||||||
|
showcaseListenerMetrics,
|
||||||
|
} = await import(SHOWCASE_MODULE);
|
||||||
|
|
||||||
|
resetShowcaseListenerMetrics();
|
||||||
|
|
||||||
|
const cleanupA = setupShowcaseScroll('modelModal');
|
||||||
|
cleanupA();
|
||||||
|
|
||||||
|
const cleanupB = setupShowcaseScroll('modelModal');
|
||||||
|
cleanupB();
|
||||||
|
|
||||||
|
expect(showcaseListenerMetrics.wheelListeners).toBe(0);
|
||||||
|
expect(showcaseListenerMetrics.mutationObservers).toBe(0);
|
||||||
|
expect(showcaseListenerMetrics.backToTopHandlers).toBe(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user