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 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughTwo invite-user form components (organization details dialog and users list) were refactored to use ChangesInvite Users Form UX Refactor
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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
| {isLoading ? ( | ||
| <Skeleton height="80px" /> | ||
| ) : ( | ||
| <Field | ||
| label="Emails" | ||
| error={errors?.emails?.message || errors?.emails?.[0]?.message}> | ||
| <Controller | ||
| name="emails" | ||
| control={control} | ||
| render={({ field }) => { | ||
| const { value, ...rest } = field; | ||
| return ( | ||
| <TextArea | ||
| {...rest} | ||
| value={Array.isArray(value) ? value.join(", ") : (value ?? "") as string} | ||
| placeholder="abc@example.com, xyz@example.com" | ||
| className={styles["invite-users-emails-textarea"]} | ||
| /> | ||
| ); | ||
| }} | ||
| /> | ||
| </Field> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Emails field unnecessarily gated behind the loading skeleton, and inconsistently loses its label.
The Emails textarea has no dependency on the roles/organizations queries, yet it's fully hidden (including its "Emails" label) behind isLoading, unlike the Role/Organization fields where the Field wrapper (and its label) always renders and only the inner control is skeleton-swapped. This:
- Unnecessarily blocks users from typing emails while unrelated org/role data loads.
- Causes a jarring "label pop-in" once loading completes, inconsistent with the other two fields.
- Diverges from the PR's stated intent of adding the skeleton loader "to the Organization select" only.
Consider always rendering the Field (with its label), and only swapping the inner TextArea/Controller for a Skeleton, matching the Role/Organization pattern.
💡 Suggested fix
- {isLoading ? (
- <Skeleton height="80px" />
- ) : (
- <Field
- label="Emails"
- error={errors?.emails?.message || errors?.emails?.[0]?.message}>
- <Controller
- name="emails"
- control={control}
- render={({ field }) => {
- const { value, ...rest } = field;
- return (
- <TextArea
- {...rest}
- value={Array.isArray(value) ? value.join(", ") : (value ?? "") as string}
- placeholder="abc@example.com, xyz@example.com"
- className={styles["invite-users-emails-textarea"]}
- />
- );
- }}
- />
- </Field>
- )}
+ <Field
+ label="Emails"
+ error={errors?.emails?.message || errors?.emails?.[0]?.message}>
+ {isLoading ? (
+ <Skeleton height="80px" />
+ ) : (
+ <Controller
+ name="emails"
+ control={control}
+ render={({ field }) => {
+ const { value, ...rest } = field;
+ return (
+ <TextArea
+ {...rest}
+ value={Array.isArray(value) ? value.join(", ") : (value ?? "") as string}
+ placeholder="abc@example.com, xyz@example.com"
+ className={styles["invite-users-emails-textarea"]}
+ />
+ );
+ }}
+ />
+ )}
+ </Field>📝 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.
| {isLoading ? ( | |
| <Skeleton height="80px" /> | |
| ) : ( | |
| <Field | |
| label="Emails" | |
| error={errors?.emails?.message || errors?.emails?.[0]?.message}> | |
| <Controller | |
| name="emails" | |
| control={control} | |
| render={({ field }) => { | |
| const { value, ...rest } = field; | |
| return ( | |
| <TextArea | |
| {...rest} | |
| value={Array.isArray(value) ? value.join(", ") : (value ?? "") as string} | |
| placeholder="abc@example.com, xyz@example.com" | |
| className={styles["invite-users-emails-textarea"]} | |
| /> | |
| ); | |
| }} | |
| /> | |
| </Field> | |
| )} | |
| <Field | |
| label="Emails" | |
| error={errors?.emails?.message || errors?.emails?.[0]?.message}> | |
| {isLoading ? ( | |
| <Skeleton height="80px" /> | |
| ) : ( | |
| <Controller | |
| name="emails" | |
| control={control} | |
| render={({ field }) => { | |
| const { value, ...rest } = field; | |
| return ( | |
| <TextArea | |
| {...rest} | |
| value={Array.isArray(value) ? value.join(", ") : (value ?? "") as string} | |
| placeholder="abc@example.com, xyz@example.com" | |
| className={styles["invite-users-emails-textarea"]} | |
| /> | |
| ); | |
| }} | |
| /> | |
| )} | |
| </Field> |
Coverage Report for CI Build 28654224331Coverage remained the same at 44.865%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
2 participants
Summary
Fixes the Organization select in the Users-list,
Invite userdialog was missing a skeleton loader while its data loads. This adds the skeleton (driven by the real query loading state) and bundles a set of consistency cleanups across both admin invite dialogs.Changes
Fieldcomposite (Fieldlabel/error), replacing the customLabel+ errorTextmarkup.invite-users-dialog-label,form-error-message) frominvite-users.module.css.Fieldmigration + submit-gating to the org-Members "Invite user" dialog for consistency.Technical Details
const isLoading = true(which froze the dialog on skeletons) withisLoading = isRolesLoading || isOrganizationsLoading.Fieldhandles label↔control association and error rendering (aria-invalid), so the custom label/error CSS is no longer needed.watch(...)+useMemo(isDisabled): disabled on an empty email list, missing role/org, while submitting, or whenerrors.emailsis set.rolescome synchronously fromOrganizationContext(there's no loading state to gate on). Its sharedinvite-users-dialog-label/form-error-messageCSS classes are retained becauseadd-tokens-dialog.tsxstill uses them.admin/distwas rebuilt (pnpm build) so the change reaches consumers.Test Plan
SQL Safety (if your PR touches
*_repository.goorgoqu.*)N/A