Skip to content

Commit 42385ae

Browse files
committed
refactor: improve code quality based on review feedback
- Add JSDoc documentation for soft reload mechanism - Use proper TypeScript typing instead of 'as any' - Extract STATE_UPDATE_DEBOUNCE_MS as named constant - Add detailed comments explaining the flickering prevention mechanism
1 parent 7eaa8e4 commit 42385ae

File tree

5 files changed

+54
-9
lines changed

5 files changed

+54
-9
lines changed

src/core/webview/ClineProvider.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,26 @@ export class ClineProvider
141141
private recentTasksCache?: string[]
142142
private pendingOperations: Map<string, PendingEditOperation> = new Map()
143143
private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds
144-
private isSoftReloading = false // Flag to indicate soft reload state (cancel/checkpoint restore)
145-
private stateUpdateDebounceTimer: NodeJS.Timeout | null = null // Debounce timer for state updates
144+
private static readonly STATE_UPDATE_DEBOUNCE_MS = 50 // Default debounce delay for state updates
145+
146+
/**
147+
* Soft reload mechanism to prevent UI flickering during task recreation.
148+
* When true, the UI preserves its state (scroll position, input values) during updates.
149+
* This is used during cancel operations and checkpoint restoration to maintain UI stability.
150+
*/
151+
public isSoftReloading = false
152+
153+
/**
154+
* Debounce timer for state updates to prevent rapid consecutive updates
155+
* that can cause UI flickering and performance issues.
156+
*/
157+
private stateUpdateDebounceTimer: NodeJS.Timeout | null = null
158+
159+
/**
160+
* Configurable debounce delay in milliseconds, mainly for testing purposes.
161+
* In production, this uses STATE_UPDATE_DEBOUNCE_MS.
162+
*/
163+
private stateUpdateDebounceDelay: number = ClineProvider.STATE_UPDATE_DEBOUNCE_MS
146164

147165
public isViewLaunched = false
148166
public settingsImportedAt?: number
@@ -1634,6 +1652,24 @@ export class ClineProvider
16341652
return
16351653
}
16361654

1655+
// If debounce delay is 0 (for tests), execute immediately
1656+
if (this.stateUpdateDebounceDelay === 0) {
1657+
const state = await this.getStateToPostToWebview()
1658+
// Include soft reload flag to prevent UI flickering
1659+
this.postMessageToWebview({
1660+
type: "state",
1661+
state,
1662+
isSoftReload: this.isSoftReloading,
1663+
})
1664+
1665+
// Check MDM compliance and send user to account tab if not compliant
1666+
// Only redirect if there's an actual MDM policy requiring authentication
1667+
if (this.mdmService?.requiresCloudAuth() && !this.checkMdmCompliance()) {
1668+
await this.postMessageToWebview({ type: "action", action: "cloudButtonClicked" })
1669+
}
1670+
return
1671+
}
1672+
16371673
// Debounce state updates to prevent rapid flickering
16381674
this.stateUpdateDebounceTimer = setTimeout(async () => {
16391675
const state = await this.getStateToPostToWebview()
@@ -1651,7 +1687,7 @@ export class ClineProvider
16511687
}
16521688

16531689
this.stateUpdateDebounceTimer = null
1654-
}, 50) // 50ms debounce delay
1690+
}, this.stateUpdateDebounceDelay)
16551691
}
16561692

16571693
/**

src/core/webview/__tests__/ClineProvider.spec.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,10 @@ describe("ClineProvider", () => {
418418
onDidChangeVisibility: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })),
419419
} as unknown as vscode.WebviewView
420420

421+
// Create provider with immediate state updates for tests (no debouncing)
421422
provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
423+
// Set debounce delay to 0 for tests to ensure synchronous behavior
424+
;(provider as any).stateUpdateDebounceDelay = 0
422425

423426
defaultTaskOptions = {
424427
provider,

src/core/webview/__tests__/checkpointRestoreHandler.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ describe("checkpointRestoreHandler", () => {
4848
mockProvider = {
4949
getCurrentTask: vi.fn(() => mockCline),
5050
postMessageToWebview: vi.fn(),
51+
postStateToWebview: vi.fn().mockResolvedValue(undefined),
5152
getTaskWithId: vi.fn(() => ({
5253
historyItem: { id: "test-task-123", messages: mockCline.clineMessages },
5354
})),
@@ -56,6 +57,7 @@ describe("checkpointRestoreHandler", () => {
5657
contextProxy: {
5758
globalStorageUri: { fsPath: "/test/storage" },
5859
},
60+
isSoftReloading: false,
5961
}
6062

6163
// Mock pWaitFor to resolve immediately

src/core/webview/checkpointRestoreHandler.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,17 @@ export interface CheckpointRestoreConfig {
2222
/**
2323
* Handles checkpoint restoration for both delete and edit operations.
2424
* This consolidates the common logic while handling operation-specific behavior.
25+
*
26+
* The soft reload mechanism prevents UI flickering by maintaining state during
27+
* checkpoint restoration operations. This ensures the chat window doesn't flash
28+
* or lose scroll position when restoring to a previous checkpoint.
2529
*/
2630
export async function handleCheckpointRestoreOperation(config: CheckpointRestoreConfig): Promise<void> {
2731
const { provider, currentCline, messageTs, checkpoint, operation, editData } = config
2832

2933
try {
30-
// Set soft reload flag to prevent UI flickering
31-
;(provider as any).isSoftReloading = true
34+
// Set soft reload flag to prevent UI flickering during checkpoint restoration
35+
provider.isSoftReloading = true
3236

3337
// For delete operations, ensure the task is properly aborted to handle any pending ask operations
3438
// This prevents "Current ask promise was ignored" errors
@@ -83,13 +87,13 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore
8387
// will trigger reinitialization, which will process pendingEditAfterRestore
8488

8589
// Reset soft reload flag after operation completes
86-
;(provider as any).isSoftReloading = false
90+
provider.isSoftReloading = false
8791

8892
// Send a refresh without flickering
8993
await provider.postStateToWebview()
9094
} catch (error) {
9195
// Reset soft reload flag on error
92-
;(provider as any).isSoftReloading = false
96+
provider.isSoftReloading = false
9397

9498
console.error(`Error in checkpoint restore (${operation}):`, error)
9599
vscode.window.showErrorMessage(

webview-ui/src/components/chat/ChatView.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -728,8 +728,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
728728
const trimmedInput = text?.trim()
729729

730730
if (isStreaming) {
731-
// Set a flag to indicate soft reload for cancel operation
732-
vscode.postMessage({ type: "cancelTask", isSoftReload: true })
731+
// Cancel the streaming task
732+
vscode.postMessage({ type: "cancelTask" })
733733
setDidClickCancel(true)
734734
return
735735
}

0 commit comments

Comments
 (0)