-
Notifications
You must be signed in to change notification settings - Fork 462
Zustand POC #4919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Zustand POC #4919
Conversation
}; | ||
|
||
export const zoomStore = create<ZoomStore>()( | ||
devtools( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
y: state.initialCoordinates.y, | ||
}, | ||
}), | ||
false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to replace the state or not
}, | ||
}), | ||
false, | ||
'fitToScreen' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name for redux dev tools
) | ||
); | ||
|
||
export const useZoom = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 hooks, 1 for values and another for "actions"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Migrates zoom state management from a React Context provider to a global Zustand store, updating all consumers and removing the previous provider and one related test.
- Replace ZoomProvider with a singleton Zustand store (zoom.store.ts) and refactor imports/usages.
- Update components and tests to use new hooks; remove zoom-fit-screen test.
- Add new zoom actions (resetZoom) and dependency on zustand.
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
application/ui/src/routes/inference/inference.tsx | Removed ZoomProvider wrapper around StreamContainer. |
application/ui/src/features/dataset/media-preview/primary-toolbar/zoom/zoom-selector.test.tsx | Updated mocks to target new Zustand store. |
application/ui/src/features/dataset/media-preview/primary-toolbar/zoom/zoom-selector.component.tsx | Refactored to use new useZoom/useSetZoom from store. |
application/ui/src/features/dataset/media-preview/primary-toolbar/zoom/zoom-fit-screen.test.tsx | Removed test covering fit-to-screen action. |
application/ui/src/features/dataset/media-preview/primary-toolbar/zoom/zoom-fit-screen.component.tsx | Switched to store hook import. |
application/ui/src/features/dataset/media-preview/media-preview.component.tsx | Removed ZoomProvider wrapper. |
application/ui/src/features/annotator/tools/segment-anything-tool/segment-anything-tool.component.tsx | Switched zoom hook import. |
application/ui/src/features/annotator/tools/bounding-box-tool/bounding-box-tool.component.tsx | Switched zoom hook import. |
application/ui/src/features/annotator/annotations/editable-annotation.component.tsx | Switched zoom hook import. |
application/ui/src/components/zoom/zoom.test.tsx | Test no longer wraps with provider. |
application/ui/src/components/zoom/zoom.store.ts | Added new global Zustand zoom store and hooks. |
application/ui/src/components/zoom/zoom.provider.tsx | Removed legacy context provider implementation. |
application/ui/src/components/zoom/zoom-transform.tsx | Adjusted to use global store directly; added fresh state reads. |
application/ui/src/components/zoom/use-sync-zoom.hook.tsx | Updated import to new store hook. |
application/ui/package.json | Added zustand dependency. |
Files not reviewed (1)
- application/ui/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Docker Image Sizes
|
📊 Test coverage report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I don't really see the need for this. In Geti the issue with providers was that we were exposing state from queries and mutations from a provider.
We don't have to do this here and can instead use the useQuery
hooks locally since they all use react-query's global state already.
Summary
Resources:
How to test
Checklist