mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
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
This commit is contained in:
parent
7eaa8e4c12
commit
42385ae9f1
5 changed files with 54 additions and 9 deletions
|
|
@ -141,8 +141,26 @@ export class ClineProvider
|
|||
private recentTasksCache?: string[]
|
||||
private pendingOperations: Map<string, PendingEditOperation> = new Map()
|
||||
private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds
|
||||
private isSoftReloading = false // Flag to indicate soft reload state (cancel/checkpoint restore)
|
||||
private stateUpdateDebounceTimer: NodeJS.Timeout | null = null // Debounce timer for state updates
|
||||
private static readonly STATE_UPDATE_DEBOUNCE_MS = 50 // Default debounce delay for state updates
|
||||
|
||||
/**
|
||||
* Soft reload mechanism to prevent UI flickering during task recreation.
|
||||
* When true, the UI preserves its state (scroll position, input values) during updates.
|
||||
* This is used during cancel operations and checkpoint restoration to maintain UI stability.
|
||||
*/
|
||||
public isSoftReloading = false
|
||||
|
||||
/**
|
||||
* Debounce timer for state updates to prevent rapid consecutive updates
|
||||
* that can cause UI flickering and performance issues.
|
||||
*/
|
||||
private stateUpdateDebounceTimer: NodeJS.Timeout | null = null
|
||||
|
||||
/**
|
||||
* Configurable debounce delay in milliseconds, mainly for testing purposes.
|
||||
* In production, this uses STATE_UPDATE_DEBOUNCE_MS.
|
||||
*/
|
||||
private stateUpdateDebounceDelay: number = ClineProvider.STATE_UPDATE_DEBOUNCE_MS
|
||||
|
||||
public isViewLaunched = false
|
||||
public settingsImportedAt?: number
|
||||
|
|
@ -1634,6 +1652,24 @@ export class ClineProvider
|
|||
return
|
||||
}
|
||||
|
||||
// If debounce delay is 0 (for tests), execute immediately
|
||||
if (this.stateUpdateDebounceDelay === 0) {
|
||||
const state = await this.getStateToPostToWebview()
|
||||
// Include soft reload flag to prevent UI flickering
|
||||
this.postMessageToWebview({
|
||||
type: "state",
|
||||
state,
|
||||
isSoftReload: this.isSoftReloading,
|
||||
})
|
||||
|
||||
// Check MDM compliance and send user to account tab if not compliant
|
||||
// Only redirect if there's an actual MDM policy requiring authentication
|
||||
if (this.mdmService?.requiresCloudAuth() && !this.checkMdmCompliance()) {
|
||||
await this.postMessageToWebview({ type: "action", action: "cloudButtonClicked" })
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// Debounce state updates to prevent rapid flickering
|
||||
this.stateUpdateDebounceTimer = setTimeout(async () => {
|
||||
const state = await this.getStateToPostToWebview()
|
||||
|
|
@ -1651,7 +1687,7 @@ export class ClineProvider
|
|||
}
|
||||
|
||||
this.stateUpdateDebounceTimer = null
|
||||
}, 50) // 50ms debounce delay
|
||||
}, this.stateUpdateDebounceDelay)
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -418,7 +418,10 @@ describe("ClineProvider", () => {
|
|||
onDidChangeVisibility: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })),
|
||||
} as unknown as vscode.WebviewView
|
||||
|
||||
// Create provider with immediate state updates for tests (no debouncing)
|
||||
provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
|
||||
// Set debounce delay to 0 for tests to ensure synchronous behavior
|
||||
;(provider as any).stateUpdateDebounceDelay = 0
|
||||
|
||||
defaultTaskOptions = {
|
||||
provider,
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ describe("checkpointRestoreHandler", () => {
|
|||
mockProvider = {
|
||||
getCurrentTask: vi.fn(() => mockCline),
|
||||
postMessageToWebview: vi.fn(),
|
||||
postStateToWebview: vi.fn().mockResolvedValue(undefined),
|
||||
getTaskWithId: vi.fn(() => ({
|
||||
historyItem: { id: "test-task-123", messages: mockCline.clineMessages },
|
||||
})),
|
||||
|
|
@ -56,6 +57,7 @@ describe("checkpointRestoreHandler", () => {
|
|||
contextProxy: {
|
||||
globalStorageUri: { fsPath: "/test/storage" },
|
||||
},
|
||||
isSoftReloading: false,
|
||||
}
|
||||
|
||||
// Mock pWaitFor to resolve immediately
|
||||
|
|
|
|||
|
|
@ -22,13 +22,17 @@ export interface CheckpointRestoreConfig {
|
|||
/**
|
||||
* Handles checkpoint restoration for both delete and edit operations.
|
||||
* This consolidates the common logic while handling operation-specific behavior.
|
||||
*
|
||||
* The soft reload mechanism prevents UI flickering by maintaining state during
|
||||
* checkpoint restoration operations. This ensures the chat window doesn't flash
|
||||
* or lose scroll position when restoring to a previous checkpoint.
|
||||
*/
|
||||
export async function handleCheckpointRestoreOperation(config: CheckpointRestoreConfig): Promise<void> {
|
||||
const { provider, currentCline, messageTs, checkpoint, operation, editData } = config
|
||||
|
||||
try {
|
||||
// Set soft reload flag to prevent UI flickering
|
||||
;(provider as any).isSoftReloading = true
|
||||
// Set soft reload flag to prevent UI flickering during checkpoint restoration
|
||||
provider.isSoftReloading = true
|
||||
|
||||
// For delete operations, ensure the task is properly aborted to handle any pending ask operations
|
||||
// This prevents "Current ask promise was ignored" errors
|
||||
|
|
@ -83,13 +87,13 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore
|
|||
// will trigger reinitialization, which will process pendingEditAfterRestore
|
||||
|
||||
// Reset soft reload flag after operation completes
|
||||
;(provider as any).isSoftReloading = false
|
||||
provider.isSoftReloading = false
|
||||
|
||||
// Send a refresh without flickering
|
||||
await provider.postStateToWebview()
|
||||
} catch (error) {
|
||||
// Reset soft reload flag on error
|
||||
;(provider as any).isSoftReloading = false
|
||||
provider.isSoftReloading = false
|
||||
|
||||
console.error(`Error in checkpoint restore (${operation}):`, error)
|
||||
vscode.window.showErrorMessage(
|
||||
|
|
|
|||
|
|
@ -728,8 +728,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
|
|||
const trimmedInput = text?.trim()
|
||||
|
||||
if (isStreaming) {
|
||||
// Set a flag to indicate soft reload for cancel operation
|
||||
vscode.postMessage({ type: "cancelTask", isSoftReload: true })
|
||||
// Cancel the streaming task
|
||||
vscode.postMessage({ type: "cancelTask" })
|
||||
setDidClickCancel(true)
|
||||
return
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue