🎚️ feat: Add Focus Management and Drag-to-Scrub to MessageNav (#13497)

* 🧭 feat: Add MessageNav Focus Management and Drag-to-Scroll

Resolves #13491: move keyboard focus into the conversation when a message indicator is selected, and add a Shift+Alt+M shortcut to jump focus back to the nav. Also adds drag-to-scrub interaction across the rib column.

* 🖱️ style: Use grab cursor for MessageNav drag affordance

* 🐛 fix: Harden MessageNav drag against stale pointer state and dead clicks

Addresses Codex/Copilot review on #13497:
- Ignore pre-drag pointermoves when the primary button is not held, preventing a stale press (released outside the column) from starting a spurious scrub or calling setPointerCapture on an inactive pointer.
- Clear the post-drag click-suppression flag after the synthetic-click window so a later activation (including keyboard) is never swallowed.
- Match the advertised Shift+Alt+M shortcut by accepting the layout-aware key in addition to the physical code.

* 🎯 fix: Track MessageNav drag globally so it survives leaving the column

Round-2 Codex review on #13497: the 4px threshold was applied before any pointer capture, so a drag that left the narrow rib column before crossing it silently failed to scrub.

Replace per-element capture with document-level pointermove/up/cancel listeners attached on pointerdown:
- Drag tracking continues regardless of pointer position (fixes diagonal/touch drags off the ribs).
- pointerup is always received, so no stale drag state and no setPointerCapture on an inactive pointer (removes the NotFoundError path entirely).
- Native click is preserved, so the keyboard/click selection a11y path is unchanged.
- Listeners are torn down on pointerup/cancel and on unmount.

* 🧹 fix: Reset drag state on pointer replace and gate MessageNav shortcut

Round-3 Codex review on #13497:
- When a second pointerdown replaces an in-progress drag, run the cleanup with the real drag state so draggingRef is cleared and the rib column resumes auto-centering (was hardcoded to finish(false)).
- Only preventDefault on Shift+Alt+M when the nav is actually rendered and has a focus target, so the shortcut no longer swallows browser/AT shortcuts when the nav is absent (<3 messages). focusNav now reports whether it moved focus.

*  fix: Make MessageNav drag span the whole thread and harden teardown

Round-4 Codex review on #13497:
- Map the drag pointer proportionally across the full entries range instead of the visible rib rects, so long conversations whose mini-nav overflows are fully scrubbable in one drag. This is also wobble-immune, so the column auto-centering no longer needs to be frozen mid-drag (removed the freeze and draggingRef).
- focusNav now reports success only if focus actually landed, so Shift+Alt+M does not preventDefault when the nav is mounted-but-hidden (hidden md:flex on small viewports).
- End the drag if the primary button is released mid-move or the window loses focus, covering pointers released outside the document where pointerup/cancel never arrive.
This commit is contained in:
Danny Avila 2026-06-03 19:23:08 -04:00 committed by GitHub
parent baa23a8e24
commit 4dce0fd3ab
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 498 additions and 10 deletions

View file

@ -73,6 +73,7 @@ function getMessageEntries(root: ParentNode, messagesById: Map<string, TMessage>
const JUMP_EPS = 4;
const SCROLL_DURATION = 400;
const BOTTOM_SNAP_RETRIES = 2;
const DRAG_THRESHOLD = 4;
function easeOutCubic(t: number): number {
return 1 - Math.pow(1 - t, 3);
@ -86,6 +87,18 @@ function readScrollMargin(el: HTMLElement | null): number {
return Number.isFinite(value) ? value : 0;
}
function computeTargetScroll(
container: HTMLElement,
el: HTMLElement,
scrollMargin: number,
): number {
const cRect = container.getBoundingClientRect();
const elRect = el.getBoundingClientRect();
const target = container.scrollTop + (elRect.top - cRect.top) - scrollMargin;
const max = container.scrollHeight - container.clientHeight;
return Math.max(0, Math.min(target, max));
}
const indicatorButtonClasses = cn(
'flex h-[5px] items-center justify-center rounded-sm',
'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-xheavy',
@ -180,6 +193,9 @@ function MessageNav({ scrollableRef }: { scrollableRef: React.RefObject<HTMLDivE
const messagesByIdRef = useRef(messagesById);
const scrollTokenRef = useRef(0);
const scrollMarginRef = useRef(0);
const navRef = useRef<HTMLElement>(null);
const dragCleanupRef = useRef<(() => void) | null>(null);
const suppressClickRef = useRef(false);
const getCurrentVisibleId = useCallback((): string | null => {
let nextId: string | null = null;
@ -246,11 +262,7 @@ function MessageNav({ scrollableRef }: { scrollableRef: React.RefObject<HTMLDivE
if (!current) {
return;
}
const cRect = container.getBoundingClientRect();
const elRect = current.getBoundingClientRect();
const targetScroll = container.scrollTop + (elRect.top - cRect.top) - scrollMargin;
const max = container.scrollHeight - container.clientHeight;
const clamped = Math.max(0, Math.min(targetScroll, max));
const clamped = computeTargetScroll(container, current, scrollMargin);
container.scrollTop = startScroll + (clamped - startScroll) * easeOutCubic(progress);
if (progress < 1) {
requestAnimationFrame(step);
@ -260,6 +272,142 @@ function MessageNav({ scrollableRef }: { scrollableRef: React.RefObject<HTMLDivE
requestAnimationFrame(step);
}, []);
const scrollToImmediate = useCallback((id: string) => {
const el = document.getElementById(id);
if (!el) {
return;
}
const container = el.closest<HTMLElement>('.scrollbar-gutter-stable');
if (!container) {
return;
}
scrollTokenRef.current++;
const scrollMargin = scrollMarginRef.current || readScrollMargin(el);
container.scrollTop = computeTargetScroll(container, el, scrollMargin);
}, []);
const focusMessage = useCallback((id: string) => {
const el = document.getElementById(id);
if (!el) {
return;
}
if (!el.hasAttribute('tabindex')) {
el.setAttribute('tabindex', '-1');
}
el.focus({ preventScroll: true });
}, []);
const handleSelect = useCallback(
(id: string) => {
if (suppressClickRef.current) {
suppressClickRef.current = false;
return;
}
scrollToStart(id);
focusMessage(id);
},
[scrollToStart, focusMessage],
);
const focusNav = useCallback((): boolean => {
const nav = navRef.current;
if (!nav) {
return false;
}
const target =
nav.querySelector<HTMLElement>('[aria-current="true"]') ??
nav.querySelector<HTMLElement>('[data-msg-id]');
if (!target) {
return false;
}
target.focus();
return document.activeElement === target;
}, []);
const scrubTo = useCallback(
(clientY: number) => {
const col = columnRef.current;
if (!col) {
return;
}
const ribs = col.querySelectorAll<HTMLElement>('[data-msg-id]');
const count = ribs.length;
if (count === 0) {
return;
}
const rect = col.getBoundingClientRect();
const fraction = rect.height > 0 ? (clientY - rect.top) / rect.height : 0;
const index = Math.max(0, Math.min(count - 1, Math.round(fraction * (count - 1))));
const id = ribs[index].getAttribute('data-msg-id');
if (id) {
scrollToImmediate(id);
}
},
[scrollToImmediate],
);
const handlePointerDown = useCallback(
(e: React.PointerEvent<HTMLDivElement>) => {
if (e.button !== 0) {
return;
}
dragCleanupRef.current?.();
suppressClickRef.current = false;
const state = { pointerId: e.pointerId, startY: e.clientY, dragging: false };
const finish = (wasDragging: boolean) => {
document.removeEventListener('pointermove', onMove);
document.removeEventListener('pointerup', onUp);
document.removeEventListener('pointercancel', onUp);
window.removeEventListener('blur', onBlur);
dragCleanupRef.current = null;
if (wasDragging) {
suppressClickRef.current = true;
window.setTimeout(() => {
suppressClickRef.current = false;
}, 0);
}
};
function onMove(ev: PointerEvent) {
if (ev.pointerId !== state.pointerId) {
return;
}
if ((ev.buttons & 1) === 0) {
finish(state.dragging);
return;
}
if (!state.dragging) {
if (Math.abs(ev.clientY - state.startY) < DRAG_THRESHOLD) {
return;
}
state.dragging = true;
}
scrubTo(ev.clientY);
}
function onUp(ev: PointerEvent) {
if (ev.pointerId !== state.pointerId) {
return;
}
finish(state.dragging);
}
function onBlur() {
finish(state.dragging);
}
dragCleanupRef.current = () => finish(state.dragging);
document.addEventListener('pointermove', onMove);
document.addEventListener('pointerup', onUp);
document.addEventListener('pointercancel', onUp);
window.addEventListener('blur', onBlur);
},
[scrubTo],
);
useEffect(() => () => dragCleanupRef.current?.(), []);
useEffect(() => {
refreshEntries();
@ -628,13 +776,27 @@ function MessageNav({ scrollableRef }: { scrollableRef: React.RefObject<HTMLDivE
}
}, [entries, scrollableRef, scrollToStart]);
useEffect(() => {
const onKeyDown = (e: KeyboardEvent) => {
if (e.altKey && e.shiftKey && (e.code === 'KeyM' || e.key.toLowerCase() === 'm')) {
if (focusNav()) {
e.preventDefault();
}
}
};
document.addEventListener('keydown', onKeyDown);
return () => document.removeEventListener('keydown', onKeyDown);
}, [focusNav]);
if (entries.length < 3) {
return null;
}
return (
<nav
ref={navRef}
aria-label={localize('com_ui_message_nav')}
aria-keyshortcuts="Shift+Alt+M"
className={cn(
'group/nav absolute right-2 top-1/2 z-40 hidden max-h-[min(24rem,calc(100%-2rem))]',
'-translate-y-1/2 flex-col items-center gap-1.5 rounded-full px-1 py-2 md:flex',
@ -655,7 +817,8 @@ function MessageNav({ scrollableRef }: { scrollableRef: React.RefObject<HTMLDivE
<div
ref={columnRef}
className="flex min-h-0 flex-col items-center gap-1.5 overflow-y-auto [&::-webkit-scrollbar]:hidden"
onPointerDown={handlePointerDown}
className="flex min-h-0 cursor-grab touch-none select-none flex-col items-center gap-1.5 overflow-y-auto active:cursor-grabbing [&::-webkit-scrollbar]:hidden"
style={{ scrollbarWidth: 'none', msOverflowStyle: 'none' }}
>
{entries.map((entry) => (
@ -664,7 +827,7 @@ function MessageNav({ scrollableRef }: { scrollableRef: React.RefObject<HTMLDivE
entry={entry}
isActive={visibleIds.has(entry.id)}
isCurrent={currentId === entry.id}
onSelect={scrollToStart}
onSelect={handleSelect}
label={localize(
entry.isUser ? 'com_ui_message_nav_go_to_user' : 'com_ui_message_nav_go_to_assistant',
{ 0: entry.preview.slice(0, 30) },

View file

@ -107,7 +107,12 @@ export default function Message(props: TMessageProps) {
<div
id={messageId ?? ''}
aria-label={getMessageAriaLabel(message, localize)}
className={cn(baseClasses.common, baseClasses.chat, 'message-render')}
className={cn(
baseClasses.common,
baseClasses.chat,
'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-xheavy',
'message-render',
)}
>
{!hasParallelContent && (
<div className="relative flex flex-shrink-0 flex-col items-center">

View file

@ -88,6 +88,21 @@ class MockIntersectionObserver {
const originalIO = global.IntersectionObserver;
class PointerEventPolyfill extends MouseEvent {
pointerId: number;
constructor(type: string, params: MouseEventInit & { pointerId?: number } = {}) {
super(type, params);
this.pointerId = params.pointerId ?? 0;
}
}
if (typeof (global as { PointerEvent?: unknown }).PointerEvent === 'undefined') {
(global as unknown as { PointerEvent: typeof PointerEventPolyfill }).PointerEvent =
PointerEventPolyfill;
(window as unknown as { PointerEvent: typeof PointerEventPolyfill }).PointerEvent =
PointerEventPolyfill;
}
import MessageNav from '../MessageNav';
function buildMessage(overrides: Partial<TestMessage> = {}): TestMessage {
@ -608,6 +623,311 @@ describe('MessageNav', () => {
});
});
describe('focus management', () => {
it('moves focus to the target message when an indicator is clicked', () => {
const messages = [
buildMessage({ messageId: 'a', text: 'alpha', isCreatedByUser: true }),
buildMessage({ messageId: 'b', text: 'bravo' }),
buildMessage({ messageId: 'c', text: 'charlie', isCreatedByUser: true }),
];
const { container } = renderNav(messages);
const indicator = container.querySelectorAll('[data-msg-id]')[1] as HTMLButtonElement;
act(() => {
fireEvent.click(indicator);
});
const message = document.getElementById('b');
expect(message).toHaveAttribute('tabindex', '-1');
expect(document.activeElement).toBe(message);
});
it('focuses the current indicator when Shift+Alt+M is pressed', () => {
const messages = [
buildMessage({ messageId: 'a', text: 'alpha', isCreatedByUser: true }),
buildMessage({ messageId: 'b', text: 'bravo' }),
buildMessage({ messageId: 'c', text: 'charlie', isCreatedByUser: true }),
];
const { container } = renderNav(messages);
const io = MockIntersectionObserver.last();
act(() => {
io!.trigger([{ target: document.getElementById('b')!, isIntersecting: true }]);
jest.advanceTimersByTime(32);
});
act(() => {
fireEvent.keyDown(document, { code: 'KeyM', altKey: true, shiftKey: true });
});
expect(document.activeElement).toBe(container.querySelector('[data-msg-id="b"]'));
});
it('focuses the current indicator via the produced key on non-QWERTY layouts', () => {
const messages = [
buildMessage({ messageId: 'a', text: 'alpha', isCreatedByUser: true }),
buildMessage({ messageId: 'b', text: 'bravo' }),
buildMessage({ messageId: 'c', text: 'charlie', isCreatedByUser: true }),
];
const { container } = renderNav(messages);
const io = MockIntersectionObserver.last();
act(() => {
io!.trigger([{ target: document.getElementById('b')!, isIntersecting: true }]);
jest.advanceTimersByTime(32);
});
act(() => {
fireEvent.keyDown(document, { key: 'm', code: 'Semicolon', altKey: true, shiftKey: true });
});
expect(document.activeElement).toBe(container.querySelector('[data-msg-id="b"]'));
});
it('consumes Shift+Alt+M only when the nav is rendered', () => {
const messages = [
buildMessage({ messageId: 'a', text: 'alpha', isCreatedByUser: true }),
buildMessage({ messageId: 'b', text: 'bravo' }),
buildMessage({ messageId: 'c', text: 'charlie', isCreatedByUser: true }),
];
renderNav(messages);
let notPrevented = true;
act(() => {
notPrevented = fireEvent.keyDown(document, {
code: 'KeyM',
altKey: true,
shiftKey: true,
});
});
expect(notPrevented).toBe(false);
});
it('does not consume Shift+Alt+M when the nav is not rendered', () => {
renderNav([buildMessage({ messageId: 'solo', text: 'only one', isCreatedByUser: true })]);
let notPrevented = true;
act(() => {
notPrevented = fireEvent.keyDown(document, {
code: 'KeyM',
altKey: true,
shiftKey: true,
});
});
expect(notPrevented).toBe(true);
});
it('ignores the keyboard shortcut without the alt and shift modifiers', () => {
const messages = [
buildMessage({ messageId: 'a', text: 'alpha', isCreatedByUser: true }),
buildMessage({ messageId: 'b', text: 'bravo' }),
buildMessage({ messageId: 'c', text: 'charlie', isCreatedByUser: true }),
];
const { container } = renderNav(messages);
act(() => {
fireEvent.keyDown(document, { code: 'KeyM' });
});
const navButtons = container.querySelectorAll('[data-msg-id]');
expect(Array.from(navButtons)).not.toContain(document.activeElement);
});
});
describe('drag to scroll', () => {
function setupDraggableNav() {
const messages = Array.from({ length: 5 }, (_, i) =>
buildMessage({
messageId: `m-${i}`,
text: `message ${i}`,
isCreatedByUser: i % 2 === 0,
}),
);
const result = renderNav(messages);
const column = result.container.querySelector('nav > div') as HTMLDivElement;
column.getBoundingClientRect = () => ({ top: 0, bottom: 50, height: 50 }) as DOMRect;
const ribs = Array.from(column.querySelectorAll('[data-msg-id]')) as HTMLElement[];
const writes: number[] = [];
Object.defineProperty(result.scrollable, 'scrollTop', {
get: () => 0,
set: (v: number) => {
writes.push(v);
},
configurable: true,
});
return { ...result, column, ribs, writes };
}
it('scrubs the conversation while dragging past the threshold', () => {
const { column, writes } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 25 });
});
expect(writes.length).toBeGreaterThan(0);
});
it('keeps tracking the drag after the pointer leaves the narrow column', () => {
const { column, writes } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
// pointer has moved off the column; the move bubbles to the document listener
fireEvent.pointerMove(document.body, { pointerId: 1, buttons: 1, clientY: 25 });
});
expect(writes.length).toBeGreaterThan(0);
});
it('maps the pointer proportionally across the full set of messages', () => {
const { column } = setupDraggableNav();
const getById = jest.spyOn(document, 'getElementById');
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 25 });
});
expect(getById.mock.calls.map((c) => c[0])).toContain('m-2');
getById.mockClear();
act(() => {
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 50 });
});
expect(getById.mock.calls.map((c) => c[0])).toContain('m-4');
getById.mockRestore();
});
it('does not scrub for movement under the threshold', () => {
const { column, writes } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 2 });
});
expect(writes.length).toBe(0);
});
it('ignores pointer moves after the interaction ends', () => {
const { column, writes } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerUp(document, { pointerId: 1, clientY: 0 });
});
act(() => {
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 40 });
});
expect(writes.length).toBe(0);
});
it('ends the drag when the primary button is released during a move', () => {
const { column, writes } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 25 });
fireEvent.pointerMove(document, { pointerId: 1, buttons: 0, clientY: 30 });
});
const before = writes.length;
act(() => {
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 45 });
});
expect(writes.length).toBe(before);
});
it('ends the drag when the window loses focus', () => {
const { column, writes } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 25 });
window.dispatchEvent(new Event('blur'));
});
const before = writes.length;
act(() => {
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 45 });
});
expect(writes.length).toBe(before);
});
it('tears down a previous drag when a new pointer starts', () => {
const { column, writes } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 25 });
});
act(() => {
fireEvent.pointerDown(column, { pointerId: 2, button: 0, buttons: 1, clientY: 0 });
});
const before = writes.length;
act(() => {
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 45 });
});
expect(writes.length).toBe(before);
});
it('selects via the native click when a press does not become a drag', () => {
const { column, ribs } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerUp(document, { pointerId: 1, clientY: 0 });
});
act(() => {
fireEvent.click(ribs[0]);
});
expect(document.activeElement).toBe(document.getElementById('m-0'));
});
it('suppresses the click that immediately follows a drag', () => {
const { column, ribs } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 25 });
fireEvent.pointerUp(document, { pointerId: 1, clientY: 25 });
});
act(() => {
fireEvent.click(ribs[0]);
});
expect(document.activeElement).not.toBe(document.getElementById('m-0'));
});
it('clears click suppression after the drag so a later activation is honored', () => {
const { column, ribs } = setupDraggableNav();
act(() => {
fireEvent.pointerDown(column, { pointerId: 1, button: 0, buttons: 1, clientY: 0 });
fireEvent.pointerMove(document, { pointerId: 1, buttons: 1, clientY: 25 });
fireEvent.pointerUp(document, { pointerId: 1, clientY: 25 });
jest.advanceTimersByTime(1);
});
act(() => {
fireEvent.click(ribs[0]);
});
expect(document.activeElement).toBe(document.getElementById('m-0'));
});
});
describe('observers', () => {
it('observes each message on mount', () => {
const messages = [

View file

@ -179,7 +179,7 @@ const MessageRender = memo(function MessageRender({
};
const conditionalClasses = {
focus: 'focus:outline-none focus:ring-2 focus:ring-border-xheavy',
focus: 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-xheavy',
};
return (

View file

@ -172,7 +172,7 @@ const ContentRender = memo(function ContentRender({
};
const conditionalClasses = {
focus: 'focus:outline-none focus:ring-2 focus:ring-border-xheavy',
focus: 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-xheavy',
};
return (