fix(admin): add skeleton loader to invite-user Org select#1735
fix(admin): add skeleton loader to invite-user Org select#1735Shreyag02 wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughTwo invite-user views now use ChangesInvite users form UX refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (1)
web/sdk/admin/views/organizations/details/layout/invite-users-dialog.tsx (1)
111-129: 📐 Maintainability & Code Quality | 🔵 TrivialDuplicate email-parsing / disable logic across both invite dialogs.
This
isDisabledcomputation (parse emails, check role, submitting, errors) is duplicated almost verbatim inweb/sdk/admin/views/users/list/invite-users.tsx. Consider extracting a shared hook (e.g.useInviteFormDisabled(values, isSubmitting, errors)) to avoid divergence when validation rules change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b195017-2b13-4e58-a43b-dbe15e9b8041
📒 Files selected for processing (3)
web/sdk/admin/views/organizations/details/layout/invite-users-dialog.tsxweb/sdk/admin/views/users/list/invite-users.module.cssweb/sdk/admin/views/users/list/invite-users.tsx
💤 Files with no reviewable changes (1)
- web/sdk/admin/views/users/list/invite-users.module.css
Coverage Report for CI Build 28794203855Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage remained the same at 44.865%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/sdk/client/views/members/components/invite-member-dialog.tsx (1)
34-46: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMinor: schema re-instantiated per validation call.
yup.string().email()is created fresh on every email in the list during.test; hoisting it outside the test callback avoids redundant object construction on each keystroke/submit.♻️ Suggested tweak
+const emailValidator = yup.string().email(); + const inviteSchema = yup.object({ ... .test('emails', 'Enter valid email address(es)', value => { ... return ( emailList.length > 0 && - emailList.every(email => yup.string().email().isValidSync(email)) + emailList.every(email => emailValidator.isValidSync(email)) ); }) });
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3802b7be-3d7f-4f7d-9169-c71410692f5c
📒 Files selected for processing (3)
web/sdk/admin/views/organizations/details/layout/invite-users-dialog.tsxweb/sdk/admin/views/users/list/invite-users.tsxweb/sdk/client/views/members/components/invite-member-dialog.tsx
| <Field | ||
| label="Emails" | ||
| error={errors?.emails?.message || errors?.emails?.[0]?.message}> | ||
| {isLoading ? ( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Emails error message only surfaces for the first invalid entry.
errors?.emails?.[0]?.message only catches a validation failure at array index 0. Since the schema validates each comma-separated email independently (z.array(z.string().email())), an invalid email at any later position (e.g. "valid@example.com, bad-email") sets the error at errors.emails[1], which this fallback never reads. Submission stays blocked but the user sees no error message — contrary to the "visible error messages" approach agreed on in prior review discussion.
🐛 Suggested fix
- <Field
- label="Emails"
- error={errors?.emails?.message || errors?.emails?.[0]?.message}>
+ <Field
+ label="Emails"
+ error={
+ errors?.emails?.message ||
+ (Array.isArray(errors?.emails)
+ ? errors.emails.find(e => e?.message)?.message
+ : undefined)
+ }>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Field | |
| label="Emails" | |
| error={errors?.emails?.message || errors?.emails?.[0]?.message}> | |
| {isLoading ? ( | |
| <Field | |
| label="Emails" | |
| error={ | |
| errors?.emails?.message || | |
| (Array.isArray(errors?.emails) | |
| ? errors.emails.find(e => e?.message)?.message | |
| : undefined) | |
| }> | |
| {isLoading ? ( |
| variant="solid" | ||
| color="accent" | ||
| disabled={isDisabled} | ||
| disabled={isLoading} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Submit button no longer gates on required fields/email validity.
disabled={isLoading} only accounts for loading state; the prior isDisabled check (email list non-empty, type selected, not submitting) was removed and nothing replaces it. This conflicts with the PR's stated goal of applying the same submit-gating behavior to this dialog — the button is now clickable with empty/invalid fields, even though the yup resolver will still block the actual submission on click.
🔧 Suggested fix
const {
register,
control,
handleSubmit,
reset,
- formState: { errors, isSubmitting }
+ formState: { errors, isSubmitting, isValid }
} = useForm({
- resolver: yupResolver(inviteSchema)
+ resolver: yupResolver(inviteSchema),
+ mode: 'onChange'
});- disabled={isLoading}
+ disabled={isLoading || isSubmitting || !isValid}3 participants
Summary
The Organization select in the Users-list "Invite user" dialog did not render a skeleton loader while its data was loading. This PR adds the skeleton (driven by the real query loading state) and applies a set of consistency cleanups across the admin invite dialogs and the client SDK invite dialog.
Following the review discussion, the previous "disable the submit button" pattern has been replaced with schema-level validation and visible field-level error messages. The submit button no longer silently disables; instead, missing required fields and malformed emails surface inline messages through the Field component.
Changes
Technical Details
Test Plan
SQL Safety (if your PR touches
*_repository.goorgoqu.*)N/A