diff --git a/packages/ui/src/components/chat/message/TextSelectionMenu.tsx b/packages/ui/src/components/chat/message/TextSelectionMenu.tsx index f3a8f530..0e4cd205 100644 --- a/packages/ui/src/components/chat/message/TextSelectionMenu.tsx +++ b/packages/ui/src/components/chat/message/TextSelectionMenu.tsx @@ -20,6 +20,12 @@ import { rangeToMarkdown, trimSelectionValue, wrapMarkdownSelectionForChat } fro import { focusChatInput } from '@/components/chat/composer/editor/dom'; import { registerActiveSelectionToolbar } from '@/lib/addSelectionToChat'; import { collectSelectionOverlayRects } from '@/lib/selectionOverlayRects'; +import { + DESKTOP_MENU_FALLBACK_HEIGHT_PX, + DESKTOP_MENU_FALLBACK_WIDTH_PX, + getDesktopClampedX, + getDesktopClampedY, +} from './selectionMenuPosition'; interface TextSelectionMenuProps { containerRef: React.RefObject; @@ -43,8 +49,6 @@ const normalizeDistilledInsight = (insight: string): string => ( insight.trim().replace(/^[-*+]\s+/, '').slice(0, PROJECT_NOTE_BODY_MAX_LENGTH) ); -const DESKTOP_MENU_SIDE_MARGIN_PX = 8; -const DESKTOP_MENU_FALLBACK_WIDTH_PX = 280; export const TextSelectionMenu: React.FC = ({ containerRef }) => { const { t } = useI18n(); const [position, setPosition] = React.useState({ x: 0, y: 0, show: false }); @@ -103,6 +107,7 @@ export const TextSelectionMenu: React.FC = ({ containerR const [isAddingToNotes, setIsAddingToNotes] = React.useState(false); const menuRef = React.useRef(null); const menuWidthRef = React.useRef(DESKTOP_MENU_FALLBACK_WIDTH_PX); + const menuHeightRef = React.useRef(DESKTOP_MENU_FALLBACK_HEIGHT_PX); const pendingSelectionRef = React.useRef(null); const openRafRef = React.useRef(null); const mouseUpTimeoutRef = React.useRef(null); @@ -196,23 +201,13 @@ export const TextSelectionMenu: React.FC = ({ containerR isMenuVisibleRef.current = false; }, []); - const getDesktopClampedX = React.useCallback((anchorX: number) => { - if (typeof window === 'undefined') { - return anchorX; - } + const getClampedX = React.useCallback((anchorX: number) => ( + getDesktopClampedX(anchorX, window.innerWidth, menuWidthRef.current) + ), []); - const viewportWidth = window.innerWidth; - const menuWidth = menuWidthRef.current; - const halfWidth = menuWidth / 2; - const minX = DESKTOP_MENU_SIDE_MARGIN_PX + halfWidth; - const maxX = viewportWidth - DESKTOP_MENU_SIDE_MARGIN_PX - halfWidth; - - if (minX > maxX) { - return viewportWidth / 2; - } - - return Math.min(Math.max(anchorX, minX), maxX); - }, []); + const getClampedY = React.useCallback((anchorY: number) => ( + getDesktopClampedY(anchorY, window.innerHeight, menuHeightRef.current) + ), []); const addMarkdownToChat = React.useCallback((markdownText: string) => { const markdownBlock = wrapMarkdownSelectionForChat(markdownText); @@ -241,8 +236,10 @@ export const TextSelectionMenu: React.FC = ({ containerR // Position menu above the selection const menuX = isMobile ? rect.left + rect.width / 2 - : getDesktopClampedX(rect.left + rect.width / 2); - const menuY = rect.top - 10; + : getClampedX(rect.left + rect.width / 2); + const menuY = isMobile + ? rect.top - 10 + : getClampedY(rect.top - 10); setSelectedText(plainText); setSelectedTextMarkdown(markdownText); @@ -264,7 +261,7 @@ export const TextSelectionMenu: React.FC = ({ containerR openRafRef.current = null; }); } - }, [addMarkdownToChat, getDesktopClampedX, hideMenu, isMobile, position.show]); + }, [addMarkdownToChat, getClampedX, getClampedY, hideMenu, isMobile, position.show]); React.useLayoutEffect(() => { if (!position.show || isMobile || !menuRef.current) { @@ -272,16 +269,25 @@ export const TextSelectionMenu: React.FC = ({ containerR } const measuredWidth = menuRef.current.offsetWidth; - if (!Number.isFinite(measuredWidth) || measuredWidth <= 0 || measuredWidth === menuWidthRef.current) { + const measuredHeight = menuRef.current.offsetHeight; + const widthChanged = Number.isFinite(measuredWidth) && measuredWidth > 0 && measuredWidth !== menuWidthRef.current; + const heightChanged = Number.isFinite(measuredHeight) && measuredHeight > 0 && measuredHeight !== menuHeightRef.current; + if (!widthChanged && !heightChanged) { return; } - menuWidthRef.current = measuredWidth; + if (widthChanged) { + menuWidthRef.current = measuredWidth; + } + if (heightChanged) { + menuHeightRef.current = measuredHeight; + } setPosition((prev) => ({ ...prev, - x: getDesktopClampedX(prev.x), + x: getClampedX(prev.x), + y: getClampedY(prev.y), })); - }, [getDesktopClampedX, isMobile, position.show]); + }, [getClampedX, getClampedY, isMobile, position.show]); // The desktop popup hangs above its anchor, so a tall comment box near the // top of the chat can climb over the app header. On the desktop shell the @@ -310,7 +316,8 @@ export const TextSelectionMenu: React.FC = ({ containerR const handleViewportResize = () => { setPosition((prev) => ({ ...prev, - x: getDesktopClampedX(prev.x), + x: getClampedX(prev.x), + y: getClampedY(prev.y), })); }; @@ -318,7 +325,7 @@ export const TextSelectionMenu: React.FC = ({ containerR return () => { window.removeEventListener('resize', handleViewportResize); }; - }, [getDesktopClampedX, isMobile, position.show]); + }, [getClampedX, getClampedY, isMobile, position.show]); const handleSelectionChange = React.useCallback(() => { // While the comment input is open, clicking or typing in it collapses the diff --git a/packages/ui/src/components/chat/message/__tests__/selectionMenuPosition.test.ts b/packages/ui/src/components/chat/message/__tests__/selectionMenuPosition.test.ts new file mode 100644 index 00000000..eb3f1a0f --- /dev/null +++ b/packages/ui/src/components/chat/message/__tests__/selectionMenuPosition.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from 'bun:test'; +import { + DESKTOP_MENU_FALLBACK_HEIGHT_PX, + DESKTOP_MENU_FALLBACK_WIDTH_PX, + DESKTOP_MENU_SIDE_MARGIN_PX, + getDesktopClampedX, + getDesktopClampedY, +} from '../selectionMenuPosition'; + +const VIEWPORT_WIDTH = 1024; +const VIEWPORT_HEIGHT = 768; +const MENU_WIDTH = DESKTOP_MENU_FALLBACK_WIDTH_PX; +const MENU_HEIGHT = DESKTOP_MENU_FALLBACK_HEIGHT_PX; + +// Regression coverage for issue #2257: selecting a long assistant response +// across a scroll boundary makes range.getBoundingClientRect().top negative, +// and the unclamped anchor (rect.top - 10) placed the menu above the viewport. +describe('getDesktopClampedY (issue #2257)', () => { + test('keeps the menu on screen when the selection starts above the viewport', () => { + const clamped = getDesktopClampedY(-210, VIEWPORT_HEIGHT, MENU_HEIGHT); + expect(clamped).toBe(DESKTOP_MENU_SIDE_MARGIN_PX + MENU_HEIGHT); + }); + + test('keeps the menu fully visible for selections near the top edge', () => { + // The menu renders with translate(-50%, -100%), so it extends upward from + // the anchor; anchors smaller than margin + menu height clip the menu. + const clamped = getDesktopClampedY(5, VIEWPORT_HEIGHT, MENU_HEIGHT); + expect(clamped).toBe(DESKTOP_MENU_SIDE_MARGIN_PX + MENU_HEIGHT); + }); + + test('clamps anchors below the viewport back to the bottom margin', () => { + const clamped = getDesktopClampedY(VIEWPORT_HEIGHT + 500, VIEWPORT_HEIGHT, MENU_HEIGHT); + expect(clamped).toBe(VIEWPORT_HEIGHT - DESKTOP_MENU_SIDE_MARGIN_PX); + }); + + test('leaves in-viewport anchors unchanged', () => { + expect(getDesktopClampedY(300, VIEWPORT_HEIGHT, MENU_HEIGHT)).toBe(300); + expect(getDesktopClampedY(MENU_HEIGHT + DESKTOP_MENU_SIDE_MARGIN_PX, VIEWPORT_HEIGHT, MENU_HEIGHT)) + .toBe(MENU_HEIGHT + DESKTOP_MENU_SIDE_MARGIN_PX); + }); + + test('falls back to the viewport middle when the viewport is shorter than the menu', () => { + const tinyViewportHeight = MENU_HEIGHT; + expect(getDesktopClampedY(10, tinyViewportHeight, MENU_HEIGHT)).toBe(tinyViewportHeight / 2); + }); +}); + +describe('getDesktopClampedX', () => { + test('clamps anchors past the left edge to the left margin', () => { + const clamped = getDesktopClampedX(-500, VIEWPORT_WIDTH, MENU_WIDTH); + expect(clamped).toBe(DESKTOP_MENU_SIDE_MARGIN_PX + MENU_WIDTH / 2); + }); + + test('clamps anchors past the right edge to the right margin', () => { + const clamped = getDesktopClampedX(VIEWPORT_WIDTH + 500, VIEWPORT_WIDTH, MENU_WIDTH); + expect(clamped).toBe(VIEWPORT_WIDTH - DESKTOP_MENU_SIDE_MARGIN_PX - MENU_WIDTH / 2); + }); + + test('leaves in-viewport anchors unchanged', () => { + expect(getDesktopClampedX(VIEWPORT_WIDTH / 2, VIEWPORT_WIDTH, MENU_WIDTH)).toBe(VIEWPORT_WIDTH / 2); + }); + + test('falls back to the viewport middle when the viewport is narrower than the menu', () => { + const tinyViewportWidth = MENU_WIDTH / 2; + expect(getDesktopClampedX(10, tinyViewportWidth, MENU_WIDTH)).toBe(tinyViewportWidth / 2); + }); +}); diff --git a/packages/ui/src/components/chat/message/selectionMenuPosition.ts b/packages/ui/src/components/chat/message/selectionMenuPosition.ts new file mode 100644 index 00000000..7a431e6d --- /dev/null +++ b/packages/ui/src/components/chat/message/selectionMenuPosition.ts @@ -0,0 +1,29 @@ +export const DESKTOP_MENU_SIDE_MARGIN_PX = 8; +export const DESKTOP_MENU_FALLBACK_WIDTH_PX = 280; +export const DESKTOP_MENU_FALLBACK_HEIGHT_PX = 38; + +export const getDesktopClampedX = (anchorX: number, viewportWidth: number, menuWidth: number): number => { + const halfWidth = menuWidth / 2; + const minX = DESKTOP_MENU_SIDE_MARGIN_PX + halfWidth; + const maxX = viewportWidth - DESKTOP_MENU_SIDE_MARGIN_PX - halfWidth; + + if (minX > maxX) { + return viewportWidth / 2; + } + + return Math.min(Math.max(anchorX, minX), maxX); +}; + +// The desktop menu renders with `transform: translate(-50%, -100%)`, so the +// anchor Y marks the menu's bottom edge and the menu extends `menuHeight` +// upward from it. The minimum keeps the whole menu below the top margin. +export const getDesktopClampedY = (anchorY: number, viewportHeight: number, menuHeight: number): number => { + const minY = DESKTOP_MENU_SIDE_MARGIN_PX + menuHeight; + const maxY = viewportHeight - DESKTOP_MENU_SIDE_MARGIN_PX; + + if (minY > maxY) { + return viewportHeight / 2; + } + + return Math.min(Math.max(anchorY, minY), maxY); +};