fix(desktop): scroll Approval Needed to the pending approval, not the bottom

Fixes #115538

ScrollToBottomButton always requested a full jump-to-bottom, including
while labeled "Approval Needed". PendingApprovalStack is decoupled from
the message that requested it and can end up above newer transcript
content, so a bottom jump can overshoot it and leave the user with
nothing to approve at the destination.

Tag PendingApprovalStack with the owning session id and, when an
approval is pending, scroll directly to that element (scoped by
session so a split view can't jump into a sibling pane's approval)
instead of requesting a bottom jump.
This commit is contained in:
chelsealong
2026-09-19 04:19:57 +00:00
committed by Teknium
parent 93537ddb77
commit 8c2f9ca9bb
3 changed files with 88 additions and 2 deletions

View File

@@ -27,6 +27,10 @@ afterEach(() => {
}
$activeSessionId.set(null)
for (const stack of document.querySelectorAll('[data-approval-stack]')) {
stack.remove()
}
})
// `getByRole('button')` excludes aria-hidden nodes, so "queryByRole null" is the
@@ -144,4 +148,52 @@ describe('ScrollToBottomButton', () => {
expect(handler).toHaveBeenCalledTimes(1)
stop()
})
it('scrolls to the session approval stack instead of the bottom when one is pending', () => {
pendingApproval()
setThreadAtBottom(false, 'sess-1')
const bottomHandler = vi.fn()
const stopBottom = onScrollToBottomRequest(bottomHandler, 'sess-1')
const stack = document.createElement('div')
stack.setAttribute('data-approval-stack', '')
stack.setAttribute('data-session-id', 'sess-1')
const scrollIntoView = vi.fn()
stack.scrollIntoView = scrollIntoView
document.body.appendChild(stack)
render(<ScrollToBottomButton sessionId="sess-1" />)
fireEvent.click(screen.getByRole('button', { name: 'Approval needed' }))
expect(scrollIntoView).toHaveBeenCalledWith({ block: 'nearest' })
expect(bottomHandler).not.toHaveBeenCalled()
stopBottom()
stack.remove()
})
it('does not jump into a sibling session’s approval stack', () => {
pendingApproval()
setThreadAtBottom(false, 'sess-1')
const otherStack = document.createElement('div')
otherStack.setAttribute('data-approval-stack', '')
otherStack.setAttribute('data-session-id', 'sess-other')
const otherScrollIntoView = vi.fn()
otherStack.scrollIntoView = otherScrollIntoView
document.body.appendChild(otherStack)
const bottomHandler = vi.fn()
const stopBottom = onScrollToBottomRequest(bottomHandler, 'sess-1')
render(<ScrollToBottomButton sessionId="sess-1" />)
fireEvent.click(screen.getByRole('button', { name: 'Approval needed' }))
expect(otherScrollIntoView).not.toHaveBeenCalled()
// No stack tagged for this session exists, so it falls back to the bottom.
expect(bottomHandler).toHaveBeenCalledTimes(1)
stopBottom()
otherStack.remove()
})
})

View File

@@ -18,6 +18,28 @@ import {
import { useComposerSurfaceId } from './composer/scope'
// Attribute-safe selector fragment. jsdom (vitest) does not ship `CSS.escape`.
const cssEscape = (value: string): string => {
if (typeof CSS !== 'undefined' && typeof CSS.escape === 'function') {
return CSS.escape(value)
}
return value.replace(/[^a-zA-Z0-9_:-]/g, ch => `\\${ch}`)
}
// The pending-approval stack renders once per pane, tagged with the owning
// session so a split view can't jump one pane's arrow into a sibling's
// approval. `nearest` keeps this a minimal scroll within the transcript's own
// scroll container instead of an unqualified scrollIntoView, which would also
// nudge any overflow-hidden ancestor's programmatic scroll offset.
function findSessionApprovalStack(sessionId: string | null): HTMLElement | null {
if (!sessionId) {
return null
}
return document.querySelector<HTMLElement>(`[data-approval-stack][data-session-id="${cssEscape(sessionId)}"]`)
}
/**
* Floating "jump to bottom" control. Sits centered just above the composer,
* clearing the out-of-flow status stack via the same measured-height CSS vars
@@ -27,8 +49,10 @@ import { useComposerSurfaceId } from './composer/scope'
* away from the bottom, with an animated count of messages below the viewport.
* Clicking re-arms sticky-bottom and pins the viewport.
*
* While approvals are pending, relabel this control to lead back to the
* transcript-owned stack using the existing scroll path.
* While an approval is pending, relabel this control and, instead of jumping
* to the transcript's true bottom (which can overshoot a mid-transcript
* approval once newer content lands below it), scroll directly to the
* session's own approval stack.
*
* 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
@@ -86,6 +110,15 @@ export function ScrollToBottomButton({ sessionId }: { sessionId: string | null }
data-state={state}
onClick={() => {
triggerHaptic('selection')
const approvalStack = visibleApproval ? findSessionApprovalStack(request?.sessionId ?? null) : null
if (approvalStack) {
approvalStack.scrollIntoView({ block: 'nearest' })
return
}
requestScrollToBottom(scrollSessionId)
}}
style={{

View File

@@ -64,6 +64,7 @@ export const PendingApprovalStack: FC = () => {
)}
data-approval-placement={placement}
data-approval-stack=""
data-session-id={sessionId ?? undefined}
data-slot="tool-approval-stack"
initial={false}
transition={reduced || requests.length ? { duration: 0 } : { duration: 0.22, ease: 'easeInOut' }}