-
Notifications
You must be signed in to change notification settings - Fork 462
Sinks List styling #5188
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?
Sinks List styling #5188
Conversation
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
This PR refactors the sinks management interface to display a list view of configured sinks with improved styling and user interaction. The changes move from inline form editing to a list-based UI with dedicated add/edit flows, matching the pattern used for sources.
Changes:
- Implemented a list view for displaying all configured sinks with status indicators
- Added separate flows for adding new sinks and editing existing ones
- Refactored sink form components to be reusable between add and edit modes
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| application/ui/src/features/inference/sinks/sink-list/sink-list.component.tsx | New component displaying sinks in a card-based list with add button |
| application/ui/src/features/inference/sinks/sink-actions.component.tsx | New component managing view state transitions between list/add/edit modes |
| application/ui/src/features/inference/sinks/add-sink/add-sink.component.tsx | New wrapper component for adding sinks with auto-connect functionality |
| application/ui/src/features/inference/sinks/edit-sink/edit-sink.component.tsx | New wrapper component for editing existing sinks with save options |
| application/ui/src/features/inference/sinks/webhook/webhook.component.tsx | Refactored to accept defaultState prop instead of managing form state internally |
| application/ui/src/features/inference/sinks/mqtt/mqtt.component.tsx | Refactored to accept defaultState prop instead of managing form state internally |
| application/ui/src/features/inference/sinks/local-folder/local-folder.component.tsx | Refactored to accept defaultState prop instead of managing form state internally |
| application/ui/src/features/inference/sinks/output-formats/output-formats.component.tsx | Moved to subdirectory and updated to use CheckboxGroup with isRequired validation |
| application/ui/src/features/inference/sinks/hooks/use-sink-action.hook.tsx | Modified to support optional onSaved callback instead of auto-connecting to pipeline |
| application/ui/src/hooks/api/pipeline.hook.ts | Added useConnectSinkToPipeline hook for connecting sinks to pipeline |
| application/ui/tests/inference/inference.spec.ts | Updated test expectations to match new button labels and list-based UI |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </Switch> | ||
| </Flex> | ||
| </Form> | ||
| <OutputFormats config={defaultState.output_formats} />{' '} |
Copilot
AI
Jan 14, 2026
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.
Remove trailing whitespace after the closing tag.
| <OutputFormats config={defaultState.output_formats} />{' '} | |
| <OutputFormats config={defaultState.output_formats} /> |
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.
We do need this space, right?
| export type SinkDisconnectedOutputFormats = components['schemas']['RosSinkConfigView']; | ||
| export type RosSinkConfig = components['schemas']['RosSinkConfigView']; |
Copilot
AI
Jan 14, 2026
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.
The type alias 'SinkDisconnectedOutputFormats' is misleading as it's assigned the same type as 'RosSinkConfig'. Either this is an error and should reference a different schema, or the name should be changed to reflect what it actually represents. Consider renaming to something more appropriate or using the correct schema type.
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.
+1
📊 Test coverage report
|
Docker Image SizesCPU
GPU
XPU
|
jpggvilaca
left a comment
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.
some minor comments but LGTM
Summary
Screen.Recording.2026-01-14.at.12.24.43.mov
How to test
Checklist