From f384e71f777f11d50f6e8f9e932623a0b42ec151 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 22 Aug 2026 01:08:44 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=AF=20fix:=20Prevent=20Quote=20Popup?= =?UTF-8?q?=20Update=20Loop=20(#15113)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/components/Chat/Input/QuoteButton.tsx | 24 +++-- .../Chat/Input/__tests__/QuoteButton.test.tsx | 100 ++++++++++++++++++ 2 files changed, 113 insertions(+), 11 deletions(-) create mode 100644 client/src/components/Chat/Input/__tests__/QuoteButton.test.tsx diff --git a/client/src/components/Chat/Input/QuoteButton.tsx b/client/src/components/Chat/Input/QuoteButton.tsx index c841afa361..1d72ea28c5 100644 --- a/client/src/components/Chat/Input/QuoteButton.tsx +++ b/client/src/components/Chat/Input/QuoteButton.tsx @@ -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(null); - const [pos, setPos] = useState<{ top: number; left: number } | null>(null); const buttonRef = useRef(null); const rangeRef = useRef(null); const clippersRef = useRef([]); @@ -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', diff --git a/client/src/components/Chat/Input/__tests__/QuoteButton.test.tsx b/client/src/components/Chat/Input/__tests__/QuoteButton.test.tsx new file mode 100644 index 0000000000..1a7fe5b619 --- /dev/null +++ b/client/src/components/Chat/Input/__tests__/QuoteButton.test.tsx @@ -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( + +
{SELECTED_TEXT}
+ + + +
, + ); + + 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).getBoundingClientRect; + } + } + }); +});