From 6b76284c7769e0ca80012a5a4b7e22b1cea05b6b Mon Sep 17 00:00:00 2001 From: brooklyn! Date: Sat, 13 Jun 2026 18:07:22 -0500 Subject: [PATCH] fix(desktop): surface off-screen approvals via the jump-to-bottom control (#45853) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes #45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring. --- .../app/chat/scroll-to-bottom-button.test.tsx | 67 +++++++++++++++++++ .../src/app/chat/scroll-to-bottom-button.tsx | 26 +++++-- apps/desktop/src/i18n/en.ts | 1 + apps/desktop/src/i18n/ja.ts | 1 + apps/desktop/src/i18n/types.ts | 1 + apps/desktop/src/i18n/zh-hant.ts | 1 + apps/desktop/src/i18n/zh.ts | 1 + 7 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 apps/desktop/src/app/chat/scroll-to-bottom-button.test.tsx diff --git a/apps/desktop/src/app/chat/scroll-to-bottom-button.test.tsx b/apps/desktop/src/app/chat/scroll-to-bottom-button.test.tsx new file mode 100644 index 000000000..f163ec848 --- /dev/null +++ b/apps/desktop/src/app/chat/scroll-to-bottom-button.test.tsx @@ -0,0 +1,67 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { clearAllPrompts, setApprovalRequest } from '@/store/prompts' +import { $activeSessionId } from '@/store/session' +import { onScrollToBottomRequest, resetThreadScroll, setThreadAtBottom } from '@/store/thread-scroll' + +import { ScrollToBottomButton } from './scroll-to-bottom-button' + +function pendingApproval() { + $activeSessionId.set('sess-1') + setApprovalRequest({ command: 'rm -rf /tmp/x', description: 'dangerous command', sessionId: 'sess-1' }) +} + +afterEach(() => { + cleanup() + clearAllPrompts() + resetThreadScroll() + $activeSessionId.set(null) +}) + +// `getByRole('button')` excludes aria-hidden nodes, so "queryByRole null" is the +// control's hidden (parked-at-bottom) state. +describe('ScrollToBottomButton', () => { + it('stays hidden while parked at the bottom', () => { + render() + + expect(screen.queryByRole('button')).toBeNull() + }) + + it('is a plain jump-to-bottom control when scrolled up with no approval', () => { + setThreadAtBottom(false) + render() + + expect(screen.getByRole('button', { name: 'Scroll to bottom' })).toBeTruthy() + expect(screen.queryByText('Approval needed')).toBeNull() + }) + + it('morphs into the approval pill when scrolled up with a pending approval', () => { + pendingApproval() + setThreadAtBottom(false) + render() + + expect(screen.getByRole('button', { name: 'Approval needed' })).toBeTruthy() + expect(screen.getByText('Approval needed')).toBeTruthy() + }) + + it('does not morph while a pending approval is still in view (at bottom)', () => { + pendingApproval() + render() + + // Parked at bottom → control hidden, so it can't claim "approval needed". + expect(screen.queryByRole('button')).toBeNull() + }) + + it('re-arms sticky-bottom on click', () => { + const handler = vi.fn() + const stop = onScrollToBottomRequest(handler) + setThreadAtBottom(false) + render() + + fireEvent.click(screen.getByRole('button')) + + expect(handler).toHaveBeenCalledTimes(1) + stop() + }) +}) diff --git a/apps/desktop/src/app/chat/scroll-to-bottom-button.tsx b/apps/desktop/src/app/chat/scroll-to-bottom-button.tsx index b5d947ac1..dfe8b4e4d 100644 --- a/apps/desktop/src/app/chat/scroll-to-bottom-button.tsx +++ b/apps/desktop/src/app/chat/scroll-to-bottom-button.tsx @@ -5,6 +5,7 @@ import { Codicon } from '@/components/ui/codicon' import { useI18n } from '@/i18n' import { triggerHaptic } from '@/lib/haptics' import { cn } from '@/lib/utils' +import { $approvalRequest } from '@/store/prompts' import { $threadJumpButtonVisible, requestScrollToBottom } from '@/store/thread-scroll' /** @@ -15,6 +16,13 @@ import { $threadJumpButtonVisible, requestScrollToBottom } from '@/store/thread- * / background cards. Visible only while the user has scrolled meaningfully * away from the bottom; clicking re-arms sticky-bottom and pins the viewport. * + * When the turn is BLOCKED on an approval, this same control morphs into an + * "Approval needed" pill — the only response surface is the inline Run/Reject + * bar on the parked tool row, which is always the bottom-most content, so the + * existing scroll-to-bottom action lands the user right on it. One control, no + * collision, no second scroll path (native scrollIntoView would scroll + * overflow:hidden ancestors that can't scroll back and wreck the layout). + * * Enter/exit motion lives in styles.css under `.thread-jump-button` — a * directional scale (contract in from 1.1, contract out to 0.9) keyed off * `data-state`. `idle` (never-shown) stays silent so it can't flash on mount; @@ -23,6 +31,11 @@ import { $threadJumpButtonVisible, requestScrollToBottom } from '@/store/thread- export function ScrollToBottomButton() { const { t } = useI18n() const visible = useStore($threadJumpButtonVisible) + const request = useStore($approvalRequest) + // Scrolled away while an approval is pending → the inline Run/Reject bar is + // below the fold. Relabel so the user knows the session needs them, not just + // that there's more to read. + const approval = visible && Boolean(request) const hasShownRef = useRef(false) if (visible) { @@ -30,15 +43,17 @@ export function ScrollToBottomButton() { } const state = visible ? 'in' : hasShownRef.current ? 'out' : 'idle' + const label = approval ? t.assistant.approval.jumpToApproval : t.assistant.thread.scrollToBottom return ( ) } diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index dc8dfc727..d9739aab9 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -1694,6 +1694,7 @@ export const en: Translations = { moreOptions: 'More approval options', allowSession: 'Allow this session', alwaysAllowMenu: 'Always allow…', + jumpToApproval: 'Approval needed', reject: 'Reject', alwaysTitle: 'Always allow this command?', alwaysDescription: pattern => diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index cae9539bc..aa0b736b8 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -1834,6 +1834,7 @@ export const ja = defineLocale({ moreOptions: 'その他の承認オプション', allowSession: 'このセッションで許可', alwaysAllowMenu: '常に許可…', + jumpToApproval: '承認が必要', reject: '拒否', alwaysTitle: 'このコマンドを常に許可しますか？', alwaysDescription: pattern => diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index 44e03e87a..d89199c51 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -1353,6 +1353,7 @@ export interface Translations { moreOptions: string allowSession: string alwaysAllowMenu: string + jumpToApproval: string reject: string alwaysTitle: string alwaysDescription: (pattern: string) => string diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index e28091a9d..eda65ad6b 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -1778,6 +1778,7 @@ export const zhHant = defineLocale({ moreOptions: '更多核准選項', allowSession: '允許本工作階段', alwaysAllowMenu: '一律允許…', + jumpToApproval: '需要核准', reject: '拒絕', alwaysTitle: '一律允許此指令？', alwaysDescription: pattern => diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index 0c073ba1e..e1082deba 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -1874,6 +1874,7 @@ export const zh: Translations = { moreOptions: '更多审批选项', allowSession: '允许本会话', alwaysAllowMenu: '始终允许…', + jumpToApproval: '需要审批', reject: '拒绝', alwaysTitle: '始终允许此命令？', alwaysDescription: pattern =>