Skip to content

Commit 8584e25

Browse files
committed
refactor: simplify collapse handling with callback wrapper pattern
Replace prop drilling of isUserCollapsingRef with a callback wrapper pattern. Created setCollapsedAgentsWithFlag wrapper that automatically sets the flag before updating state, eliminating the need to pass the ref through multiple component layers. Changes: - Add setCollapsedAgentsWithFlag and handleCollapseToggle in chat.tsx - Remove isUserCollapsingRef prop from MessageRenderer, MessageBlock, and AgentBranchItem - Update use-message-renderer to use timerStartTime consistently - Fix status indicator tests to use proper ElapsedTimeTracker mock
1 parent 5f9a28b commit 8584e25

File tree

5 files changed

+57
-66
lines changed

5 files changed

+57
-66
lines changed

cli/src/chat.tsx

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,42 @@ export const Chat = ({
228228
isUserCollapsingRef.current = false
229229
}, [collapsedAgents])
230230

231+
// Wrapper for setCollapsedAgents that sets the flag to prevent auto-scroll
232+
const setCollapsedAgentsWithFlag = useCallback(
233+
(action: React.SetStateAction<Set<string>>) => {
234+
isUserCollapsingRef.current = true
235+
setCollapsedAgents(action)
236+
},
237+
[setCollapsedAgents],
238+
)
239+
240+
const handleCollapseToggle = useCallback(
241+
(id: string) => {
242+
const wasCollapsed = collapsedAgents.has(id)
243+
244+
setCollapsedAgentsWithFlag((prev) => {
245+
const next = new Set(prev)
246+
if (next.has(id)) {
247+
next.delete(id)
248+
} else {
249+
next.add(id)
250+
}
251+
return next
252+
})
253+
254+
setUserOpenedAgents((prev) => {
255+
const next = new Set(prev)
256+
if (wasCollapsed) {
257+
next.add(id)
258+
} else {
259+
next.delete(id)
260+
}
261+
return next
262+
})
263+
},
264+
[collapsedAgents, setCollapsedAgentsWithFlag, setUserOpenedAgents],
265+
)
266+
231267
const { scrollToLatest, scrollboxProps, isAtBottom } = useChatScrollbox(
232268
scrollRef,
233269
messages,
@@ -592,11 +628,11 @@ export const Chat = ({
592628
streamingAgents={streamingAgents}
593629
isWaitingForResponse={isWaitingForResponse}
594630
timerStartTime={timerStartTime}
595-
setCollapsedAgents={setCollapsedAgents}
631+
onCollapseToggle={handleCollapseToggle}
632+
setCollapsedAgents={setCollapsedAgentsWithFlag}
596633
setFocusedAgentId={setFocusedAgentId}
597634
userOpenedAgents={userOpenedAgents}
598635
setUserOpenedAgents={setUserOpenedAgents}
599-
isUserCollapsingRef={isUserCollapsingRef}
600636
onBuildFast={handleBuildFast}
601637
onBuildMax={handleBuildMax}
602638
/>

cli/src/components/__tests__/status-indicator.timer.test.tsx

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@ import '../../state/theme-store' // Initialize theme store
1515
import { renderToStaticMarkup } from 'react-dom/server'
1616

1717
import * as codebuffClient from '../../utils/codebuff-client'
18+
import type { ElapsedTimeTracker } from '../../hooks/use-elapsed-time'
1819

19-
const createTimerStartTime = (
20+
const createMockTimer = (
2021
elapsedSeconds: number,
2122
started: boolean,
22-
): number | null =>
23-
started ? Date.now() - elapsedSeconds * 1000 : null
23+
): ElapsedTimeTracker => ({
24+
startTime: started ? Date.now() - elapsedSeconds * 1000 : null,
25+
elapsedSeconds,
26+
start: () => {},
27+
stop: () => {},
28+
})
2429

2530
describe('StatusIndicator timer rendering', () => {
2631
let getClientSpy: ReturnType<typeof spyOn>
@@ -40,7 +45,7 @@ describe('StatusIndicator timer rendering', () => {
4045
<StatusIndicator
4146
clipboardMessage={null}
4247
isActive={true}
43-
timerStartTime={createTimerStartTime(5, true)}
48+
timer={createMockTimer(5, true)}
4449
nextCtrlCWillExit={false}
4550
/>,
4651
)
@@ -51,7 +56,7 @@ describe('StatusIndicator timer rendering', () => {
5156
<StatusIndicator
5257
clipboardMessage={null}
5358
isActive={false}
54-
timerStartTime={createTimerStartTime(0, false)}
59+
timer={createMockTimer(0, false)}
5560
nextCtrlCWillExit={false}
5661
/>,
5762
)
@@ -64,7 +69,7 @@ describe('StatusIndicator timer rendering', () => {
6469
<StatusIndicator
6570
clipboardMessage="Copied!"
6671
isActive={true}
67-
timerStartTime={createTimerStartTime(12, true)}
72+
timer={createMockTimer(12, true)}
6873
nextCtrlCWillExit={false}
6974
/>,
7075
)

cli/src/components/agent-branch-item.tsx

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ interface AgentBranchItemProps {
1818
statusIndicator?: string
1919
onToggle?: () => void
2020
titleSuffix?: string
21-
isUserCollapsingRef?: React.MutableRefObject<boolean>
2221
}
2322

2423
export const AgentBranchItem = ({
@@ -35,7 +34,6 @@ export const AgentBranchItem = ({
3534
statusIndicator = '●',
3635
onToggle,
3736
titleSuffix,
38-
isUserCollapsingRef,
3937
}: AgentBranchItemProps) => {
4038
const theme = useTheme()
4139

@@ -289,13 +287,7 @@ export const AgentBranchItem = ({
289287
alignSelf: 'flex-end',
290288
marginTop: 1,
291289
}}
292-
onMouseDown={() => {
293-
// Set flag to prevent auto-scroll during user-initiated collapse
294-
if (isUserCollapsingRef) {
295-
isUserCollapsingRef.current = true
296-
}
297-
onToggle()
298-
}}
290+
onMouseDown={onToggle}
299291
>
300292
<text
301293
fg={theme.secondary}

cli/src/components/message-renderer.tsx

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ interface MessageRendererProps {
2525
streamingAgents: Set<string>
2626
isWaitingForResponse: boolean
2727
timerStartTime: number | null
28+
onCollapseToggle: (id: string) => void
2829
setCollapsedAgents: React.Dispatch<React.SetStateAction<Set<string>>>
2930
setFocusedAgentId: React.Dispatch<React.SetStateAction<string | null>>
3031
userOpenedAgents: Set<string>
3132
setUserOpenedAgents: React.Dispatch<React.SetStateAction<Set<string>>>
32-
isUserCollapsingRef: React.MutableRefObject<boolean>
3333
onBuildFast: () => void
3434
onBuildMax: () => void
3535
}
@@ -46,43 +46,14 @@ export const MessageRenderer = (props: MessageRendererProps): ReactNode => {
4646
streamingAgents,
4747
isWaitingForResponse,
4848
timerStartTime,
49+
onCollapseToggle,
4950
setCollapsedAgents,
5051
setFocusedAgentId,
5152
setUserOpenedAgents,
52-
isUserCollapsingRef,
5353
onBuildFast,
5454
onBuildMax,
5555
} = props
5656

57-
const onToggleCollapsed = useCallback(
58-
(id: string) => {
59-
const wasCollapsed = collapsedAgents.has(id)
60-
61-
// Set flag to prevent auto-scroll during user-initiated collapse
62-
isUserCollapsingRef.current = true
63-
64-
setCollapsedAgents((prev) => {
65-
const next = new Set(prev)
66-
if (next.has(id)) {
67-
next.delete(id)
68-
} else {
69-
next.add(id)
70-
}
71-
return next
72-
})
73-
setUserOpenedAgents((prev) => {
74-
const next = new Set(prev)
75-
if (wasCollapsed) {
76-
next.add(id)
77-
} else {
78-
next.delete(id)
79-
}
80-
return next
81-
})
82-
},
83-
[collapsedAgents, setCollapsedAgents, setUserOpenedAgents, isUserCollapsingRef],
84-
)
85-
8657
return (
8758
<>
8859
{topLevelMessages.map((message, idx) => {
@@ -105,7 +76,7 @@ export const MessageRenderer = (props: MessageRendererProps): ReactNode => {
10576
setFocusedAgentId={setFocusedAgentId}
10677
isWaitingForResponse={isWaitingForResponse}
10778
timerStartTime={timerStartTime}
108-
onToggleCollapsed={onToggleCollapsed}
79+
onToggleCollapsed={onCollapseToggle}
10980
onBuildFast={onBuildFast}
11081
onBuildMax={onBuildMax}
11182
/>

cli/src/hooks/use-message-renderer.tsx

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ interface UseMessageRendererProps {
3030
setFocusedAgentId: React.Dispatch<React.SetStateAction<string | null>>
3131
userOpenedAgents: Set<string>
3232
setUserOpenedAgents: React.Dispatch<React.SetStateAction<Set<string>>>
33-
isUserCollapsingRef: React.MutableRefObject<boolean>
3433
}
3534

3635
export const useMessageRenderer = (
@@ -51,7 +50,6 @@ export const useMessageRenderer = (
5150
setFocusedAgentId,
5251
userOpenedAgents,
5352
setUserOpenedAgents,
54-
isUserCollapsingRef,
5553
} = props
5654

5755
return useMemo(() => {
@@ -103,9 +101,6 @@ export const useMessageRenderer = (
103101

104102
const wasCollapsed = collapsedAgents.has(message.id)
105103

106-
// Set flag to prevent auto-scroll during user-initiated collapse
107-
isUserCollapsingRef.current = true
108-
109104
setCollapsedAgents((prev) => {
110105
const next = new Set(prev)
111106

@@ -381,7 +376,7 @@ export const useMessageRenderer = (
381376
isComplete={message.isComplete}
382377
completionTime={message.completionTime}
383378
credits={message.credits}
384-
timer={timer}
379+
timerStartTime={timer.startTime}
385380
textColor={textColor}
386381
timestampColor={timestampColor}
387382
markdownOptions={markdownOptions}
@@ -391,10 +386,7 @@ export const useMessageRenderer = (
391386
streamingAgents={streamingAgents}
392387
onToggleCollapsed={(id: string) => {
393388
const wasCollapsed = collapsedAgents.has(id)
394-
395-
// Set flag to prevent auto-scroll during user-initiated collapse
396-
isUserCollapsingRef.current = true
397-
389+
398390
setCollapsedAgents((prev) => {
399391
const next = new Set(prev)
400392
if (next.has(id)) {
@@ -447,7 +439,7 @@ export const useMessageRenderer = (
447439
isComplete={message.isComplete}
448440
completionTime={message.completionTime}
449441
credits={message.credits}
450-
timer={timer}
442+
timerStartTime={timer.startTime}
451443
textColor={textColor}
452444
timestampColor={timestampColor}
453445
markdownOptions={markdownOptions}
@@ -457,10 +449,7 @@ export const useMessageRenderer = (
457449
streamingAgents={streamingAgents}
458450
onToggleCollapsed={(id: string) => {
459451
const wasCollapsed = collapsedAgents.has(id)
460-
461-
// Set flag to prevent auto-scroll during user-initiated collapse
462-
isUserCollapsingRef.current = true
463-
452+
464453
setCollapsedAgents((prev) => {
465454
const next = new Set(prev)
466455
if (next.has(id)) {
@@ -484,7 +473,6 @@ export const useMessageRenderer = (
484473
return next
485474
})
486475
}}
487-
isUserCollapsingRef={isUserCollapsingRef}
488476
/>
489477
</box>
490478
)}
@@ -524,6 +512,5 @@ export const useMessageRenderer = (
524512
setUserOpenedAgents,
525513
setFocusedAgentId,
526514
userOpenedAgents,
527-
isUserCollapsingRef,
528515
])
529516
}

0 commit comments

Comments
 (0)