fix(ui): sync pin/unpin state across conversation tabs (#12884)#12932
fix(ui): sync pin/unpin state across conversation tabs (#12884)#12932hieptl merged 4 commits intoOpenHands:mainfrom
Conversation
all-hands-bot
left a comment
There was a problem hiding this comment.
Solid fix for the pin/unpin synchronization issue. The custom event approach works well and the test coverage is good.
| }, [conversationId]); | ||
|
|
||
| const updateState = (updates: Partial<ConversationState>) => { | ||
| setState((prev) => ({ ...prev, ...updates })); |
There was a problem hiding this comment.
🟡 Suggestion: This setState call is redundant since setConversationState will immediately dispatch an event that triggers another setState via the event listener (line 137). Consider removing this line to:
- Avoid double state updates (both are queued synchronously)
- Make localStorage the single source of truth
- Prevent potential inconsistencies if the localStorage write fails
The event-driven update alone is sufficient for keeping all components in sync.
|
Addressed review feedback in 893eda4 by removing the redundant local setState in updateState (localStorage/event-driven sync remains the single source of truth).\n\nRe-ran targeted tests:\n- npm run test -- tests/components/features/conversation/conversation-tabs.test.tsx tests/conversation-local-storage.test.ts\n- Result: 2 test files passed, 22 tests passed. |
Summary
Fixes tab pin/unpin behavior in the conversation tab menu so changes are reflected immediately in the tab bar.
Fixes #12884
Root cause
ConversationTabsandConversationTabsContextMenuboth useuseConversationLocalStorageState, but each component had independent local hook state. UpdatingunpinnedTabsfrom the context menu wrote to localStorage but did not trigger a same-tab re-sync inConversationTabs.What changed
conversation-state-updated.setConversationStateandclearConversationLocalStorage.useConversationLocalStorageStateto listen to:storagefor cross-tab updatesconversation-state-updatedfor same-tab updatesTesting
npm run test -- __tests__/components/features/conversation/conversation-tabs.test.tsx __tests__/conversation-local-storage.test.ts