mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
Fix marketplace tab switching and redraw issue that occurred every 30 seconds
This commit is contained in:
parent
2c6ef8a184
commit
62d5589397
2 changed files with 147 additions and 9 deletions
|
|
@ -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 { MarketplaceItem, MarketplaceSource, MatchInfo } from "../../../../src/services/marketplace/types"
|
||||||
import { vscode } from "../../utils/vscode"
|
import { vscode } from "../../utils/vscode"
|
||||||
import { WebviewMessage } from "../../../../src/shared/WebviewMessage"
|
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
|
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
|
// Save state to sessionStorage if available
|
||||||
if (typeof sessionStorage !== "undefined") {
|
if (typeof sessionStorage !== "undefined") {
|
||||||
|
|
@ -186,11 +222,12 @@ export class MarketplaceViewStateManager {
|
||||||
}
|
}
|
||||||
this.notifyStateChange()
|
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.fetchTimeoutId = setTimeout(() => {
|
||||||
this.clearFetchTimeout()
|
this.clearFetchTimeout()
|
||||||
// On timeout, preserve items if we have them
|
// On timeout, preserve items if we have them
|
||||||
if (currentItems.length > 0) {
|
if (currentItems.length > 0) {
|
||||||
|
// Only update the isFetching flag without triggering a full redraw
|
||||||
this.state = {
|
this.state = {
|
||||||
...this.state,
|
...this.state,
|
||||||
isFetching: false,
|
isFetching: false,
|
||||||
|
|
@ -198,13 +235,34 @@ export class MarketplaceViewStateManager {
|
||||||
displayItems: currentItems,
|
displayItems: currentItems,
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
// Preserve the current tab and only update necessary state
|
||||||
|
const { activeTab, sources } = this.state
|
||||||
this.state = {
|
this.state = {
|
||||||
...this.getDefaultState(),
|
...this.getDefaultState(),
|
||||||
sources: [...this.state.sources],
|
sources: [...sources],
|
||||||
activeTab: this.state.activeTab,
|
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)
|
}, this.FETCH_TIMEOUT)
|
||||||
|
|
||||||
break
|
break
|
||||||
|
|
@ -560,7 +618,15 @@ export class MarketplaceViewStateManager {
|
||||||
allItems: sortedItems,
|
allItems: sortedItems,
|
||||||
displayItems: newDisplayItems,
|
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
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -725,6 +725,78 @@ describe("MarketplaceViewStateManager", () => {
|
||||||
expect(state.allItems).toHaveLength(1)
|
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 () => {
|
it("should prevent concurrent fetches during timeout period", async () => {
|
||||||
jest.clearAllMocks() // Clear mock to ignore initialize() call
|
jest.clearAllMocks() // Clear mock to ignore initialize() call
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue