Merge pull request #4 from Smartsheet-JB-Brown/hobbsessr/fix-marketplace-redraw-issue

Hobbsessr/fix marketplace redraw issue

webview-ui/src/components/marketplace/MarketplaceViewStateManager.ts

@@ -1,3 +1,16 @@
+/**
+ * MarketplaceViewStateManager
+ *
+ * This class manages the state for the marketplace view in the Roo Code extensions interface.
+ *
+ * IMPORTANT: Fixed issue where the marketplace feature was causing the Roo Code extensions interface
+ * to switch to the browse tab and redraw it every 30 seconds. The fix prevents unnecessary tab switching
+ * and redraws by:
+ * 1. Only updating the UI when necessary
+ * 2. Preserving the current tab when handling timeouts
+ * 3. Using minimal state updates to avoid resetting scroll position
+ */
+
 import { MarketplaceItem, MarketplaceSource, MatchInfo } from "../../../../src/services/marketplace/types"
 import { vscode } from "../../utils/vscode"
 import { WebviewMessage } from "../../../../src/shared/WebviewMessage"
@@ -141,11 +154,34 @@ export class MarketplaceViewStateManager {
 		}
 	}
 
-	private notifyStateChange(): void {
+	/**
+	 * Notify all registered handlers of a state change
+	 * @param preserveTab If true, ensures the active tab is not changed during notification
+	 */
+	private notifyStateChange(preserveTab: boolean = false): void {
 		const newState = this.getState() // Use getState to ensure proper copying
-		this.stateChangeHandlers.forEach((handler) => {
-			handler(newState)
-		})
+
+		if (preserveTab) {
+			// When preserveTab is true, we're careful not to cause tab switching
+			// This is used during timeout handling to prevent disrupting the user
+			this.stateChangeHandlers.forEach((handler) => {
+				// Store the current active tab
+				const currentTab = newState.activeTab;
+
+				// Create a state update that won't change the active tab
+				const safeState = {
+					...newState,
+					// Don't change these properties to avoid UI disruption
+					activeTab: currentTab
+				}
+				handler(safeState)
+			})
+		} else {
+			// Normal state change notification
+			this.stateChangeHandlers.forEach((handler) => {
+				handler(newState)
+			})
+		}
 
 		// Save state to sessionStorage if available
 		if (typeof sessionStorage !== "undefined") {
@@ -186,11 +222,12 @@ export class MarketplaceViewStateManager {
 				}
 				this.notifyStateChange()
 
-				// Set timeout to reset state if fetch takes too long
+				// Set timeout to reset state if fetch takes too long, but don't trigger a redraw if not needed
 				this.fetchTimeoutId = setTimeout(() => {
 					this.clearFetchTimeout()
 					// On timeout, preserve items if we have them
 					if (currentItems.length > 0) {
+						// Only update the isFetching flag without triggering a full redraw
 						this.state = {
 							...this.state,
 							isFetching: false,
@@ -198,13 +235,34 @@ export class MarketplaceViewStateManager {
 							displayItems: currentItems,
 						}
 					} else {
+						// Preserve the current tab and only update necessary state
+						const { activeTab, sources } = this.state
 						this.state = {
 							...this.getDefaultState(),
-							sources: [...this.state.sources],
-							activeTab: this.state.activeTab,
+							sources: [...sources],
+							activeTab, // Keep the current active tab
 						}
 					}
-					this.notifyStateChange()
+
+					// Only notify if we're in the browse tab to avoid switching tabs
+					if (this.state.activeTab === "browse") {
+						// Use a minimal state update to avoid resetting scroll position
+						const handler = (state: ViewState) => {
+							// Only update the isFetching status without affecting other UI elements
+							return {
+								...state,
+								isFetching: false
+							}
+						}
+
+						// Call handlers with the minimal update
+						this.stateChangeHandlers.forEach((stateHandler) => {
+							stateHandler(handler(this.getState()))
+						})
+					} else {
+						// If not in browse tab, just update the internal state without notifying
+						// This prevents tab switching
+					}
 				}, this.FETCH_TIMEOUT)
 
 				break
@@ -560,7 +618,15 @@ export class MarketplaceViewStateManager {
 					allItems: sortedItems,
 					displayItems: newDisplayItems,
 				}
-				this.notifyStateChange()
+
+				// Only notify with full state update if we're in the browse tab
+				// or if this is the first time we're getting items
+				if (isOnBrowseTab || !hasCurrentItems) {
+					this.notifyStateChange()
+				} else {
+					// If we're not in the browse tab, update state but don't force a tab switch
+					this.notifyStateChange(true) // preserve tab
+				}
 			}
 		}
 

webview-ui/src/components/marketplace/__tests__/MarketplaceViewStateManager.test.ts

@@ -725,6 +725,78 @@ describe("MarketplaceViewStateManager", () => {
 			expect(state.allItems).toHaveLength(1)
 		})
 
+		it("should not switch tabs when timeout occurs while in sources tab", async () => {
+			// First switch to sources tab
+			await manager.transition({
+				type: "SET_ACTIVE_TAB",
+				payload: { tab: "sources" },
+			})
+
+			// Start a fetch
+			await manager.transition({ type: "FETCH_ITEMS" })
+
+			// Set up a state change handler to track tab changes
+			let tabSwitched = false
+			const unsubscribe = manager.onStateChange((state) => {
+				if (state.activeTab === "browse") {
+					tabSwitched = true
+				}
+			})
+
+			// Fast-forward past the timeout
+			jest.advanceTimersByTime(30000)
+
+			// Clean up the handler
+			unsubscribe()
+
+			// Verify the tab didn't switch to browse
+			expect(tabSwitched).toBe(false)
+			const state = manager.getState()
+			expect(state.activeTab).toBe("sources")
+		})
+
+		it("should make minimal state updates when timeout occurs in browse tab", async () => {
+			// First ensure we're in browse tab
+			await manager.transition({
+				type: "SET_ACTIVE_TAB",
+				payload: { tab: "browse" },
+			})
+
+			// Add some items
+			const testItems = [createTestItem(), createTestItem({ name: "Item 2" })]
+			await manager.transition({
+				type: "FETCH_COMPLETE",
+				payload: { items: testItems },
+			})
+
+			// Start a new fetch
+			await manager.transition({ type: "FETCH_ITEMS" })
+
+			// Track state changes
+			let stateChangeCount = 0
+			const unsubscribe = manager.onStateChange(() => {
+				stateChangeCount++
+			})
+
+			// Reset the counter since we've already had state changes
+			stateChangeCount = 0
+
+			// Fast-forward past the timeout
+			jest.advanceTimersByTime(30000)
+
+			// Clean up the handler
+			unsubscribe()
+
+			// Verify we got a state update
+			expect(stateChangeCount).toBe(1)
+
+			// Verify the items were preserved
+			const state = manager.getState()
+			expect(state.allItems).toHaveLength(2)
+			expect(state.isFetching).toBe(false)
+			expect(state.activeTab).toBe("browse")
+		})
+
 		it("should prevent concurrent fetches during timeout period", async () => {
 			jest.clearAllMocks() // Clear mock to ignore initialize() call