From 0254c9d0e900ce27e7e0236014c2f4414bd15d2b Mon Sep 17 00:00:00 2001 From: Will Miao Date: Thu, 27 Nov 2025 18:00:59 +0800 Subject: [PATCH] fix(showcase): tear down modal listeners --- static/js/components/shared/ModelModal.js | 8 ++- .../shared/showcase/ShowcaseView.js | 69 ++++++++++++++---- .../showcase.listenerMetrics.test.js | 72 +++++++++++++++++++ 3 files changed, 135 insertions(+), 14 deletions(-) create mode 100644 tests/frontend/components/showcase.listenerMetrics.test.js diff --git a/static/js/components/shared/ModelModal.js b/static/js/components/shared/ModelModal.js index 26a9e2ef..13e22fd6 100644 --- a/static/js/components/shared/ModelModal.js +++ b/static/js/components/shared/ModelModal.js @@ -454,6 +454,8 @@ export async function showModelModal(model, modelType) { `; + let showcaseCleanup; + const onCloseCallback = function() { // Clean up all handlers when modal closes for LoRA const modalElement = document.getElementById(modalId); @@ -461,6 +463,10 @@ export async function showModelModal(model, modelType) { modalElement.removeEventListener('click', modalElement._clickHandler); delete modalElement._clickHandler; } + if (showcaseCleanup) { + showcaseCleanup(); + showcaseCleanup = null; + } }; modalManager.showModal(modalId, content, null, onCloseCallback); @@ -475,7 +481,7 @@ export async function showModelModal(model, modelType) { currentVersionId: civitaiVersionId, }); setupEditableFields(modelWithFullData.file_path, modelType); - setupShowcaseScroll(modalId); + showcaseCleanup = setupShowcaseScroll(modalId); setupTabSwitching({ onTabChange: async (tab) => { if (tab === 'versions') { diff --git a/static/js/components/shared/showcase/ShowcaseView.js b/static/js/components/shared/showcase/ShowcaseView.js index 91a6fec0..b6eac4b2 100644 --- a/static/js/components/shared/showcase/ShowcaseView.js +++ b/static/js/components/shared/showcase/ShowcaseView.js @@ -15,6 +15,18 @@ import { import { generateMetadataPanel } from './MetadataPanel.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 * @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 */ export function setupShowcaseScroll(modalId) { - // Listen for wheel events - document.addEventListener('wheel', (event) => { + const wheelOptions = { passive: false }; + const wheelHandler = (event) => { const modalContent = document.querySelector(`#${modalId} .modal-content`); if (!modalContent) return; @@ -543,7 +555,9 @@ export function setupShowcaseScroll(modalId) { 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 const observer = new MutationObserver((mutations) => { @@ -558,12 +572,28 @@ export function setupShowcaseScroll(modalId) { }); 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 const modalContent = document.querySelector(`#${modalId} .modal-content`); if (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 */ function setupBackToTopButton(modalContent) { - // Remove any existing scroll listeners to avoid duplicates - modalContent.onscroll = null; - - // Add new scroll listener - modalContent.addEventListener('scroll', () => { + teardownBackToTopButton(modalContent); + + const handler = () => { const backToTopBtn = modalContent.querySelector('.back-to-top'); if (backToTopBtn) { if (modalContent.scrollTop > 300) { @@ -584,8 +612,23 @@ function setupBackToTopButton(modalContent) { backToTopBtn.classList.remove('visible'); } } - }); - - // Trigger a scroll event to check initial position - modalContent.dispatchEvent(new Event('scroll')); -} \ No newline at end of file + }; + + modalContent._backToTopScrollHandler = handler; + 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; + } +} diff --git a/tests/frontend/components/showcase.listenerMetrics.test.js b/tests/frontend/components/showcase.listenerMetrics.test.js new file mode 100644 index 00000000..87d71749 --- /dev/null +++ b/tests/frontend/components/showcase.listenerMetrics.test.js @@ -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 = ` +
+
+
+ + +
+
+
+ `; + }); + + 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); + }); +});