fix: panel buttons unresponsive after window resize#882
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughPanel components (ElementsPanel, PropertiesPanel, TestBenchPanel, TimingDiagramPanel) import and call an exported setupPanelListeners(selector) during onMounted; TestBenchPanel and TimingDiagramPanel also call an exported minimizePanel(selector) on mount. The simulator UX module (src/simulator/src/ux.js and v1 equivalent) adds and exports setupPanelListeners and minimizePanel, refactors setupPanels to invoke setupPanelListeners for multiple panels, and replaces direct DOM click triggers with namespaced event handlers and minimizePanel calls to centralize panel event wiring. Drag handler binding was made idempotent by namespacing the mousedown handler (mousedown.dragging). 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/simulator/src/ux.js (1)
380-408: Duplicate event handlers will accumulate on repeated calls.Each time
setupPanelListenersis called (after every Vue remount on window resize), new jQuery handlers are attached without removing the previous ones. This causes:
- Multiple click/dblclick handlers firing for a single interaction.
- The
minimizedclosure variable becomes stale — each call creates a newminimized, but old handlers still reference their own copy.Detach existing handlers before re-attaching:
🐛 Proposed fix to prevent duplicate handlers
function setupPanelListeners(panelSelector) { var headerSelector = `${panelSelector} .panel-header` var minimizeSelector = `${panelSelector} .minimize` var maximizeSelector = `${panelSelector} .maximize` var bodySelector = `${panelSelector} > .panel-body` dragging(headerSelector, panelSelector) - // Current Panel on Top - var minimized = false - $(headerSelector).on('dblclick', () => + // Remove existing handlers before re-attaching + $(headerSelector).off('dblclick') + $(minimizeSelector).off('click') + $(maximizeSelector).off('click') + + // Track minimized state via DOM visibility instead of closure + $(headerSelector).on('dblclick', () => { + var minimized = $(bodySelector).is(':hidden') minimized ? $(maximizeSelector).trigger('click') : $(minimizeSelector).trigger('click') - ) + }) // Minimize $(minimizeSelector).on('click', () => { $(bodySelector).hide() $(minimizeSelector).hide() $(maximizeSelector).show() - minimized = true }) // Maximize $(maximizeSelector).on('click', () => { $(bodySelector).show() $(minimizeSelector).show() $(maximizeSelector).hide() - minimized = false }) }
🤖 Fix all issues with AI agents
In `@v1/src/simulator/src/ux.js`:
- Around line 353-357: setupPanelListeners is currently binding jQuery handlers
multiple times (called from setupPanels and again on Vue mount), causing stacked
event handlers; update setupPanelListeners to use namespaced events and call
.off('event.namespace') (or .off('.namespace')) on the specific panel selectors
before attaching handlers so each bind replaces previous ones (use a unique
namespace like '.panelListeners'); ensure all .on(...) calls inside
setupPanelListeners are converted to namespaced events (e.g.,
'click.panelListeners') and removed via .off(...) on the same selectors prior to
re-binding.
🧹 Nitpick comments (1)
src/components/Panels/TimingDiagramPanel/TimingDiagramPanel.vue (1)
56-59: Add TypeScript declarations for global window helpers.These functions are assigned to
windowinsrc/simulator/src/ux.jsbut lack TypeScript declarations, causing potential type errors when called from.vueand.tsfiles. Add these declarations to your global typings file (e.g.,src/env.d.tsor a dedicatedglobal.d.ts):Global typing declaration
+declare global { + interface Window { + setupPanelListeners?: (panelSelector: string) => void + minimizePanel?: (panelSelector: string) => void + } +}
|
ready to review @tachyons @ThatDeparted2061 @Nihal4777 |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
v1/src/simulator/src/ux.js (1)
381-414:⚠️ Potential issue | 🟠 MajorAdd cleanup before jQuery event binding in
dragging().The
dragging()function at line 75 attaches amousedownlistener via$(DragEl).on('mousedown', ...)without removing previous listeners. WhensetupPanelListeners()runs on repeated remounts, these handlers will stack, causing the z-index logic to execute multiple times per click.Add
.off('mousedown')before.on()to prevent accumulation:Suggested fix
$(DragEl) .off('mousedown') .on('mousedown', () => { $(`.draggable-panel:not(${DragEl})`).css('z-index', '99') $(DragEl).css('z-index', '99') })The
interact.jscall handles repeated invocation safely (replaces config), but the jQuery binding does not.src/simulator/src/ux.js (1)
381-413:⚠️ Potential issue | 🟡 MinorAdd
.off('mousedown')cleanup before reattaching handler indragging().The jQuery event binding on line 75 lacks cleanup. While
interact.json line 51 likely replaces configuration safely, the$(DragEl).on('mousedown')handler will stack ifdragging()is called repeatedly. Use.off('mousedown')before.on(), or adopt a namespace like.off('mousedown.dragging').on('mousedown.dragging')to match the idempotent pattern already used elsewhere inux.js(lines 390, 398, 407).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@v1/src/simulator/src/ux.js`:
- Around line 386-388: The dragging() function is currently attaching a jQuery
mousedown handler with no cleanup, which allows duplicate handlers to accumulate
when setupPanelListeners is called multiple times; update dragging() to remove
existing handlers before attaching by using namespaced events (e.g.,
.off('mousedown.dragging') before .on('mousedown.dragging', ...)) or call
.off('mousedown') first, and ensure any interact.js instantiation inside
dragging() is made idempotent (destroy or reconfigure existing interactable
before re-initializing) so repeated calls do not stack listeners or duplicate
interact.js behavior.
|
implemented suggestions @Nihal4777 |
Nihal4777
dismissed
their stale review
coderabbit suggestion not implemented.
…tps://github.com/senutpal/cv-frontend-vue into fix/panel-buttons-unresponsive-after-resize-#874
|
implemented coderabbit suggestions @Nihal4777 |
|
Thanks @senutpal for the quick changes. ❤️ |
2 participants
Fixes #874
Describe the changes you have made in this PR -
Root Cause
jQuery event listeners lost when Vue re-renders panel components during responsive switching.
Solution
1. Expose Functions Globally
File:
ux.js2. Call in Vue Components
Files: ElementsPanel, TimingDiagramPanel, TestBenchPanel, PropertiesPanel
Screenshots of the UI changes (If any) -
Recording.2026-02-04.185656.mp4
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
The problem occurs because of a mismatch between Vue's component lifecycle and jQuery's event handling. When the window resizes across the 991px breakpoint, Vue conditionally renders/unmounts panel components using
v-if="!simulatorMobileStore.showMobileView". This destroys the DOM elements that jQuery listeners were attached to.So I re-attached listeners on component mount.window.setupPanelListeners(selector): Re-attaches jQuery click handlers for minimize/maximize buttons on a specific panelwindow.minimizePanel(selector): Triggers the minimize click event programmatically.onMounted(): Vue lifecycle hook that runs when component's DOM is ready.Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.
Summary by CodeRabbit