DT-3070: Add codec server form component #2797
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
GiantRobots
reviewed
Jul 1, 2025
andrewzamojc
reviewed
Jul 9, 2025
|
|
||
| <!-- Toggle Switches --> | ||
| <div class="space-y-3"> | ||
| <ToggleSwitch |
There was a problem hiding this comment.
Possible change: How do you feel about switches here? I'm under the impression that checkboxes are better on forms that use a submit button. The switch metaphor is intended to be "live" like a light switch, and should make a result immediately (e.g. turn dark mode on/off).
I found random article about it, but happy to hear more from you if you really prefer switches here. https://blog.uxtweak.com/checkbox-vs-toggle-switch/
There was a problem hiding this comment.
Switches are pretty, but I don't mind either way. I think we could bring it up with design.
| const adapter = createCodecAdapter(); | ||
| </script> | ||
| <CodecServerForm {adapter} /> |
There was a problem hiding this comment.
Comment: This seems nice. The fetching is captured and reusable, while the form is purer. I like the separation.
| PUT /api/v1/settings/codec | ||
| ``` | ||
|
|
||
| The codec adapter handles the transformation between the form data structure and the API format, ensuring compatibility with both current and future endpoints. |
There was a problem hiding this comment.
Comment: This doc was really helpful! I dig it.
andrewzamojc
approved these changes
Jul 9, 2025
Implement codec server configuration form using search-attributes-form as template: - Form fields: endpoint URL, access token toggle, CORS credentials toggle - Expandable custom error message and redirect link section - Smart visibility logic for custom fields when values are populated - Zod validation for required URL endpoints and optional custom links - Toggle switches for better UX instead of checkboxes - Tainted state tracking with badge on save button - SuperForms integration with dataType: 'json' and resetForm: false - Skeleton loading states matching existing patterns - API integration with codec adapter for OSS endpoints - Storybook documentation and examples - Direct imports (no barrel exports) for better tree shaking - Svelte 5 state management instead of legacy stores
- Use fetchSettings() from settings-service instead of direct API calls - Properly integrate with existing Settings type structure - Follow same pattern as other codec functionality in codebase - Clear messaging about development limitations vs actual functionality
- Remove namespace parameter from createCodecAdapter() for OSS - OSS operates at cluster level, Cloud will be namespace-scoped - Update messaging and documentation to reflect cluster-level scope - Prepare for future Cloud adapter implementation
Create FormMessage component to handle both form validation errors and status messages. Abstract TaintedBadge logic from duplicated implementations in both forms.
- Move form components to /form/ directory and rename for better organization - Make TaintedBadge and Message components "dumb" - accept resolved values instead of stores - Add comprehensive i18n translations for codec server form - Fix search attributes form reset function error - Clean up component interfaces with better prop naming (message -> value, formErrors -> errors) - Extract tainted count calculation to derived variables in parent components - Add proper TypeScript types for Alert intent
…ating - Replace custom HTML link with Holocene Link component for better consistency - Split endpoint description into prefix/suffix pattern for cleaner templating - Remove manual styling in favor of component defaults - Follow cloud-ui patterns for codec server endpoint links
Move retry state and logic from callers into ApiError component itself. Components now pass retryConfig with retryFn instead of managing retry state externally. - Add internal retry state (retryCount, isRetrying) - Replace onRetry callback with retryConfig interface - Add loading state during retry operations - Simplify caller interface to config-based approach
13ff880 to
636831b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description & motivation 💭
Creates a reusable codec server configuration form following the established patterns from search-attributes-form. The implementation emphasizes component abstraction to reduce code duplication while maintaining clean separation of concerns.
Key architectural decisions include using the SuperForms + reactive adapter pattern for flexible data integration, extracting common form components (Message and TaintedBadge) to eliminate duplication between forms, and implementing smart section visibility where the custom error section automatically shows when values are populated.
The form integrates comprehensive i18n support with proper translation templating for HTML links, uses Zod schema validation for URL requirements and type safety, and includes tainted state tracking with visual feedback through badge indicators.
Screenshots (if applicable) 📸
Storybook stories available at Forms/Codec Server Form showing:
Storybook Links
Design Considerations 🎨
Testing 🧪
How was this tested 👻
Manual Testing Steps
Steps for others to test: 🚶🏽♂️🚶🏽♀️
Checklists
Draft Checklist
Merge Checklist
Issue(s) closed
DT-3070
Docs
Any docs updates needed?
Component includes comprehensive MDX documentation in Storybook with usage examples and API reference.