Conversation
There was a problem hiding this comment.
Pull request overview
This PR integrates the shared Batcher utility into the SkyBrowser topic subscription flow, updating the SkyBrowser reducer to accept partial/batched updates and reduce Redux dispatch frequency.
Changes:
- Batch incoming
skybrowsertopic messages viaBatcherbefore dispatchingupdateSkyBrowser. - Update
updateSkyBrowserreducer to acceptPartial<SkyBrowserUpdate>and apply field-level updates. - Adjust derived SkyBrowser state updates to be conditional on receiving
browsersdata.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/redux/skybrowser/skybrowserSlice.ts |
Switch updateSkyBrowser to partial updates and conditionally update state fields / derived arrays. |
src/redux/skybrowser/skybrowserMiddleware.ts |
Add Batcher to batch topic iterator updates before dispatching to Redux. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (action.payload.selectedBrowserId !== undefined) { | ||
| state.selectedBrowserId = action.payload.selectedBrowserId; | ||
| } | ||
|
|
||
| if (action.payload.browsers && state.selectedBrowserId in action.payload.browsers) { |
There was a problem hiding this comment.
updateSkyBrowser now accepts Partial<SkyBrowserUpdate>, but the reducer still resets the entire SkyBrowser state whenever payload.browsers is missing (because the if (action.payload.browsers && ...) falls through to the reset else). With batching/partial updates this can cause the UI to clear on updates that only include selectedBrowserId/cameraInSolarSystem. Adjust the logic so that the reset/validation only happens when payload.browsers is present, and when payload.selectedBrowserId is present without browsers validate it against the existing state.browsers (reset only if the id is invalid).
engbergandreas
left a comment
There was a problem hiding this comment.
Lol realised now that I'm basically reiterating what Copilot already found 🤷♂️
| event: 'stop_supscription' | ||
| event: 'stop_subscription' |
Adds the newly introduced batching to the SkyBrowser topic updates