From 1ef11446102eed1947cc7977ac069d0c96ea43df Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Tue, 9 Jun 2026 11:01:02 +0800 Subject: [PATCH] polish(web-shell): address ytahdn review on mode picker - ApprovalModeMessage: dismiss on touchstart too (tap-outside on touch devices) and skip when the press was already defaultPrevented. - MessageList: re-check shouldFollow inside the rAF so a scroll-up during the frame gap doesn't get fought by the tail reveal. - StatusBar: add aria-haspopup="listbox" so the trigger announces it opens a picker. --- .../client/components/MessageList.tsx | 6 +++- .../web-shell/client/components/StatusBar.tsx | 1 + .../messages/ApprovalModeMessage.tsx | 29 ++++++++++++------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/web-shell/client/components/MessageList.tsx b/packages/web-shell/client/components/MessageList.tsx index 51d1e8bc63..8d110cba94 100644 --- a/packages/web-shell/client/components/MessageList.tsx +++ b/packages/web-shell/client/components/MessageList.tsx @@ -429,7 +429,11 @@ export function MessageList({ !prevHasTailContent.current ) { shouldFollow.current = true; - requestAnimationFrame(scrollToBottom); + // Re-check follow inside the frame: if the user scrolls up in the gap + // before it fires (Rule 2 clears the flag), don't fight them. + requestAnimationFrame(() => { + if (shouldFollow.current) scrollToBottom(); + }); } prevHasTailContent.current = hasTailContent; }, [autoScrollTailIntoView, hasTailContent, scrollToBottom]); diff --git a/packages/web-shell/client/components/StatusBar.tsx b/packages/web-shell/client/components/StatusBar.tsx index 56afef0942..cd21a91ab9 100644 --- a/packages/web-shell/client/components/StatusBar.tsx +++ b/packages/web-shell/client/components/StatusBar.tsx @@ -54,6 +54,7 @@ export function StatusBar({ onSelectMode }: StatusBarProps) { onClick={onSelectMode} onMouseDown={(e) => e.stopPropagation()} title={t('mode.select')} + aria-haspopup="listbox" > {modeIndicator.label} diff --git a/packages/web-shell/client/components/messages/ApprovalModeMessage.tsx b/packages/web-shell/client/components/messages/ApprovalModeMessage.tsx index 5bbddd4789..e87e28a81e 100644 --- a/packages/web-shell/client/components/messages/ApprovalModeMessage.tsx +++ b/packages/web-shell/client/components/messages/ApprovalModeMessage.tsx @@ -59,24 +59,31 @@ export function ApprovalModeMessage({ return () => emitActive(false); }, [emitActive]); - // Close when the user presses the mouse outside the panel. The panel is - // rendered inline (no modal backdrop), so we listen on the document. The - // click that opened the panel has already finished propagating by the time - // this effect runs, so it cannot self-close. + // Close when the user presses outside the panel. The panel is rendered + // inline (no modal backdrop), so we listen on the document. The press that + // opened the panel has already finished propagating by the time this effect + // runs, so it cannot self-close. We cover touch as well so a tap outside + // dismisses on touch devices, not only via Escape / a row click. useEffect(() => { - const onMouseDownOutside = (event: MouseEvent) => { - // Only the primary (left) button dismisses. Middle-click on Linux/X11 - // pastes, and right-click opens a context menu — neither should close - // the panel out from under the user. - if (event.button !== 0) return; + const onPointerOutside = (event: Event) => { + // Only the primary (left) mouse button dismisses. Middle-click on + // Linux/X11 pastes, and right-click opens a context menu — neither should + // close the panel out from under the user. (Touch events have no button.) + if (event instanceof MouseEvent && event.button !== 0) return; + // If another handler already consumed the press, leave the panel alone. + if (event.defaultPrevented) return; const panel = panelRef.current; const target = event.target; if (panel && target instanceof Node && !panel.contains(target)) { onCloseRef.current(); } }; - window.addEventListener('mousedown', onMouseDownOutside); - return () => window.removeEventListener('mousedown', onMouseDownOutside); + window.addEventListener('mousedown', onPointerOutside); + window.addEventListener('touchstart', onPointerOutside); + return () => { + window.removeEventListener('mousedown', onPointerOutside); + window.removeEventListener('touchstart', onPointerOutside); + }; }, []); useEffect(() => {