Skip to content

Commit a988da8

Browse files
committed
feat: improve status indicator and completion time display
- Show 'working...' shimmer text with elapsed time in status indicator - Swap credits and elapsed time order in message completion (credits first) - Document ShimmerText reconciliation issues with <box> in knowledge.md
1 parent b8946af commit a988da8

File tree

2 files changed

+77
-14
lines changed

2 files changed

+77
-14
lines changed

cli/knowledge.md

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,55 @@ The cleanest solution is to use a direct ternary with separate `<text>` elements
360360

361361
**Note:** Helper components like `ConditionalText` are not recommended as they add unnecessary abstraction without providing meaningful benefits. The direct ternary pattern is clearer and easier to maintain.
362362

363+
### Combining ShimmerText with Other Inline Elements
364+
365+
**Problem**: When you need to display multiple inline elements alongside a dynamically updating component like `ShimmerText` (e.g., showing elapsed time + shimmer text), using `<box>` causes reconciliation errors.
366+
367+
**Why `<box>` fails:**
368+
369+
```tsx
370+
// ❌ PROBLEMATIC: ShimmerText in a <box> with other elements causes reconciliation errors
371+
<box style={{ gap: 1 }}>
372+
<text fg={theme.secondary}>{elapsedSeconds}s</text>
373+
<text wrap={false}>
374+
<ShimmerText text="working..." />
375+
</text>
376+
</box>
377+
```
378+
379+
The issue occurs because:
380+
1. ShimmerText constantly updates its internal state (pulse animation)
381+
2. Each update re-renders with different `<span>` structures
382+
3. OpenTUI's reconciler struggles to match up the changing children inside the `<box>`
383+
4. Results in "Component of type 'span' must be created inside of a text node" error
384+
385+
**✅ Solution: Use a Fragment with inline spans**
386+
387+
Instead of using `<box>`, return a Fragment containing all inline elements:
388+
389+
```tsx
390+
// Component returns Fragment with inline elements
391+
if (elapsedSeconds > 0) {
392+
return (
393+
<>
394+
<span fg={theme.secondary}>{elapsedSeconds}s </span>
395+
<ShimmerText text="working..." />
396+
</>
397+
)
398+
}
399+
400+
// Parent wraps in <text>
401+
<text style={{ wrapMode: 'none' }}>{statusIndicatorNode}</text>
402+
```
403+
404+
**Key principles:**
405+
- Avoid wrapping dynamically updating components (like ShimmerText) in `<box>` elements
406+
- Use Fragments to group inline elements that will be wrapped in `<text>` by the parent
407+
- Include spacing as part of the text content (e.g., `"{elapsedSeconds}s "` with trailing space)
408+
- Let the parent component provide the `<text>` wrapper for proper rendering
409+
410+
This pattern works because all elements remain inline within a single stable `<text>` container, avoiding the reconciliation issues that occur when ShimmerText updates inside a `<box>`.
411+
363412
### The "Text Must Be Created Inside of a Text Node" Error
364413

365414
**Error message:**

cli/src/components/status-indicator.tsx

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import React, { useEffect, useState } from 'react'
22

3-
import { ElapsedTimer } from './elapsed-timer'
43
import { ShimmerText } from './shimmer-text'
54
import { useTheme } from '../hooks/use-theme'
65
import { getCodebuffClient } from '../utils/codebuff-client'
76

7+
import type { ElapsedTimeTracker } from '../hooks/use-elapsed-time'
8+
89
const useConnectionStatus = () => {
910
const [isConnected, setIsConnected] = useState(true)
1011

@@ -37,16 +38,17 @@ const useConnectionStatus = () => {
3738
export const StatusIndicator = ({
3839
clipboardMessage,
3940
isActive = false,
40-
timerStartTime,
41+
timer,
4142
nextCtrlCWillExit,
4243
}: {
4344
clipboardMessage?: string | null
4445
isActive?: boolean
45-
timerStartTime: number | null
46+
timer: ElapsedTimeTracker
4647
nextCtrlCWillExit: boolean
4748
}) => {
4849
const theme = useTheme()
4950
const isConnected = useConnectionStatus()
51+
const elapsedSeconds = timer.elapsedSeconds
5052

5153
if (nextCtrlCWillExit) {
5254
return <span fg={theme.secondary}>Press Ctrl-C again to exit</span>
@@ -67,16 +69,29 @@ export const StatusIndicator = ({
6769
}
6870

6971
if (isActive) {
70-
if (!timerStartTime || Date.now() - timerStartTime < 1000) {
72+
// If we have elapsed time > 0, show it with "working..."
73+
if (elapsedSeconds > 0) {
7174
return (
72-
<ShimmerText
73-
text="thinking..."
74-
interval={160}
75-
primaryColor={theme.secondary}
76-
/>
75+
<>
76+
<ShimmerText
77+
text="working..."
78+
interval={160}
79+
primaryColor={theme.secondary}
80+
/>
81+
<span fg={theme.muted}> </span>
82+
<span fg={theme.secondary}>{elapsedSeconds}s</span>
83+
</>
7784
)
7885
}
79-
return <ElapsedTimer startTime={timerStartTime} />
86+
87+
// Otherwise show thinking...
88+
return (
89+
<ShimmerText
90+
text="thinking..."
91+
interval={160}
92+
primaryColor={theme.secondary}
93+
/>
94+
)
8095
}
8196

8297
return null
@@ -85,18 +100,17 @@ export const StatusIndicator = ({
85100
export const useHasStatus = (params: {
86101
isActive: boolean
87102
clipboardMessage?: string | null
88-
timerStartTime?: number | null
103+
timer?: ElapsedTimeTracker
89104
nextCtrlCWillExit: boolean
90105
}): boolean => {
91-
const { isActive, clipboardMessage, timerStartTime, nextCtrlCWillExit } =
92-
params
106+
const { isActive, clipboardMessage, timer, nextCtrlCWillExit } = params
93107

94108
const isConnected = useConnectionStatus()
95109
return (
96110
isConnected === false ||
97111
isActive ||
98112
Boolean(clipboardMessage) ||
99-
Boolean(timerStartTime) ||
113+
Boolean(timer?.startTime) ||
100114
nextCtrlCWillExit
101115
)
102116
}

0 commit comments

Comments
 (0)