Side panel #32
Side panel #32
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a configurable side panel feature to the collection creation form, allowing consumers to preview form output in real-time. It also includes field name standardization for token length parameters.
Key Changes:
- Refactored output preparation to support preview functionality by splitting
prepareOutputintopreprocessOutput(for preview) andprepareOutput(for validation) - Standardized field names from
min_token_length/max_token_lengthtomin_token_len/max_token_lenacross the codebase - Replaced the fixed sidebar with a configurable sidebar that displays custom preview content via the
onPreviewFormOutputprop
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/prepareOutput.js | Refactored to split output preparation logic, added filter for defensive programming, and renamed token length fields to use abbreviated names |
| src/inputs/ButtonGroupWithInputs.jsx | Updated field names in documentation comments to match new abbreviated token length naming |
| src/flow.js | Minor formatting improvement for multiline description text |
| src/collection-schema.jtd.json | Updated schema to use abbreviated token length field names and removed trailing whitespace |
| src/ThemedComponents.jsx | Restructured sidebar styling from absolute to sticky positioning, added scrolling support, and simplified component structure |
| src/Sidebar.jsx | New component that renders customizable sidebar content using the preview output function |
| src/CreateCollectionForm.jsx | Replaced hideSidebar prop with onPreviewFormOutput prop, integrated new Sidebar component, and updated layout to use responsive grid system |
| package-lock.json | Updated dependency resolution (removed peer flags), and minor version bumps for js-yaml and vite |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const CCFormSidebarInner = styled(Box, { | ||
| name: "MuiCreateCollectionForm", | ||
| slot: "sidebarStickyInner", | ||
| })(({ theme }) => ({ | ||
| position: "sticky", | ||
| top: "2rem", | ||
| [theme.breakpoints.down("md")]: { | ||
| position: "static", | ||
| }, | ||
| })); | ||
| })(); |
There was a problem hiding this comment.
The CCFormSidebarInner styled component now has an empty implementation () with no styles. However, it's still being used in the CCFormSidebar component (line 357) as a wrapper. Consider removing this component entirely if it serves no styling purpose, or document why it's kept as a structural component.
| setOutputData(handleOutput(previewOutput(formData, path))); | ||
| } |
There was a problem hiding this comment.
The useEffect hook is missing handleOutput cleanup logic. If handleOutput becomes null or undefined after initially being set, the outputData will retain its previous value instead of being cleared. Consider adding an else clause to set outputData to null when handleOutput is not a function.
| setOutputData(handleOutput(previewOutput(formData, path))); | |
| } | |
| setOutputData(handleOutput(previewOutput(formData, path))); | |
| } else { | |
| setOutputData(null); | |
| } | |
| } else { | |
| setOutputData(null); |
| </CCFormButton> | ||
| </Grid> | ||
| ) : ( | ||
| <></> |
There was a problem hiding this comment.
[nitpick] The empty fragment <></> is unnecessary. Consider using null instead for cleaner code when nothing needs to be rendered.
| <></> | |
| null |
No description provided.