Biz/dj 2813 support custom input strings in connect react #15356
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThis pull request introduces a new feature in the Changes
Sequence DiagramsequenceDiagram
participant User
participant ControlSelect
participant Options Array
User->>ControlSelect: Enter new option
ControlSelect->>ControlSelect: handleCreate(inputValue)
ControlSelect->>Options Array: Unshift new option
Options Array-->>ControlSelect: Updated options list
ControlSelect-->>User: Display updated select options
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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: 2
🔭 Outside diff range comments (1)
packages/connect-react/src/components/ControlSelect.tsx (1)
Line range hint
13-19: Enhance type safety for creatable options.The component allows creating new options with string values, but the generic type
Tdoesn't enforce this constraint. This could lead to type mismatches whenisCreatableis true.Consider constraining the type when
isCreatableis true:type ControlSelectProps<T> = { isCreatable?: boolean; options: {label: string; value: T extends string ? T : never;}[]; selectProps?: ReactSelectProps; showLoadMoreButton?: boolean; onLoadMore?: () => void; };
🧹 Nitpick comments (1)
packages/connect-react/CHANGELOG.md (1)
4-7: Enhance changelog entry with more details.The current entry is too brief and doesn't provide enough context about the feature's impact and usage.
Consider expanding the entry:
# [1.0.0-preview.22] - 2025-01-21 -Allow custom string input for remote options +### Added +- Support for custom string input in remote options + - Users can now enter custom values when `isCreatable` is enabled + - Custom values are treated as both label and value + +### BREAKING CHANGES +- Remote options are now creatable by default + - To maintain previous behavior, explicitly set `isCreatable={false}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/connect-react/examples/nextjs/package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
packages/connect-react/CHANGELOG.md(1 hunks)packages/connect-react/package.json(1 hunks)packages/connect-react/src/components/ControlSelect.tsx(2 hunks)packages/connect-react/src/components/RemoteOptionsContainer.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/connect-react/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: pnpm publish
| const handleCreate = (inputValue: string) => { | ||
| options.unshift({ | ||
| label: inputValue, | ||
| value: inputValue, | ||
| }) | ||
| }; |
There was a problem hiding this comment.
Avoid direct array mutation and add input validation.
The handleCreate function directly mutates the options array, which could lead to unexpected behavior in React. Additionally, there's no validation of the input value.
Consider this safer implementation:
- const handleCreate = (inputValue: string) => {
- options.unshift({
- label: inputValue,
- value: inputValue,
- })
- };
+ const handleCreate = (inputValue: string) => {
+ if (!inputValue?.trim()) return;
+ const newOption = {
+ label: inputValue,
+ value: inputValue as T,
+ };
+ onChange([newOption, ...(Array.isArray(value) ? value : [])]);
+ };Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| return ( | ||
| <ControlSelect | ||
| isCreatable={true} |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Confirmed: isCreatable needs to be configurable
The component's props interface (RemoteOptionsContainerProps) currently only includes queryEnabled and doesn't have any configuration for controlling the creatable behavior. Making isCreatable configurable through props would indeed improve component flexibility.
- Add
allowCustomInput?: booleantoRemoteOptionsContainerProps - Document the new prop's purpose and default behavior
🔗 Analysis chain
Make isCreatable configurable instead of hardcoded.
Hardcoding isCreatable to true forces all remote options to be creatable, which might not be appropriate for all use cases and could bypass server-side validation.
Consider making it configurable:
- isCreatable={true}
+ isCreatable={prop.allowCustomInput}This would require adding an allowCustomInput flag to the prop configuration schema.
Let's check if the prop configuration schema already supports this:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for prop configuration schema
rg -A 5 "type.*PropConfiguration" || rg -A 5 "interface.*PropConfiguration"
Length of output: 76
Script:
#!/bin/bash
# Check RemoteOptionsContainer.tsx for prop definitions
cat packages/connect-react/src/components/RemoteOptionsContainer.tsx | grep -A 10 "interface.*Props"
# Search for any remote options related types/interfaces
rg -g "*.ts*" -A 5 "interface.*Remote.*Props|type.*Remote.*Props"
Length of output: 760
bzwrk
deleted the
biz/dj-2813-support-custom-input-strings-in-connect-react
branch
Add support for custom string input
