🧯 fix: Prevent Quote Popup Update Loop (#15113)

This commit is contained in:
Danny Avila 2026-08-22 01:08:44 -04:00 committed by GitHub
parent 67b7b441b2
commit f384e71f77
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 113 additions and 11 deletions

View file

@ -248,7 +248,6 @@ const resolveTop = (anchor: Anchor, height: number, preferBelow: boolean): numbe
function QuoteButton({ conversationId }: { conversationId: string }) {
const localize = useLocalize();
const [selection, setSelection] = useState<SelectionState | null>(null);
const [pos, setPos] = useState<{ top: number; left: number } | null>(null);
const buttonRef = useRef<HTMLButtonElement>(null);
const rangeRef = useRef<Range | null>(null);
const clippersRef = useRef<HTMLElement[]>([]);
@ -284,7 +283,6 @@ function QuoteButton({ conversationId }: { conversationId: string }) {
rangeRef.current = null;
clippersRef.current = [];
setSelection(null);
setPos(null);
};
const hide = () => {
@ -429,13 +427,16 @@ function QuoteButton({ conversationId }: { conversationId: string }) {
};
}, []);
/** Clamp using the button's real size so it never lands off-screen. Runs
* before paint, so the first visible frame is already in its final spot. */
/** Clamp using the button's real size so it never lands off-screen. Apply the
* measured layout directly before paint: feeding it back through state adds
* a synchronous render to every selection update and can exhaust React's
* nested-update limit while the browser is still changing the selection. */
useLayoutEffect(() => {
if (!selection || !buttonRef.current) {
const button = buttonRef.current;
if (!selection || !button) {
return;
}
const { width, height } = buttonRef.current.getBoundingClientRect();
const { width, height } = button.getBoundingClientRect();
const maxLeft = Math.max(EDGE_MARGIN, window.innerWidth - width - EDGE_MARGIN);
const left = Math.min(
Math.max(anchorCenterX(selection.anchor) - width / 2, EDGE_MARGIN),
@ -443,7 +444,9 @@ function QuoteButton({ conversationId }: { conversationId: string }) {
);
const top = resolveTop(selection.anchor, height, selection.viaTouch);
setPos((prev) => (prev && prev.top === top && prev.left === left ? prev : { top, left }));
button.style.top = `${top}px`;
button.style.left = `${left}px`;
button.style.visibility = 'visible';
}, [selection]);
const commitQuote = useCallback(
@ -455,7 +458,6 @@ function QuoteButton({ conversationId }: { conversationId: string }) {
clippersRef.current = [];
pressedTextRef.current = null;
setSelection(null);
setPos(null);
window.getSelection()?.removeAllRanges();
document.getElementById(mainTextareaId)?.focus();
},
@ -538,10 +540,10 @@ function QuoteButton({ conversationId }: { conversationId: string }) {
aria-label={localize('com_ui_add_to_chat')}
data-testid="add-to-chat-button"
style={{
top: pos?.top ?? 0,
left: pos?.left ?? 0,
top: 0,
left: 0,
/** Hidden until measured so it never flashes at an unclamped position. */
visibility: pos == null ? 'hidden' : 'visible',
visibility: 'hidden',
}}
className={cn(
'fixed z-50 inline-flex items-center gap-1.5 rounded-full border border-border-light bg-surface-secondary text-sm font-medium text-text-primary shadow-lg transition-colors hover:bg-surface-tertiary',

View file

@ -0,0 +1,100 @@
import React, { Profiler } from 'react';
import { RecoilRoot } from 'recoil';
import { render, screen, fireEvent } from '@testing-library/react';
jest.mock('~/hooks', () => ({
useLocalize: () => (key: string) => key,
}));
import QuoteButton from '../QuoteButton';
const CONVO_ID = 'convo-1';
const SELECTED_TEXT = 'Selected assistant text';
const rect = ({
top,
bottom,
left,
right,
}: {
top: number;
bottom: number;
left: number;
right: number;
}): DOMRect =>
({
top,
bottom,
left,
right,
x: left,
y: top,
width: right - left,
height: bottom - top,
toJSON: () => ({}),
}) as DOMRect;
describe('QuoteButton', () => {
afterEach(() => {
window.getSelection()?.removeAllRanges();
});
it('positions a new selection without scheduling a layout-state render', () => {
const rangeRect = rect({ top: 100, bottom: 120, left: 200, right: 260 });
const buttonRect = rect({ top: 0, bottom: 30, left: 0, right: 100 });
const originalRangeRect = Range.prototype.getBoundingClientRect;
const elementRect = jest
.spyOn(HTMLElement.prototype, 'getBoundingClientRect')
.mockImplementation(function (this: HTMLElement) {
return this instanceof HTMLButtonElement
? buttonRect
: rect({ top: 0, bottom: 0, left: 0, right: 0 });
});
Object.defineProperty(Range.prototype, 'getBoundingClientRect', {
configurable: true,
value: () => rangeRect,
});
try {
const onRender = jest.fn();
render(
<RecoilRoot>
<div className="message-render">{SELECTED_TEXT}</div>
<Profiler id="quote-button" onRender={onRender}>
<QuoteButton conversationId={CONVO_ID} />
</Profiler>
</RecoilRoot>,
);
onRender.mockClear();
const textNode = screen.getByText(SELECTED_TEXT).firstChild;
if (!textNode) {
throw new Error('Selection text node was not rendered');
}
const range = document.createRange();
range.setStart(textNode, 0);
range.setEnd(textNode, SELECTED_TEXT.length);
window.getSelection()?.removeAllRanges();
window.getSelection()?.addRange(range);
fireEvent.mouseUp(document);
expect(screen.getByTestId('add-to-chat-button')).toHaveStyle({
top: '62px',
left: '180px',
visibility: 'visible',
});
expect(onRender).toHaveBeenCalledTimes(1);
} finally {
elementRect.mockRestore();
if (originalRangeRect) {
Object.defineProperty(Range.prototype, 'getBoundingClientRect', {
configurable: true,
value: originalRangeRect,
});
} else {
delete (Range.prototype as Partial<Range>).getBoundingClientRect;
}
}
});
});