fix: dispose diff comment zone mouse listener (#4065)
Dispose the diff-comment inline zone mousedown stopper when zones are removed or the editor is torn down.
This commit is contained in:
parent
9fd58094c8
commit
b733368f0d
|
|
@ -0,0 +1,21 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { installDiffCommentZoneMouseDownStopper } from './diff-comment-zone-mouse-events'
|
||||
|
||||
describe('installDiffCommentZoneMouseDownStopper', () => {
|
||||
it('removes the mousedown listener on dispose', () => {
|
||||
const target = new EventTarget()
|
||||
const dispose = installDiffCommentZoneMouseDownStopper(target)
|
||||
|
||||
const first = new Event('mousedown')
|
||||
const firstStop = vi.spyOn(first, 'stopPropagation')
|
||||
target.dispatchEvent(first)
|
||||
expect(firstStop).toHaveBeenCalledOnce()
|
||||
|
||||
dispose()
|
||||
|
||||
const second = new Event('mousedown')
|
||||
const secondStop = vi.spyOn(second, 'stopPropagation')
|
||||
target.dispatchEvent(second)
|
||||
expect(secondStop).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
export function installDiffCommentZoneMouseDownStopper(target: EventTarget): () => void {
|
||||
const stopMouseDownPropagation = (ev: Event): void => ev.stopPropagation()
|
||||
target.addEventListener('mousedown', stopMouseDownPropagation)
|
||||
return () => target.removeEventListener('mousedown', stopMouseDownPropagation)
|
||||
}
|
||||
|
|
@ -12,6 +12,7 @@ import { useAppStore } from '@/store'
|
|||
import { TooltipProvider } from '@/components/ui/tooltip'
|
||||
import { DiffCommentCard } from './DiffCommentCard'
|
||||
import { getDiffCommentPopoverTop } from './diff-comment-popover-position'
|
||||
import { installDiffCommentZoneMouseDownStopper } from './diff-comment-zone-mouse-events'
|
||||
import { NotesSendMenu, type NotesSendMenuScope } from '../editor/NotesSendMenu'
|
||||
|
||||
// Why: Monaco glyph-margin *decorations* don't expose click events in a way
|
||||
|
|
@ -62,6 +63,7 @@ type ZoneEntry = {
|
|||
// mutating the delegate is the supported way to grow a zone in place.
|
||||
delegate: monacoEditor.IViewZone
|
||||
root: Root
|
||||
disposeMouseDownStopper: () => void
|
||||
lastRenderSignature: string
|
||||
// Why: Monaco invokes IViewZone.onDomNodeTop on every render once the zone
|
||||
// is in the layout. The first invocation is our deterministic "this zone is
|
||||
|
|
@ -399,7 +401,10 @@ export function useDiffCommentDecorator({
|
|||
// delay. Clear `zones` synchronously so a subsequent editor mount sees
|
||||
// empty bookkeeping immediately. This matches the deferred unmount in
|
||||
// the diff-pass effect below.
|
||||
const rootsToUnmount = Array.from(zones.values(), (z) => z.root)
|
||||
const rootsToUnmount = Array.from(zones.values(), (z) => {
|
||||
z.disposeMouseDownStopper()
|
||||
return z.root
|
||||
})
|
||||
zones.clear()
|
||||
if (rootsToUnmount.length > 0) {
|
||||
queueMicrotask(() => {
|
||||
|
|
@ -559,6 +564,7 @@ export function useDiffCommentDecorator({
|
|||
for (const [commentId, entry] of zones) {
|
||||
if (!relevantMap.has(commentId)) {
|
||||
accessor.removeZone(entry.zoneId)
|
||||
entry.disposeMouseDownStopper()
|
||||
rootsToUnmount.push(entry.root)
|
||||
zones.delete(commentId)
|
||||
// Why: if the user requested a scroll-to-note on a comment that
|
||||
|
|
@ -581,7 +587,7 @@ export function useDiffCommentDecorator({
|
|||
// steal focus (or start a selection drag) when the user interacts
|
||||
// with anything inside the card. Delete still fires because click is
|
||||
// attached directly on the button.
|
||||
dom.addEventListener('mousedown', (ev) => ev.stopPropagation())
|
||||
const disposeMouseDownStopper = installDiffCommentZoneMouseDownStopper(dom)
|
||||
|
||||
const root = createRoot(dom)
|
||||
|
||||
|
|
@ -631,6 +637,7 @@ export function useDiffCommentDecorator({
|
|||
domNode: dom,
|
||||
delegate,
|
||||
root,
|
||||
disposeMouseDownStopper,
|
||||
lastRenderSignature: getRenderSignature(c, formatCommentPrompt),
|
||||
laidOut: false
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue