fix(ui): stop the reveal reflow from dismissing popovers (#10487)

* fix(ui): stop the reveal reflow from dismissing popovers

The main process reflows the renderer on every reveal, resume, and restore.
On macOS 26 that nudges the emulated device scale factor, and Chromium fires
a real window resize for it even though innerWidth/innerHeight are identical.
Anything bound directly to resize treated that as a user resize.

The selection copy menu and the markdown link bubble both dismiss on resize
unconditionally, so they closed on their own whenever the window was revealed
or restored. Gate both on an actual dimension change.

Only fixes the macOS 26 path: the pre-Tahoe repaint jiggles the native frame
by a real pixel, so the renderer sees a genuine dimension change and no
change-detection guard can — or should — filter it.

Co-authored-by: Orca <help@stably.ai>

* test(ui): cover the reveal-reflow guard at the component level

Pins both halves of the wiring: a same-size resize must not dismiss, a real
one still must. Mutation-checked — reverting to a bare resize listener, or
dropping the listener entirely, each fails the suite.

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>
(cherry picked from commit cafa958e70)
This commit is contained in:
Neil
2026-07-24 23:53:06 -07:00
committed by Brennan Benson
parent 612b2db364
commit aa27cc05d2
6 changed files with 275 additions and 4 deletions
@@ -0,0 +1,71 @@
// @vitest-environment happy-dom
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { SelectedTextCopyMenu } from './SelectedTextCopyMenu'
vi.mock('@/i18n/i18n', () => ({
translate: (_key: string, fallback: string) => fallback
}))
afterEach(cleanup)
beforeEach(() => {
Object.assign(window, { api: { ui: { writeClipboardText: vi.fn() } } })
setViewportSize(1200, 800)
})
function setViewportSize(width: number, height: number): void {
Object.assign(window, { innerWidth: width, innerHeight: height })
}
function openMenuOnSelectedText(): void {
render(
<SelectedTextCopyMenu>
<span data-testid="content">selected words</span>
</SelectedTextCopyMenu>
)
const content = screen.getByTestId('content')
const range = document.createRange()
range.selectNodeContents(content)
const selection = window.getSelection()
selection?.removeAllRanges()
selection?.addRange(range)
fireEvent.contextMenu(content, { clientX: 120, clientY: 240 })
}
describe('SelectedTextCopyMenu', () => {
it('opens on right-click over selected text', () => {
openMenuOnSelectedText()
expect(screen.getByRole('button', { name: 'Copy' })).toBeDefined()
})
it('stays open when a resize reports the same viewport size', () => {
openMenuOnSelectedText()
// Why: the reveal reflow fires a real resize event without changing any dimension.
fireEvent(window, new Event('resize'))
expect(screen.queryByRole('button', { name: 'Copy' })).not.toBeNull()
})
it('closes when a resize actually changes the viewport size', () => {
openMenuOnSelectedText()
setViewportSize(900, 800)
fireEvent(window, new Event('resize'))
expect(screen.queryByRole('button', { name: 'Copy' })).toBeNull()
})
it('still closes on a real resize that follows a no-op one', () => {
openMenuOnSelectedText()
fireEvent(window, new Event('resize'))
setViewportSize(1200, 640)
fireEvent(window, new Event('resize'))
expect(screen.queryByRole('button', { name: 'Copy' })).toBeNull()
})
})
@@ -2,6 +2,7 @@ import React from 'react'
import { createPortal } from 'react-dom'
import { Copy } from 'lucide-react'
import { translate } from '@/i18n/i18n'
import { addViewportSizeChangeListener } from '@/hooks/viewport-size-change-listener'
type SelectedTextCopyMenuProps = {
children: React.ReactNode
@@ -58,12 +59,14 @@ export function SelectedTextCopyMenu({
window.addEventListener('pointerdown', close)
window.addEventListener('keydown', handleKeyDown, true)
window.addEventListener('scroll', close, true)
window.addEventListener('resize', close)
// Why: a bare resize listener also fires on the main process's reveal reflow, which changes
// no dimensions — the menu would close on every window restore.
const removeViewportListener = addViewportSizeChangeListener(close)
return () => {
window.removeEventListener('pointerdown', close)
window.removeEventListener('keydown', handleKeyDown, true)
window.removeEventListener('scroll', close, true)
window.removeEventListener('resize', close)
removeViewportListener()
}
}, [menu])
@@ -0,0 +1,83 @@
// @vitest-environment happy-dom
import { cleanup, fireEvent, render } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { TooltipProvider } from '@/components/ui/tooltip'
import { RichMarkdownLinkBubble, type LinkBubbleState } from './RichMarkdownLinkBubble'
vi.mock('@/i18n/i18n', () => ({
translate: (_key: string, fallback: string) => fallback
}))
afterEach(cleanup)
beforeEach(() => {
setViewportSize(1200, 800)
})
function setViewportSize(width: number, height: number): void {
Object.assign(window, { innerWidth: width, innerHeight: height })
}
const LINK_BUBBLE: LinkBubbleState = {
kind: 'markdown',
href: 'https://example.com',
left: 40,
top: 60,
openEnabled: true,
copyEnabled: true
}
function renderBubble(): { onDismiss: ReturnType<typeof vi.fn> } {
const anchorElement = document.createElement('div')
document.body.append(anchorElement)
const onDismiss = vi.fn()
render(
<TooltipProvider>
<RichMarkdownLinkBubble
anchorElement={anchorElement}
linkBubble={LINK_BUBBLE}
isEditing={false}
onDismiss={onDismiss}
onSave={vi.fn()}
onRemove={vi.fn()}
onEditStart={vi.fn()}
onEditCancel={vi.fn()}
onOpen={vi.fn()}
onCopy={vi.fn()}
/>
</TooltipProvider>
)
onDismiss.mockClear()
return { onDismiss }
}
describe('RichMarkdownLinkBubble viewport dismissal', () => {
it('stays open when a resize reports the same viewport size', () => {
const { onDismiss } = renderBubble()
// Why: the reveal reflow fires a real resize event without changing any dimension.
fireEvent(window, new Event('resize'))
expect(onDismiss).not.toHaveBeenCalled()
})
it('dismisses when a resize actually changes the viewport size', () => {
const { onDismiss } = renderBubble()
setViewportSize(1200, 640)
fireEvent(window, new Event('resize'))
expect(onDismiss).toHaveBeenCalled()
})
it('still dismisses on a real resize that follows a no-op one', () => {
const { onDismiss } = renderBubble()
fireEvent(window, new Event('resize'))
setViewportSize(900, 800)
fireEvent(window, new Event('resize'))
expect(onDismiss).toHaveBeenCalled()
})
})
@@ -5,6 +5,7 @@ import { Copy, ExternalLink, Pencil, Unlink } from 'lucide-react'
import { translate } from '@/i18n/i18n'
import { Button } from '@/components/ui/button'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { addViewportSizeChangeListener } from '@/hooks/viewport-size-change-listener'
export type LinkBubbleState = {
kind: 'markdown' | 'html-superscript'
@@ -258,7 +259,9 @@ export function RichMarkdownLinkBubble({
window.addEventListener('pointerdown', dismissOutside, true)
window.addEventListener('focusin', dismissOutside, true)
window.addEventListener('scroll', dismissOnScroll, true)
window.addEventListener('resize', dismiss)
// Why: a bare resize listener also fires on the main process's reveal reflow, which changes
// no dimensions — the bubble would dismiss on every window restore.
const removeViewportListener = addViewportSizeChangeListener(dismiss)
return () => {
resizeObserver.disconnect()
intersectionObserver.disconnect()
@@ -266,7 +269,7 @@ export function RichMarkdownLinkBubble({
window.removeEventListener('pointerdown', dismissOutside, true)
window.removeEventListener('focusin', dismissOutside, true)
window.removeEventListener('scroll', dismissOnScroll, true)
window.removeEventListener('resize', dismiss)
removeViewportListener()
}
}, [anchorElement])
@@ -0,0 +1,79 @@
import { describe, expect, it, vi } from 'vitest'
import { addViewportSizeChangeListener } from './viewport-size-change-listener'
function createViewport(width: number, height: number) {
const listeners = new Set<() => void>()
return {
innerWidth: width,
innerHeight: height,
addEventListener: (_type: 'resize', listener: () => void) => {
listeners.add(listener)
},
removeEventListener: (_type: 'resize', listener: () => void) => {
listeners.delete(listener)
},
resizeTo(nextWidth: number, nextHeight: number) {
this.innerWidth = nextWidth
this.innerHeight = nextHeight
for (const listener of listeners) {
listener()
}
},
emitResize() {
for (const listener of listeners) {
listener()
}
},
get listenerCount() {
return listeners.size
}
}
}
describe('addViewportSizeChangeListener', () => {
it('runs the callback when the viewport size changes', () => {
const viewport = createViewport(1024, 768)
const onChange = vi.fn()
addViewportSizeChangeListener(onChange, viewport)
viewport.resizeTo(1024, 900)
expect(onChange).toHaveBeenCalledTimes(1)
})
// Why: the reveal reflow nudges the device scale factor, so Chromium fires resize with identical
// dimensions — dismissing transient UI there closes it on every window restore.
it('ignores a resize that leaves the dimensions untouched', () => {
const viewport = createViewport(1024, 768)
const onChange = vi.fn()
addViewportSizeChangeListener(onChange, viewport)
viewport.emitResize()
expect(onChange).not.toHaveBeenCalled()
})
it('still reports a later real change after ignoring a no-op resize', () => {
const viewport = createViewport(1024, 768)
const onChange = vi.fn()
addViewportSizeChangeListener(onChange, viewport)
viewport.emitResize()
viewport.resizeTo(800, 768)
expect(onChange).toHaveBeenCalledTimes(1)
})
it('treats each size as the new baseline rather than the original', () => {
const viewport = createViewport(1024, 768)
const onChange = vi.fn()
addViewportSizeChangeListener(onChange, viewport)
viewport.resizeTo(900, 768)
viewport.emitResize()
expect(onChange).toHaveBeenCalledTimes(1)
})
it('stops listening once removed', () => {
const viewport = createViewport(1024, 768)
const onChange = vi.fn()
const remove = addViewportSizeChangeListener(onChange, viewport)
remove()
expect(viewport.listenerCount).toBe(0)
viewport.resizeTo(640, 480)
expect(onChange).not.toHaveBeenCalled()
})
})
@@ -0,0 +1,32 @@
/**
* Run `onChange` only when a `resize` event actually changed the viewport's CSS size.
*
* Why: the main process reflows the renderer by briefly nudging the emulated device scale factor
* (macOS 26) or the native frame (elsewhere) on every reveal, resume, and restore. Chromium fires
* a real `resize` for those even though `innerWidth`/`innerHeight` are identical, so transient UI
* bound directly to `resize` gets dismissed with no user interaction.
*/
type ViewportEventTarget = Pick<Window, 'innerWidth' | 'innerHeight'> & {
addEventListener: (type: 'resize', listener: () => void) => void
removeEventListener: (type: 'resize', listener: () => void) => void
}
export function addViewportSizeChangeListener(
onChange: () => void,
// Why: the suite runs in node with no DOM, so tests pass a stub instead.
target: ViewportEventTarget = window
): () => void {
let lastWidth = target.innerWidth
let lastHeight = target.innerHeight
const handleResize = (): void => {
const { innerWidth, innerHeight } = target
if (innerWidth === lastWidth && innerHeight === lastHeight) {
return
}
lastWidth = innerWidth
lastHeight = innerHeight
onChange()
}
target.addEventListener('resize', handleResize)
return () => target.removeEventListener('resize', handleResize)
}