fix(chat): clamp text selection menu Y position to the viewport (#2733)
fix(chat): clamp text selection menu Y position to the viewport
This commit is contained in:
@@ -20,6 +20,12 @@ import { rangeToMarkdown, trimSelectionValue, wrapMarkdownSelectionForChat } fro
|
|||||||
import { focusChatInput } from '@/components/chat/composer/editor/dom';
|
import { focusChatInput } from '@/components/chat/composer/editor/dom';
|
||||||
import { registerActiveSelectionToolbar } from '@/lib/addSelectionToChat';
|
import { registerActiveSelectionToolbar } from '@/lib/addSelectionToChat';
|
||||||
import { collectSelectionOverlayRects } from '@/lib/selectionOverlayRects';
|
import { collectSelectionOverlayRects } from '@/lib/selectionOverlayRects';
|
||||||
|
import {
|
||||||
|
DESKTOP_MENU_FALLBACK_HEIGHT_PX,
|
||||||
|
DESKTOP_MENU_FALLBACK_WIDTH_PX,
|
||||||
|
getDesktopClampedX,
|
||||||
|
getDesktopClampedY,
|
||||||
|
} from './selectionMenuPosition';
|
||||||
|
|
||||||
interface TextSelectionMenuProps {
|
interface TextSelectionMenuProps {
|
||||||
containerRef: React.RefObject<HTMLElement | null>;
|
containerRef: React.RefObject<HTMLElement | null>;
|
||||||
@@ -43,8 +49,6 @@ const normalizeDistilledInsight = (insight: string): string => (
|
|||||||
insight.trim().replace(/^[-*+]\s+/, '').slice(0, PROJECT_NOTE_BODY_MAX_LENGTH)
|
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<TextSelectionMenuProps> = ({ containerRef }) => {
|
export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerRef }) => {
|
||||||
const { t } = useI18n();
|
const { t } = useI18n();
|
||||||
const [position, setPosition] = React.useState<MenuPosition>({ x: 0, y: 0, show: false });
|
const [position, setPosition] = React.useState<MenuPosition>({ x: 0, y: 0, show: false });
|
||||||
@@ -103,6 +107,7 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
|
|||||||
const [isAddingToNotes, setIsAddingToNotes] = React.useState(false);
|
const [isAddingToNotes, setIsAddingToNotes] = React.useState(false);
|
||||||
const menuRef = React.useRef<HTMLDivElement>(null);
|
const menuRef = React.useRef<HTMLDivElement>(null);
|
||||||
const menuWidthRef = React.useRef(DESKTOP_MENU_FALLBACK_WIDTH_PX);
|
const menuWidthRef = React.useRef(DESKTOP_MENU_FALLBACK_WIDTH_PX);
|
||||||
|
const menuHeightRef = React.useRef(DESKTOP_MENU_FALLBACK_HEIGHT_PX);
|
||||||
const pendingSelectionRef = React.useRef<SelectionPayload | null>(null);
|
const pendingSelectionRef = React.useRef<SelectionPayload | null>(null);
|
||||||
const openRafRef = React.useRef<number | null>(null);
|
const openRafRef = React.useRef<number | null>(null);
|
||||||
const mouseUpTimeoutRef = React.useRef<number | null>(null);
|
const mouseUpTimeoutRef = React.useRef<number | null>(null);
|
||||||
@@ -196,23 +201,13 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
|
|||||||
isMenuVisibleRef.current = false;
|
isMenuVisibleRef.current = false;
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
const getDesktopClampedX = React.useCallback((anchorX: number) => {
|
const getClampedX = React.useCallback((anchorX: number) => (
|
||||||
if (typeof window === 'undefined') {
|
getDesktopClampedX(anchorX, window.innerWidth, menuWidthRef.current)
|
||||||
return anchorX;
|
), []);
|
||||||
}
|
|
||||||
|
|
||||||
const viewportWidth = window.innerWidth;
|
const getClampedY = React.useCallback((anchorY: number) => (
|
||||||
const menuWidth = menuWidthRef.current;
|
getDesktopClampedY(anchorY, window.innerHeight, menuHeightRef.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 addMarkdownToChat = React.useCallback((markdownText: string) => {
|
const addMarkdownToChat = React.useCallback((markdownText: string) => {
|
||||||
const markdownBlock = wrapMarkdownSelectionForChat(markdownText);
|
const markdownBlock = wrapMarkdownSelectionForChat(markdownText);
|
||||||
@@ -241,8 +236,10 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
|
|||||||
// Position menu above the selection
|
// Position menu above the selection
|
||||||
const menuX = isMobile
|
const menuX = isMobile
|
||||||
? rect.left + rect.width / 2
|
? rect.left + rect.width / 2
|
||||||
: getDesktopClampedX(rect.left + rect.width / 2);
|
: getClampedX(rect.left + rect.width / 2);
|
||||||
const menuY = rect.top - 10;
|
const menuY = isMobile
|
||||||
|
? rect.top - 10
|
||||||
|
: getClampedY(rect.top - 10);
|
||||||
|
|
||||||
setSelectedText(plainText);
|
setSelectedText(plainText);
|
||||||
setSelectedTextMarkdown(markdownText);
|
setSelectedTextMarkdown(markdownText);
|
||||||
@@ -264,7 +261,7 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
|
|||||||
openRafRef.current = null;
|
openRafRef.current = null;
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}, [addMarkdownToChat, getDesktopClampedX, hideMenu, isMobile, position.show]);
|
}, [addMarkdownToChat, getClampedX, getClampedY, hideMenu, isMobile, position.show]);
|
||||||
|
|
||||||
React.useLayoutEffect(() => {
|
React.useLayoutEffect(() => {
|
||||||
if (!position.show || isMobile || !menuRef.current) {
|
if (!position.show || isMobile || !menuRef.current) {
|
||||||
@@ -272,16 +269,25 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
|
|||||||
}
|
}
|
||||||
|
|
||||||
const measuredWidth = menuRef.current.offsetWidth;
|
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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
menuWidthRef.current = measuredWidth;
|
if (widthChanged) {
|
||||||
|
menuWidthRef.current = measuredWidth;
|
||||||
|
}
|
||||||
|
if (heightChanged) {
|
||||||
|
menuHeightRef.current = measuredHeight;
|
||||||
|
}
|
||||||
setPosition((prev) => ({
|
setPosition((prev) => ({
|
||||||
...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
|
// 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
|
// top of the chat can climb over the app header. On the desktop shell the
|
||||||
@@ -310,7 +316,8 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
|
|||||||
const handleViewportResize = () => {
|
const handleViewportResize = () => {
|
||||||
setPosition((prev) => ({
|
setPosition((prev) => ({
|
||||||
...prev,
|
...prev,
|
||||||
x: getDesktopClampedX(prev.x),
|
x: getClampedX(prev.x),
|
||||||
|
y: getClampedY(prev.y),
|
||||||
}));
|
}));
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -318,7 +325,7 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
|
|||||||
return () => {
|
return () => {
|
||||||
window.removeEventListener('resize', handleViewportResize);
|
window.removeEventListener('resize', handleViewportResize);
|
||||||
};
|
};
|
||||||
}, [getDesktopClampedX, isMobile, position.show]);
|
}, [getClampedX, getClampedY, isMobile, position.show]);
|
||||||
|
|
||||||
const handleSelectionChange = React.useCallback(() => {
|
const handleSelectionChange = React.useCallback(() => {
|
||||||
// While the comment input is open, clicking or typing in it collapses the
|
// While the comment input is open, clicking or typing in it collapses the
|
||||||
|
|||||||
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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);
|
||||||
|
};
|
||||||
Reference in New Issue
Block a user