fix : clearable select and add a story in storybook #546
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FieldSelect
participant zFormSchema
User->>FieldSelect: Select color option
FieldSelect->>zFormSchema: Validate color selection
zFormSchema-->>FieldSelect: Return selected color
FieldSelect-->>User: Display selected color
User->>FieldSelect: Clear selection
FieldSelect->>FieldSelect: Check action type
FieldSelect->>FieldSelect: Set value to null
FieldSelect-->>User: Display no selection
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/components/Form/FieldSelect/docs.stories.tsx (1)
148-175: Consider enhancing the Clearable story documentationThe implementation correctly demonstrates the clearable functionality and follows the established story pattern. Consider these enhancements:
export const Clearable = () => { + // Demonstrates a select field that can be cleared, setting its value to null const form = useForm<FormSchema>({ ...formOptions, + defaultValues: { + color: 'red', // Shows the clear button when a value is selected + }, }); return (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Form/FieldSelect/docs.stories.tsx(2 hunks)src/components/Form/FieldSelect/index.tsx(1 hunks)
🔇 Additional comments (2)
src/components/Form/FieldSelect/index.tsx (1)
64-69: LGTM! Clear action is now properly handled.
The implementation correctly sets the value to null when the select is cleared, which fixes the original issue. The early return pattern keeps the code clean and maintainable.
src/components/Form/FieldSelect/docs.stories.tsx (1)
15-15: LGTM: Schema update correctly supports clearable functionality
The modification to make the color field nullable is appropriate and necessary to support the clearable select feature.
| // @ts-expect-error TODO should fix the typing. This error pops when | ||
| // we propagate the `selectProps` | ||
| onChange(newValue?.value); | ||
| }} |
There was a problem hiding this comment.
💡 Codebase verification
The typing issue can be fixed by properly typing the onChange handler
The @ts-expect-error occurs because the component is using chakra-react-select's types but not properly handling the onChange event typing. The fix would involve:
- The base
Selectcomponent uses generics from chakra-react-select FieldSelectandFieldMultiSelectneed to properly type theironChangehandlers to match the generic type parameters
Example locations that need attention:
src/components/Form/FieldSelect/index.tsx: Add proper typing foronChangehandlersrc/components/Form/FieldMultiSelect/index.tsx: Same typing issue exists here
🔗 Analysis chain
Consider addressing the typing issue.
The @ts-expect-error indicates a typing issue with selectProps propagation. While the functionality works, improving type safety would prevent potential runtime issues.
Let's check for similar typing patterns in other components:
Let me gather more context about the Select components and their type definitions.
Let me check the base Select component implementation to understand the typing issue better.
Let me check the Chakra UI Select implementation since the code seems to be using Chakra's wrapped version of react-select.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar Select component usage patterns to understand the typing approach
ast-grep --pattern 'onChange={(newValue, actionMeta) => { $$$ }}'
# Look for other @ts-expect-error occurrences that might provide insights
rg "@ts-expect-error" -A 2
Length of output: 1100
Script:
#!/bin/bash
# Check the type definitions and imports in both Select components
rg -A 10 "interface.*Props" src/components/Form/FieldSelect/
rg -A 10 "interface.*Props" src/components/Form/FieldMultiSelect/
# Look for Select component imports and their types
rg "import.*Select" src/components/Form/Field*
# Check the component definitions
ast-grep --pattern 'const FieldSelect: $_ = ($_) => { $$$ }'
ast-grep --pattern 'const FieldMultiSelect: $_ = ($_) => { $$$ }'
Length of output: 914
Script:
#!/bin/bash
# Check the base Select component implementation and its types
rg -A 10 "interface.*Props" src/components/Select/
rg -A 10 "type.*Props" src/components/Select/
# Look for the Select component implementation
ast-grep --pattern 'const Select = ($_) => { $$$ }'
ast-grep --pattern 'export interface SelectProps { $$$ }'
# Check how the value and onChange props are typed
rg "onChange.*:" src/components/Select/
Length of output: 2048
Script:
#!/bin/bash
# Check for Chakra Select component imports and types
rg "import.*chakra.*select" -i src/components/Select/
rg "ChakraReactSelect" -A 5 src/components/Select/
# Look for the value type definitions
rg "type.*Value" src/components/Select/
rg "interface.*Value" src/components/Select/
# Check for any type exports or re-exports
rg "export type" src/components/Select/
Length of output: 991
| @@ -12,7 +12,7 @@ export default { | |||
| type FormSchema = z.infer<ReturnType<typeof zFormSchema>>; | |||
| const zFormSchema = () => | |||
| z.object({ | |||
| color: z.enum(['red', 'green', 'blue']), | |||
There was a problem hiding this comment.
We don't want it to be nullish for most of the stories. For the change you made, please create a new schema
Describe your changes
Fix the select with the clearable props. The value was not set to null when the field was cleared
Screenshots
Screen.Recording.2024-11-07.at.17.54.35.mov
Summary by CodeRabbit
New Features
Bug Fixes