-
Notifications
You must be signed in to change notification settings - Fork 4
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
fix(web): resolve selecting new member issue #1358
Conversation
WalkthroughThe pull request introduces modifications to the Changes
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 (
|
✅ Deploy Preview for reearth-cms ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
web/src/components/molecules/Member/MemberAddModal/index.tsx (1)
Line range hint
164-182
: Add error handling for search operations.The search functionality should handle and display errors that might occur during the search operation. Consider adding error handling and user feedback.
<AutoComplete open={isResultOpen} options={options} popupMatchSelectWidth={433} notFoundContent={t("No result")} onSearch={handleMemberNameChange} + status={searchError ? "error" : undefined} onFocus={() => { if (options?.length) { setIsResultOpen(true); } }} // ... rest of the props > <Search size="large" allowClear loading={searchLoading} + placeholder={searchError ? t("Error occurred during search") : t("Email address or user name")} placeholder={t("Email address or user name")} /> </AutoComplete>
🧹 Nitpick comments (2)
web/src/components/molecules/Member/MemberAddModal/index.tsx (2)
109-111
: Consider resetting AutoComplete's internal state.While resetting the form field is good, the AutoComplete component might maintain its own internal state. Consider also clearing the AutoComplete's value directly to ensure consistent behavior across all browsers.
const handleSelect = useCallback( (user: User) => { onUserAdd(user); resultClear(); form.resetFields(["search"]); + // Consider adding: + // if (autoCompleteRef.current) { + // autoCompleteRef.current.setValue(""); + // } }, [onUserAdd, resultClear, form], );
164-164
: Consider adding search field validation.Now that the search field is properly connected to form state, consider adding validation rules to prevent empty searches or enforce minimum length requirements.
-<Form.Item label={t("Search user")} name="search"> +<Form.Item + label={t("Search user")} + name="search" + rules={[ + { min: 2, message: t("Please enter at least 2 characters") }, + ]}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/components/molecules/Member/MemberAddModal/index.tsx
(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - reearth-cms
- GitHub Check: Header rules - reearth-cms
- GitHub Check: Pages changed - reearth-cms
- GitHub Check: ci-web / ci
🔇 Additional comments (2)
web/src/components/molecules/Member/MemberAddModal/index.tsx (2)
87-87
: LGTM! Using user.id as option value improves reliability.Using the user's ID as the option value ensures proper identification and selection of users.
30-30
: Verify the impact of FormValues type change.The type change from
Record<string, Role>
to{ search: string } & Record<string, Role>
is more specific and provides better type safety. However, this is a breaking change.Let's verify other usages of this type:
Overview
This PR fixes the bug you can't select suggested member.
Summary by CodeRabbit
New Features
Bug Fixes