diff --git a/webview-ui/src/components/marketplace/MarketplaceViewStateManager.ts b/webview-ui/src/components/marketplace/MarketplaceViewStateManager.ts index 9bb6ef6421..58c2ea142d 100644 --- a/webview-ui/src/components/marketplace/MarketplaceViewStateManager.ts +++ b/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 + } } } diff --git a/webview-ui/src/components/marketplace/__tests__/MarketplaceViewStateManager.test.ts b/webview-ui/src/components/marketplace/__tests__/MarketplaceViewStateManager.test.ts index b3f02b459d..da52c59a4c 100644 --- a/webview-ui/src/components/marketplace/__tests__/MarketplaceViewStateManager.test.ts +++ b/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