-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[WEB-4488] feat: brand revamp #7544
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
Conversation
* chore: empty state asset and theme improvement * chore: upgrade modal improvement and code refactor
* refactor: consolidate password strength indicator into shared UI package * chore: remove old password strength meter implementations * chore: update package dependencies for password strength refactor * chore: code refactor * chore: brand logo added * chore: terms and conditions refactor * chore: auth form refactor * chore: oauth enhancements and refactor * chore: plane new logos added * chore: auth input form field added to ui package * chore: password input component added * chore: web auth refactor * chore: update brand colors and remove onboarding-specific styles * chore: clean up unused assets * chore: profile menu text overflow * chore: theme related changes * chore: logo spinner updated * chore: onboarding constant and types updated * chore: theme changes and code refactor * feat: onboarding flow revamp * fix: build error and code refactoring * chore: code refactor * fix: build error * chore: consent option added to onboarding and code refactor * fix: build fix * chore: code refactor * chore: auth screen revamp and code refactor * chore: onboarding enhancements * chore: code refactor * chore: onboarding logic improvement * chore: code refactor * fix: onboarding pre release improvements * chore: color token updated * chore: color token updated * chore: auth screen line height and size improvements * chore: input height updated * chore: n-progress theme updated * chore: theme and logo enhancements * chore: space auth and code refactor
* chore: updated logo, og image, and loaders * chore: updated branding colors
WalkthroughThis update delivers a comprehensive redesign and modernization of the authentication and onboarding flows across the web and admin apps. It introduces new modular React components for authentication, onboarding, and workspace creation, refactors existing logic for clarity and maintainability, and standardizes styling with updated color tokens and branding assets. Email templates, icons, and CSS variables are updated for branding consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthBase
participant AuthHeader
participant AuthRoot
participant AuthFormRoot
participant OAuthOptions
participant TermsAndConditions
User->>AuthBase: Navigates to Auth page
AuthBase->>AuthHeader: Render header (mode-aware)
AuthBase->>AuthRoot: Render root with auth mode
AuthRoot->>OAuthOptions: Render OAuth buttons (if enabled)
AuthRoot->>AuthFormRoot: Render form (step-based)
AuthRoot->>TermsAndConditions: Render T&C (mode-aware)
AuthFormRoot->>User: Handles email/code/password steps
OAuthOptions->>User: Handles OAuth login
sequenceDiagram
participant User
participant OnboardingRoot
participant OnboardingHeader
participant OnboardingStepRoot
participant ProfileSetupStep
participant RoleSetupStep
participant UseCaseSetupStep
participant WorkspaceSetupStep
participant InviteTeamStep
User->>OnboardingRoot: Starts onboarding
OnboardingRoot->>OnboardingHeader: Show progress/header
OnboardingRoot->>OnboardingStepRoot: Render current step
OnboardingStepRoot->>ProfileSetupStep: Step 1 (profile)
OnboardingStepRoot->>RoleSetupStep: Step 2 (role)
OnboardingStepRoot->>UseCaseSetupStep: Step 3 (use case)
OnboardingStepRoot->>WorkspaceSetupStep: Step 4 (workspace)
OnboardingStepRoot->>InviteTeamStep: Step 5 (invite team)
User->>OnboardingRoot: Navigates steps
OnboardingRoot->>User: Updates onboarding state
Estimated code review effort🎯 5 (Critical) | ⏱️ ~80+ minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 30
🔭 Outside diff range comments (6)
apps/space/core/components/account/terms-and-conditions.tsx (1)
6-12: Prop shape now diverges from the new auth APIOther auth components were migrated to
authType: EAuthModes, but this component still exposesisSignUp?: boolean. Inconsistent props will cause type mismatches when composing the revamped flow.Recommend aligning with the enum API before merge.
apps/web/core/components/onboarding/create-workspace.tsx (1)
168-181: Sanitize slug consistently when it is typed directly
onChangefor the Name field generates a slug with.replace(/ /g, "-"), but the Slug input’sonChangehandler only lower-cases the value.
This means if a user types invalid characters (e.g. “@”) into the slug box, the internalslugvalue may contain them while the rendered value is cleaned via.replace()in thevalueprop. The mismatch can cause a “jumping-cursor” UX issue and forces back-and-forth validation.- onChange={(e) => { - if (/^[a-zA-Z0-9_-]+$/.test(e.target.value)) setInvalidSlug(false); - else setInvalidSlug(true); - onChange(e.target.value.toLowerCase()); - }} + onChange={(e) => { + const sanitized = e.target.value + .toLowerCase() + .trim() + .replace(/ /g, "-"); + setInvalidSlug(!/^[a-z0-9_-]+$/.test(sanitized)); + onChange(sanitized); + }}A small helper like
slugify(value)would centralise this logic and keep state/UI in sync.apps/web/core/components/onboarding/invite-members.tsx (1)
203-205: Fix inconsistent CSS class usage.The stroke classes still use the old
onboarding-*naming convention while other parts of the component have been updated tocustom-*classes.Apply this diff to maintain consistency:
<ChevronDown className={`size-3 ${ !getValues(`emails.${index}.role_active`) - ? "stroke-onboarding-text-400" - : "stroke-onboarding-text-100" + ? "stroke-custom-text-400" + : "stroke-custom-text-100" }`} />apps/web/core/components/account/auth-forms/auth-header.tsx (1)
14-20: Update type definition to remove unused propsThe
TAuthHeadertype includes props that are no longer used in the simplified component.type TAuthHeader = { workspaceSlug: string | undefined; invitationId: string | undefined; invitationEmail: string | undefined; - authMode: EAuthModes; - currentAuthStep: EAuthSteps; };apps/web/core/components/onboarding/profile-setup.tsx (2)
119-133: Improve error handling in handleSetPassword.The function catches errors but doesn't propagate them or inform the user of failures. This could lead to silent failures.
const handleSetPassword = async (password: string) => { const token = await authService.requestCSRFToken().then((data) => data?.csrf_token); - await authService + return await authService .setPassword(token, { password }) .then(() => { captureSuccess({ eventName: AUTH_TRACKER_EVENTS.password_created, }); }) - .catch(() => { + .catch((error) => { captureError({ eventName: AUTH_TRACKER_EVENTS.password_created, }); + throw error; // Re-throw to handle in the calling function }); };
186-200: Handle partial failures in Promise.all.Using
Promise.allhere means if password setting fails but user details update succeeds, the user will see an error but their details will be updated. Consider handling these operations separately or usingPromise.allSettled.try { - await Promise.all([ - updateCurrentUser(userDetailsPayload), - formData.password && handleSetPassword(formData.password), - ]).then(() => { + await updateCurrentUser(userDetailsPayload); + + if (formData.password) { + try { + await handleSetPassword(formData.password); + captureView({ + elementName: ONBOARDING_TRACKER_ELEMENTS.PASSWORD_CREATION_SELECTED, + }); + } catch (error) { + // Password setting failed, but user details were updated + setToast({ + type: TOAST_TYPE.WARNING, + title: "Partial Success", + message: "Profile updated but password setting failed. Please try setting password later.", + }); + captureView({ + elementName: ONBOARDING_TRACKER_ELEMENTS.PASSWORD_CREATION_FAILED, + }); + } + } else { captureView({ - elementName: formData.password - ? ONBOARDING_TRACKER_ELEMENTS.PASSWORD_CREATION_SELECTED - : ONBOARDING_TRACKER_ELEMENTS.PASSWORD_CREATION_SKIPPED, + elementName: ONBOARDING_TRACKER_ELEMENTS.PASSWORD_CREATION_SKIPPED, }); - setProfileSetupStep(EProfileSetupSteps.USER_PERSONALIZATION); - }); + } + + setProfileSetupStep(EProfileSetupSteps.USER_PERSONALIZATION); } catch {
♻️ Duplicate comments (4)
apps/space/public/favicon/site.webmanifest (2)
2-3: Populatenameandshort_namejust like the admin manifest
Replicate the branded values here to keep both apps consistent.
5-6: Same absolute-path + maskable-icon considerations as in admin manifestapps/web/core/components/common/latest-feature-block.tsx (1)
33-34: See previous remark – reuse the computedbgClassOnce
bgClassis extracted, replace this conditional with${bgClass}to keep styles consistent.apps/web/core/components/workspace/settings/workspace-details.tsx (1)
170-172: Same inline hex colour – see earlier feedbackThe placeholder avatar now uses
bg-[#026292]. Please apply the tokenised colour strategy suggested forworkspace/logo.tsxto keep styling centrally managed.
🧹 Nitpick comments (30)
apps/web/core/components/common/logo-spinner.tsx (1)
13-15: Improve spinner accessibility and semanticsThe
alttext"logo"does not convey the purpose of this animated spinner and will be read aloud by screen readers on every render.
If the GIF is purely decorative during loading, setalt=""and addrole="status"(oraria-hidden="true").
If you want it announced, use a meaningful description such as"Loading …".- <Image src={logoSrc} alt="logo" className="h-10 w-auto sm:h-20" /> + <Image + src={logoSrc} + alt="" + role="status" + aria-label="Loading..." + className="h-10 w-auto sm:h-20" + />apps/admin/public/favicon/site.webmanifest (1)
5-6: Confirm absolute icon paths + add maskable purpose
The leading slash makes the URLs resolve from the domain root. If the admin app is ever hosted under a sub-path (e.g./admin/behind a reverse proxy), the icons will 404. Consider using a relative path ("favicon/…") or adding a build-time prefix.
While touching the icon array, add"purpose": "any maskable"so Android uses the adaptive icon when available:- { "src": "/favicon/android-chrome-192x192.png", "sizes": "192x192", "type": "image/png" }, - { "src": "/favicon/android-chrome-512x512.png", "sizes": "512x512", "type": "image/png" } + { "src": "/favicon/android-chrome-192x192.png", "sizes": "192x192", "type": "image/png", "purpose": "any maskable" }, + { "src": "/favicon/android-chrome-512x512.png", "sizes": "512x512", "type": "image/png", "purpose": "any maskable" }apps/web/core/components/common/latest-feature-block.tsx (2)
16-16: Nit: apply text colour on the anchor instead of its wrapper
text-custom-text-100is applied to the whole paragraph, but the anchor inside already inherits link-specific styling. You can avoid overriding default link colours (especially in dark mode) by moving the text-colour class to the<span>/<Link>element, leaving the paragraph to inherit the body colour.
24-26: Duplicate conditional class logic – extract once for maintainabilityThe dark-mode background decision is hard-coded twice (here and in Line 33). A single
const bgClass = …declared beforereturnkeeps the markup cleaner and eliminates the risk of the two diverging.+ const bgClass = + resolvedTheme === "dark" ? "bg-custom-background-100" : "bg-custom-primary-70"; @@ - className={`mx-auto mt-8 overflow-hidden rounded-md border border-custom-border-200 object-cover sm:h-52 sm:w-96 ${ - resolvedTheme === "dark" ? "bg-custom-background-100" : "bg-custom-primary-70" - }`} + className={`mx-auto mt-8 overflow-hidden rounded-md border border-custom-border-200 object-cover sm:h-52 sm:w-96 ${bgClass}`}Apply the same
bgClassin the<Image>below.apps/web/core/components/workspace/logo.tsx (1)
20-21: Hard-coding brand color reduces theme flexibilityPreviously the component relied on the design-token class
bg-custom-primary-500; switching to a literal hex (bg-[#026292]) bakes the colour straight into markup, making future re-branding, dark-mode tweaks or Tailwind theme overrides harder.-!props.logo && "rounded bg-[#026292] text-white" +!props.logo && "rounded bg-custom-primary-500 text-white"If the new hue is intentional, add it to the Tailwind config (e.g.
theme.colors.brand) and reference via a semantic utility (bg-brand) instead of inline hex.apps/web/core/components/workspace/sidebar/dropdown-item.tsx (1)
52-53: Inline colour literal – consider design tokenRepeating the raw hex here duplicates styling logic and risks drift if the brand colour changes. Recommend moving the colour to a shared Tailwind class/variable and re-using it.
apps/admin/app/(all)/(dashboard)/sidebar-dropdown.tsx (1)
44-46:truncatemay not take effect on an inline<span>Tailwind’s
truncateutility relies onoverflow-hidden,whitespace-nowrap, andtext-overflow: ellipsis, which only clip on block-level or inline-block elements. A plain inline<span>often fails to apply the ellipsis, so very long emails could still spill.-<span className="px-2 text-custom-sidebar-text-200 truncate">{currentUser?.email}</span> +<span className="block px-2 text-custom-sidebar-text-200 truncate">{currentUser?.email}</span>Optionally add a
title={currentUser?.email}for hover reveal.apps/web/core/components/workspace/sidebar/user-menu-root.tsx (1)
105-107: Ensuretruncateactually truncatesSame concern as in the admin sidebar: an inline
<span>may not clip, so the ellipsis might not show up. Consider promoting it toblock/inline-block(or addblockclass) and optionally expose the full email via thetitleattribute for accessibility.-<span className="px-2 text-custom-sidebar-text-200 truncate">{currentUser?.email}</span> +<span className="block px-2 text-custom-sidebar-text-200 truncate" title={currentUser?.email}> + {currentUser?.email} +</span>apps/space/styles/globals.css (1)
15-33: Duplicated palette values – consider centralizing tokensThe exact same
--color-primary-*values appear in three differentglobals.cssfiles (admin,space,web). Keeping hard-coded RGB literals in multiple places will drift quickly and makes brand re-theming expensive.-/* in every app … */ +@import "@/styles/tokens/colors.css"; /* or expose a JS/TS design-tokens module that Tailwind consumes */A shared source-of-truth (design-tokens file, PostCSS
:where()import, or Tailwind theme extension) removes duplication while guaranteeing all surfaces stay in sync.apps/admin/styles/globals.css (1)
27-45: Same colour literals duplicated across appsSee comment in
apps/space/styles/globals.css. Consider extracting these values into a shared token to avoid triple maintenance.apps/web/core/components/onboarding/index.ts (1)
1-11: Nit: maintain alphabetical grouping of barrel exportsFor long barrels like this, keeping exports alphabetically ordered reduces merge-conflict churn and eases scanning.
export * from "./create-or-join-workspaces"; +export * from "./create-workspace"; +export * from "./header"; +export * from "./invite-members"; +export * from "./invitations"; +export * from "./profile-setup"; export * from "./root"; -export * from "./create-workspace"; -export * from "./invite-members"; -export * from "./invitations"; -export * from "./header"; export * from "./step-indicator"; export * from "./switch-account-dropdown"; export * from "./switch-account-modal";apps/web/core/components/onboarding/steps/index.ts (1)
1-6: Minor ordering / maintainability nitFor quicker grepping and merge-conflict avoidance, re-export blocks are usually kept alphabetical:
export * from "./profile"; export * from "./role"; +export * from "./root"; export * from "./team"; export * from "./usecase"; -export * from "./root";Not critical, just increases scanability.
apps/web/core/lib/n-progress/AppProgressBar.tsx (1)
13-14: Add CSS-variable fallback for robustnessIf
--color-primary-100isn’t defined on some pages the bar becomes transparent. Provide a default:- color = "rgb(var(--color-primary-100))", + color = "rgb(var(--color-primary-100, 10 47 255))", // #0A2FFF fallbackAlso consider extracting the string into a
const defaultColorto avoid repeating it in the template literal below.apps/web/core/components/onboarding/create-workspace.tsx (1)
213-217: Avoid directwindowaccess in JSXInside a
"use client"component this is safe, but a defensive pattern avoids potential reference errors if the component is ever rendered server-side:- <span className="whitespace-nowrap text-sm">{window && window.location.host}/</span> + <span className="whitespace-nowrap text-sm"> + {typeof window !== "undefined" ? window.location.host : ""} + / + </span>Not critical today, but future-proofs the component against accidental SSR.
apps/web/core/components/account/auth-forms/common/header.tsx (1)
4-6: Use semantic headings for accessibilityThe header is rendered with
<span>elements. Screen-readers will not announce them as headings, and the page will miss a heading outline.- <span className="text-2xl font-semibold text-custom-text-100">{title}</span> - <span className="text-2xl font-semibold text-custom-text-400">{description}</span> + <h1 className="text-2xl font-semibold text-custom-text-100">{title}</h1> + <p className="text-2xl font-semibold text-custom-text-400">{description}</p>This keeps the visual style while improving semantics.
apps/web/core/components/onboarding/steps/profile/consent.tsx (1)
11-24: Consider accessibility improvements for the custom checkbox.The component implementation is clean and functional, but could benefit from better accessibility:
<button type="button" onClick={() => handleChange(!isChecked)} + role="checkbox" + aria-checked={isChecked} + aria-labelledby="marketing-consent-label" className={`size-4 rounded border-2 flex items-center justify-center ${ isChecked ? "bg-custom-primary-100 border-custom-primary-100" : "border-custom-border-300" }`} > {isChecked && <Check className="w-3 h-3 text-white" />} </button> - <span className="text-sm text-custom-text-300">I agree to Plane marketing communications</span> + <span id="marketing-consent-label" className="text-sm text-custom-text-300">I agree to Plane marketing communications</span>This improves screen reader support and follows ARIA best practices for custom checkbox implementations.
apps/web/core/components/onboarding/steps/root.tsx (1)
19-40: Well-structured step routing component with good performance optimization.The component effectively centralizes onboarding step rendering with proper memoization and consistent prop passing. The layout provides good centering and responsive design.
Minor observation: Line 28 uses
invitations ?? []for null safety, but theinvitationsprop is typed asIWorkspaceMemberInvitation[](non-optional). Consider either:
- Making the prop optional in the type definition if null values are expected
- Removing the null coalescing if the prop is guaranteed to be an array
The current implementation works correctly but the type and null safety don't align perfectly.
apps/web/core/components/onboarding/switch-account-dropdown.tsx (2)
37-50: Consider adding a dropdown indicator for better UX.The avatar implementation and button styling look good, but removing the
ChevronDownicon might make it less obvious to users that this is a dropdown. Consider adding a visual indicator to maintain discoverability.<span className="text-sm font-medium text-custom-text-200">{displayName}</span> + <ChevronDown className="w-4 h-4 text-custom-text-400" />
70-70: Consider internationalizing the hardcoded text.The "Wrong e-mail address?" text should be internationalized for consistency with the rest of the application.
- Wrong e-mail address? + {t("onboarding.switch_account.wrong_email")}apps/space/core/components/account/auth-forms/auth-root.tsx (1)
161-196: Extract OAuth providers to a configuration constantConsider extracting the OAuth configuration to a separate constant outside the component for better maintainability and testability.
+const OAUTH_PROVIDERS = { + GOOGLE: { id: "google", name: "Google" }, + GITHUB: { id: "github", name: "GitHub" }, + GITLAB: { id: "gitlab", name: "GitLab" }, +} as const; export const AuthRoot: FC = observer(() => { // ... existing code ... - const OAuthConfig = [ - { - id: "google", - text: `${content} with Google`, - // ... rest of config - }, - // ... other providers - ]; + const OAuthConfig = [ + { + id: OAUTH_PROVIDERS.GOOGLE.id, + text: `${content} with ${OAUTH_PROVIDERS.GOOGLE.name}`, + icon: <Image src={GoogleLogo} height={18} width={18} alt={`${OAUTH_PROVIDERS.GOOGLE.name} Logo`} />, + onClick: () => { + window.location.assign(`${API_BASE_URL}/auth/${OAUTH_PROVIDERS.GOOGLE.id}/${nextPath ? `?next_path=${encodeURIComponent(nextPath)}` : ``}`); + }, + enabled: config?.is_google_enabled, + }, + // ... similar for other providers + ];apps/web/core/components/account/auth-forms/reset-password.tsx (1)
102-106: Consider using programmatic form submission for better error handlingThe form uses traditional POST submission which makes error handling difficult. Consider using programmatic submission like in
SetPasswordForm.This would allow:
- Better error handling with toast notifications
- Consistent UX with other forms
- Prevention of page reload on errors
- Better analytics tracking
apps/web/core/components/onboarding/steps/workspace/join-invites.tsx (1)
81-83: Remove redundant condition checkThe condition
invitations && invitations.length > 0is already checked in line 77.- {invitations && - invitations.length > 0 && - invitations.map((invitation) => { + {invitations.map((invitation) => {apps/web/core/components/account/auth-forms/auth-header.tsx (2)
4-4: Remove unused importThe
useTranslationhook is imported but thetfunction is only used once. Since the component has been simplified to use static strings, this import appears unnecessary.-import { useTranslation } from "@plane/i18n";
107-107: Remove unnecessary translation wrapperSince
headeris now always a string in the simplified version, the translation check is redundant.- {typeof header === "string" ? t(header) : header} + {header}apps/web/core/components/onboarding/header.tsx (1)
44-56: Consider extracting step order configurationThe step order array could be extracted as a constant for better maintainability.
+const getStepOrder = (hasInvitations: boolean): TOnboardingStep[] => [ + EOnboardingSteps.PROFILE_SETUP, + EOnboardingSteps.ROLE_SETUP, + EOnboardingSteps.USE_CASE_SETUP, + ...(hasInvitations + ? [EOnboardingSteps.WORKSPACE_CREATE_OR_JOIN] + : [EOnboardingSteps.WORKSPACE_CREATE_OR_JOIN, EOnboardingSteps.INVITE_MEMBERS]), +]; const getCurrentStepNumber = (): number => { - const stepOrder: TOnboardingStep[] = [ - EOnboardingSteps.PROFILE_SETUP, - EOnboardingSteps.ROLE_SETUP, - EOnboardingSteps.USE_CASE_SETUP, - ...(hasInvitations - ? [EOnboardingSteps.WORKSPACE_CREATE_OR_JOIN] - : [EOnboardingSteps.WORKSPACE_CREATE_OR_JOIN, EOnboardingSteps.INVITE_MEMBERS]), - ]; + const stepOrder = getStepOrder(hasInvitations); return stepOrder.indexOf(currentStep) + 1; };apps/web/core/components/onboarding/steps/profile/root.tsx (1)
145-146: Simplify button disabled logicThe nested ternary operators make the logic hard to follow.
- const isButtonDisabled = - !isSubmitting && isValid ? (isPasswordAlreadySetup ? false : isValidPassword ? false : true) : true; + const isButtonDisabled = isSubmitting || !isValid || (!isPasswordAlreadySetup && !isValidPassword);apps/web/app/(all)/onboarding/page.tsx (1)
17-17: Remove unused import.
WorkspaceContentWrapperis imported but not used in the component.-import { WorkspaceContentWrapper } from "@/plane-web/components/workspace";apps/web/core/components/onboarding/profile-setup.tsx (1)
292-575: Well-structured form implementation.The form is properly implemented with good validation rules and error handling. Consider breaking this into smaller sub-components in the future for better maintainability.
apps/web/app/(all)/accounts/reset-password/page.tsx (1)
4-4: Remove unused importEAuthModesThe
EAuthModesenum is imported but only used as a value accessor. Consider importing just the value you need or removing if truly unused.-import { EAuthModes } from "@plane/constants";apps/web/core/components/onboarding/steps/team/root.tsx (1)
256-256: Replace curly apostrophe with straight apostropheUse a straight apostrophe to avoid potential encoding issues.
- <span className="mt-1 text-xs text-red-500">That doesn{"'"}t look like an email address.</span> + <span className="mt-1 text-xs text-red-500">That doesn't look like an email address.</span>
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: 30
🔭 Outside diff range comments (3)
apps/web/styles/globals.css (1)
424-428: Missing semicolon breaks two CSS custom-properties
--color-text-350and--color-text-400are concatenated on one line without a semicolon, which invalidates both variables and everything that follows inside the rule:--color-text-350: 190, 190, 190 --color-text-400: 115, 115, 115;Suggested fix:
--color-text-350: 190, 190, 190; /* tertiary-plus text */ --color-text-400: 115, 115, 115; /* placeholder text */Without this, any Tailwind class that resolves to
text-custom-text-350/400(or directly references these vars) will silently fall back to unset values.apps/web/core/components/onboarding/invite-members.tsx (1)
202-205: Update stroke colors to use custom tokens for consistency.These stroke colors still use onboarding-specific tokens while the rest of the styling has been updated to custom tokens. This creates inconsistency.
<ChevronDown className={`size-3 ${ !getValues(`emails.${index}.role_active`) - ? "stroke-onboarding-text-400" - : "stroke-onboarding-text-100" + ? "stroke-custom-text-400" + : "stroke-custom-text-100" }`} />apps/web/core/components/onboarding/profile-setup.tsx (1)
77-77: Move service instantiation inside the component.Instantiating the service outside the component can cause issues with server-side rendering.
-const authService = new AuthService(); export const ProfileSetup: React.FC<Props> = observer((props) => { const { user, totalSteps, stepChange, finishOnboarding } = props; + // services + const authService = useMemo(() => new AuthService(), []);
♻️ Duplicate comments (4)
apps/web/core/components/workspace/sidebar/dropdown-item.tsx (1)
51-53: Duplicate inline colour – push into a shared Tailwind tokenSame concern as in
workspace/logo.tsx: repeatingbg-[#026292]increases maintenance cost and risks divergence if the colour changes again.Refactor to reuse a shared class (e.g.
bg-brand-primary) after adding it totailwind.config.jsto keep brand colours single-sourced.apps/web/core/components/workspace/settings/workspace-details.tsx (1)
170-172: Consistency & accessibility follow-upYet another occurrence of the hard-coded blue. After replacing other spots, update this one too to the semantic class to stay DRY and ensure the WCAG contrast fix propagates everywhere.
apps/web/core/store/user/profile.store.ts (1)
59-60: Same type-sync concern as space storeConfirm
has_marketing_email_consentexists in the sharedTUserProfileinterface; keep both stores and the API schema in lock-step.apps/web/core/components/account/auth-forms/password.tsx (1)
120-134: Same duplicate “email” field issue as space variant.See comment above – remove
name="email"from the disabled input to avoid accidental double submission.
🧹 Nitpick comments (42)
apps/space/public/favicon/site.webmanifest (1)
2-3: Optional: provide meaningfulname/short_nameBoth properties are still empty. Filling them improves PWA install UX and accessibility.
apps/web/core/components/workspace/sidebar/user-menu-root.tsx (1)
105-107: Expose full email viatitleto preserve accessibility & copy-abilityAdding
truncateprevents overflow but also hides the full address from keyboard-only users and makes copying harder. A lightweight fix is to add atitle(or tooltip) so the full text is still discoverable:-<span className="px-2 text-custom-sidebar-text-200 truncate">{currentUser?.email}</span> +<span + className="px-2 text-custom-sidebar-text-200 truncate" + title={currentUser?.email ?? ""} +> + {currentUser?.email} +</span>Keeps the visual change while preserving UX and a11y.
apps/admin/app/(all)/(dashboard)/sidebar-dropdown.tsx (1)
44-46: Sametitleimprovement applies here – consider a small shared componentReplicate the
titleattribute (see previous comment) so admins can still view / copy long emails:-<span className="px-2 text-custom-sidebar-text-200 truncate">{currentUser?.email}</span> +<span + className="px-2 text-custom-sidebar-text-200 truncate" + title={currentUser?.email ?? ""} +> + {currentUser?.email} +</span>Given both files now contain identical markup, extracting a tiny
<UserEmail>component would avoid duplication.apps/web/core/components/onboarding/step-indicator.tsx (1)
39-39: Ensure text token exists & provides readable contrast
text-custom-text-300relies on a new text scale. Double-check that 300 has ≥4.5 : 1 contrast on the chosen background; otherwise pick 400+.apps/web/core/components/auth-screens/index.ts (1)
4-5: Barrel exports: keep alphabetical order to curb merge conflictsConsider ordering the export list alphabetically (
auth-base,header, …) to make future diffs trivial.apps/web/core/components/global/product-updates/footer.tsx (1)
60-62: Provide an accessible text alternative for the SVG logo
PlaneLogorenders a purely decorative SVG but, unlike the previous<Image>tag, it exposes no implicitaltattribute. Screen-reader users will now hear nothing between the button text and “Powered by Plane Pages”. Addaria-hiddenor a visually-hidden label to ensure accessibility compliance.-<PlaneLogo className="h-4 w-auto text-custom-text-100" /> +<PlaneLogo + className="h-4 w-auto text-custom-text-100" + aria-hidden="true" /* decorative */ +/>apps/space/core/components/account/auth-forms/unique-code.tsx (1)
129-134: Consider usingclsx/cnfor clearer conditional classesInline ternaries inside template literals are hard to scan. We already import
cnelsewhere in the repo; using it would keep the condition readable and prevent accidental whitespace glitches.- className={`${ - isRequestNewCodeDisabled - ? "text-custom-text-400" - : "font-medium text-custom-primary-300 hover:text-custom-primary-200" - }`} + className={cn( + isRequestNewCodeDisabled + ? "text-custom-text-400" + : "font-medium text-custom-primary-300 hover:text-custom-primary-200" + )}apps/web/core/components/common/latest-feature-block.tsx (1)
33-34: Minor redundancyThe ternary inside the class list repeats the exact same colour logic you already applied on the parent wrapper (
divat Line 24). Unless the child really needs its own background override, you can drop the duplicate to avoid future divergence.apps/web/core/layouts/auth-layout/workspace-wrapper.tsx (1)
14-14: LGTM! Consider cleanup of unused imports and variables.The replacement with
PlaneLogocomponent is consistent with the broader refactor. However, since the static logo images and theme-based logic are no longer needed, consider removing the unused imports and variables.Apply this diff to clean up unused code:
-import PlaneBlackLogo from "@/public/plane-logos/black-horizontal-with-blue-logo.png"; -import PlaneWhiteLogo from "@/public/plane-logos/white-horizontal-with-blue-logo.png";- const planeLogo = resolvedTheme === "dark" ? PlaneWhiteLogo : PlaneBlackLogo;Also applies to: 153-153
apps/web/core/components/account/auth-forms/common/header.tsx (1)
3-8: Consider improving visual hierarchy between title and description.Both the title and description use identical styling (
text-2xl font-semibold) with only color differentiation. Consider making the description smaller or lighter weight to establish better visual hierarchy.- <span className="text-2xl font-semibold text-custom-text-400">{description}</span> + <span className="text-lg font-medium text-custom-text-400">{description}</span>apps/web/core/components/account/auth-forms/common/container.tsx (1)
4-4: Consider removing redundant margin-top.The container already has vertical padding (
py-6), so themt-10class might be redundant. Consider whether both are necessary for the intended spacing.- <div className="flex flex-col justify-center items-center flex-grow w-full py-6 mt-10"> + <div className="flex flex-col justify-center items-center flex-grow w-full py-6">apps/web/core/components/project/card-list.tsx (1)
88-91: Alt text should be localised
alt="No matching projects"is hard-coded English. Wrap this int()so it’s translated like the surrounding copy:- alt="No matching projects" + alt={t("workspace_projects.empty_state.filter.no_match_alt")}apps/web/core/components/onboarding/invitations.tsx (1)
78-80: Hard-coded copy bypasses i18n
"You are invited!"and the subtitle are plain strings, unlike the rest of the onboarding flow that usesuseTranslation. Add translation keys so these headings are localisable.apps/space/core/components/views/header.tsx (1)
7-12: Prefer semantic<header>element for accessibilityUsing a semantic element improves screen-reader navigation and SEO. Minor tweak:
-export const AuthHeader = () => ( - <div className="flex items-center justify-between gap-6 w-full flex-shrink-0 sticky top-0"> +export const AuthHeader = () => ( + <header className="flex items-center justify-between gap-6 w-full flex-shrink-0 sticky top-0"> … - </div> + </header> );apps/web/core/components/auth-screens/footer.tsx (1)
28-37: Brand icons are decorative – addaria-hiddenfor assistive techSVG components used only for decoration should be hidden from screen readers:
- icon: <ZerodhaLogo className="h-7 w-24 text-[#387ED1]" />, + icon: <ZerodhaLogo aria-hidden className="h-7 w-24 text-[#387ED1]" />,Apply to all four logos.
apps/web/core/components/onboarding/steps/common/header.tsx (1)
10-14: Add i18n support for title & description.All visible strings should leverage the translation hook (
useTranslation) used elsewhere in onboarding to avoid hard-coding English copy.-import { FC } from "react"; +import { FC } from "react"; +import { useTranslation } from "@plane/i18n"; @@ -export const CommonOnboardingHeader: FC<Props> = ({ title, description }) => ( - <div className="text-left space-y-2"> - <h1 className="text-2xl font-semibold text-custom-text-200">{title}</h1> - <p className="text-base text-custom-text-300">{description}</p> - </div> -); +export const CommonOnboardingHeader: FC<Props> = ({ title, description }) => { + const { t } = useTranslation(); + return ( + <div className="text-left space-y-2"> + <h1 className="text-2xl font-semibold text-custom-text-200">{t(title)}</h1> + <p className="text-base text-custom-text-300">{t(description)}</p> + </div> + ); +};apps/space/core/components/account/auth-forms/password.tsx (2)
184-212: Duplicate “email” fields – dropnamefrom the disabled input.The hidden field at Line 181 already submits
Keepingname="email"on the disabled visible input is redundant and easy to forget if thedisabledattr gets removed later, causing duplicate values in the request body.-<Input - id="email" - name="email" +<Input + id="email" type="email" ... disabled />
250-289: Associate error message viaaria-describedbyfor accessibility.Screen-reader users will not automatically connect the “Passwords don’t match” span to the confirm-password field. Providing
aria-describedbyon the input (and anidon the error span) fixes this.-<Input +<Input ... onBlur={() => setIsRetryPasswordInputFocused(false)} + aria-describedby={passwordFormData.password !== passwordFormData.confirm_password ? "confirmPwdError" : undefined} /> ... - {!!passwordFormData.confirm_password && + {!!passwordFormData.confirm_password && passwordFormData.password !== passwordFormData.confirm_password && renderPasswordMatchError && ( - <span className="text-sm text-red-500">{t("auth.common.password.errors.match")}</span> + <span id="confirmPwdError" className="text-sm text-red-500"> + {t("auth.common.password.errors.match")} + </span> )}apps/space/core/components/account/auth-forms/auth-header.tsx (1)
32-32: Consider keeping the children prop for component flexibility.Removing the
childrenprop reduces the component's reusability. If this component needs to support dynamic content in the future, you'll need to reintroduce this prop.apps/web/core/components/onboarding/switch-account-dropdown.tsx (1)
37-50: Consider design system alignment and fallback logic improvements.The avatar rendering simplification is good, but there are a few considerations:
- The hardcoded
bg-green-700color may not align with the design system - consider using a custom color variable- The fallback logic
fullName?.[0] ?? "R"uses an arbitrary "R" - consider using the first letter ofdisplayNameoruser?.emailfor better contextConsider this improvement:
- <div className="size-6 rounded-full bg-green-700 flex items-center justify-center text-white font-semibold text-sm capitalize"> + <div className="size-6 rounded-full bg-custom-primary-100 flex items-center justify-center text-white font-semibold text-sm capitalize"> {user?.avatar_url ? ( <img src={getFileURL(user?.avatar_url)} alt={user?.display_name} className="w-full h-full rounded-full object-cover" /> ) : ( - <>{fullName?.[0] ?? "R"}</> + <>{displayName?.[0]?.toUpperCase() ?? "U"}</> )} </div>apps/web/core/components/account/auth-forms/forgot-password.tsx (1)
91-91: Consider using translation keys for consistency.The title and description are hard-coded while other text uses translation functions. For consistency with the rest of the component, consider using translation keys.
- <AuthFormHeader title="Reset password" description="Regain access to your account." /> + <AuthFormHeader title={t("auth.forgot_password.title")} description={t("auth.forgot_password.description")} />apps/web/core/components/account/auth-forms/set-password.tsx (3)
24-33: Minor type definition improvement.The type name
TResetPasswordFormValuesis misleading since this is for setting a password, not resetting it. Consider renaming for clarity.-type TResetPasswordFormValues = { +type TSetPasswordFormValues = { email: string; password: string; confirm_password?: string; }; -const defaultValues: TResetPasswordFormValues = { +const defaultValues: TSetPasswordFormValues = { email: "", password: "", };
78-86: Improve button disabled logic for better UX.The current logic uses a ternary operator that always returns a boolean, making the ternary unnecessary. Also, the logic could be more readable.
const isButtonDisabled = useMemo( () => - !!passwordFormData.password && - getPasswordStrength(passwordFormData.password) === E_PASSWORD_STRENGTH.STRENGTH_VALID && - passwordFormData.password === passwordFormData.confirm_password - ? false - : true, + !passwordFormData.password || + getPasswordStrength(passwordFormData.password) !== E_PASSWORD_STRENGTH.STRENGTH_VALID || + passwordFormData.password !== passwordFormData.confirm_password, [passwordFormData] );
132-132: Remove commented code.There are commented-out error handling lines that should be cleaned up.
- //hasError={Boolean(errors.email)}- //hasError={Boolean(errors.password)}Also applies to: 150-150
apps/web/core/components/onboarding/root.tsx (1)
111-132: Include userProfile in useEffect dependencies.The useEffect uses
userProfilebut doesn't include it in the dependency array. While the current implementation might work, it's better to be explicit about dependencies.handleInitialStep(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [userProfile]);apps/web/core/components/onboarding/header.tsx (2)
27-39: Add default case to handleStepBack switch statement.While the current implementation works, adding a default case improves code robustness.
const handleStepBack = () => { switch (currentStep) { case EOnboardingSteps.ROLE_SETUP: updateCurrentStep(EOnboardingSteps.PROFILE_SETUP); break; case EOnboardingSteps.USE_CASE_SETUP: updateCurrentStep(EOnboardingSteps.ROLE_SETUP); break; case EOnboardingSteps.WORKSPACE_CREATE_OR_JOIN: updateCurrentStep(EOnboardingSteps.USE_CASE_SETUP); break; + default: + // No action needed for other steps + break; } };
74-78: Add aria-label for better accessibility.The back button should have an aria-label for screen reader users.
-<button onClick={handleStepBack} className="cursor-pointer" type="button" disabled={!canGoBack}> +<button + onClick={handleStepBack} + className="cursor-pointer" + type="button" + disabled={!canGoBack} + aria-label="Go back to previous step" +> <ChevronLeft className="size-6 text-custom-text-400" /> </button>apps/web/core/components/account/auth-forms/auth-header.tsx (1)
23-52: Simplify repetitive Titles structure.Since all auth modes and steps have identical header/subHeader values, the nested structure adds unnecessary complexity.
-const Titles = { - [EAuthModes.SIGN_IN]: { - [EAuthSteps.EMAIL]: { - header: "Work in all dimensions.", - subHeader: "Welcome back to Plane.", - }, - [EAuthSteps.PASSWORD]: { - header: "Work in all dimensions.", - subHeader: "Welcome back to Plane.", - }, - [EAuthSteps.UNIQUE_CODE]: { - header: "Work in all dimensions.", - subHeader: "Welcome back to Plane.", - }, - }, - [EAuthModes.SIGN_UP]: { - [EAuthSteps.EMAIL]: { - header: "Work in all dimensions.", - subHeader: "Create your Plane account.", - }, - [EAuthSteps.PASSWORD]: { - header: "Work in all dimensions.", - subHeader: "Create your Plane account.", - }, - [EAuthSteps.UNIQUE_CODE]: { - header: "Work in all dimensions.", - subHeader: "Create your Plane account.", - }, - }, -}; +const DEFAULT_HEADER = "Work in all dimensions."; +const SUBHEADERS = { + [EAuthModes.SIGN_IN]: "Welcome back to Plane.", + [EAuthModes.SIGN_UP]: "Create your Plane account.", +};Then update the
getHeaderSubHeaderfunction to use these simplified constants.apps/space/core/components/account/auth-forms/auth-root.tsx (1)
159-159: Memoize OAuth button content.The content string is recreated on every render.
- const content = authMode === EAuthModes.SIGN_UP ? "Sign up" : "Sign in"; + const content = useMemo(() => authMode === EAuthModes.SIGN_UP ? "Sign up" : "Sign in", [authMode]);apps/web/core/components/onboarding/steps/workspace/join-invites.tsx (3)
65-65: Add context to error logging.The console.error should include more context about what failed.
- console.error(error); + console.error("Failed to join workspaces:", error);
77-134: Simplify conditional rendering.The condition
invitations && invitations.length > 0is checked twice unnecessarily.- return invitations && invitations.length > 0 ? ( + if (!invitations || invitations.length === 0) { + return <div>No Invitations found</div>; + } + + return ( <div className="flex flex-col gap-10"> <CommonOnboardingHeader title="Join invites or create a workspace" description="All your work — unified." /> <div className="flex flex-col gap-3"> - {invitations && - invitations.length > 0 && - invitations.map((invitation) => { + {invitations.map((invitation) => {
133-133: Internationalize the empty state message.The hardcoded message should use the translation system for consistency.
Add proper internationalization support for the empty state message to maintain consistency with the rest of the application.
apps/web/core/components/onboarding/profile-setup.tsx (3)
318-320: Use user's initial or a more meaningful fallback.The fallback to "R" for the avatar initial seems arbitrary when the first name is empty.
- {watch("first_name")[0] ?? "R"} + {watch("first_name")[0] ?? user?.first_name?.[0] ?? "U"}
462-464: Simplify password validation rule.The validation rule can be simplified for better readability.
validate: (value) => - watch("password") ? (value === watch("password") ? true : "Passwords don't match") : true, + !watch("password") || value === watch("password") || "Passwords don't match",
519-529: Extract chip selection into a reusable component.The chip selection UI is duplicated for both role and domain selection.
Consider extracting the chip selection logic into a reusable component to reduce code duplication:
const ChipSelector = ({ options, value, onChange, className }: ChipSelectorProps) => ( <div className="flex flex-wrap gap-2 py-2 overflow-auto break-all"> {options.map((option) => ( <div key={option} className={`flex-shrink-0 border-[0.5px] hover:cursor-pointer hover:bg-custom-background-90 ${ value === option ? "border-custom-primary-100" : "border-custom-border-300" } rounded px-3 py-1.5 text-sm font-medium`} onClick={() => onChange(option)} > {option} </div> ))} </div> );Also applies to: 549-560
apps/web/core/components/account/auth-forms/reset-password.tsx (1)
145-155: Extract password visibility toggle into a reusable component.The password visibility toggle logic is duplicated.
Consider creating a reusable password input component with built-in visibility toggle:
const PasswordInput = ({ showPassword, onToggle, ...inputProps }: PasswordInputProps) => ( <div className="relative flex items-center rounded-md bg-custom-background-100"> <Input type={showPassword ? "text" : "password"} {...inputProps} /> {showPassword ? ( <EyeOff className="absolute right-3 h-5 w-5 stroke-custom-text-400 hover:cursor-pointer" onClick={onToggle} /> ) : ( <Eye className="absolute right-3 h-5 w-5 stroke-custom-text-400 hover:cursor-pointer" onClick={onToggle} /> )} </div> );Also applies to: 174-184
apps/web/core/components/account/auth-forms/auth-root.tsx (1)
107-107: Follow consistent naming convention.Variable name should follow camelCase convention.
- const OauthButtonContent = authMode === EAuthModes.SIGN_UP ? "Sign up" : "Sign in"; + const oauthButtonContent = authMode === EAuthModes.SIGN_UP ? "Sign up" : "Sign in";apps/web/core/components/account/auth-forms/form-root.tsx (1)
20-30: Consider extracting handler types for better type safetyThe prop types include multiple handler functions. Consider creating specific types for these handlers to improve type safety and reusability across authentication components.
+type TAuthHandlers = { + setEmail: (email: string) => void; + setAuthMode: (authMode: EAuthModes) => void; + setAuthStep: (authStep: EAuthSteps) => void; + setErrorInfo: (errorInfo: TAuthErrorInfo | undefined) => void; +}; + type TAuthFormRoot = { authStep: EAuthSteps; authMode: EAuthModes; email: string; - setEmail: (email: string) => void; - setAuthMode: (authMode: EAuthModes) => void; - setAuthStep: (authStep: EAuthSteps) => void; - setErrorInfo: (errorInfo: TAuthErrorInfo | undefined) => void; currentAuthMode: EAuthModes; -}; +} & TAuthHandlers;apps/web/app/(all)/onboarding/page.tsx (1)
17-17: Remove unused importThe
WorkspaceContentWrapperimport is not used in this component.-import { WorkspaceContentWrapper } from "@/plane-web/components/workspace";apps/web/core/components/onboarding/steps/profile/root.tsx (2)
187-187: Remove unused hidden file inputThe hidden file input element is not used as the image upload is handled through the modal.
- <input type="file" className="hidden" id="profile-image-input" />
172-172: Consider making avatar background color configurableThe hardcoded color
#028375should be moved to a theme variable or configuration for consistency with the brand revamp.- className="size-12 rounded-full bg-[#028375] flex items-center justify-center text-white font-semibold text-xl" + className="size-12 rounded-full bg-custom-primary-500 flex items-center justify-center text-white font-semibold text-xl"apps/web/core/components/onboarding/steps/workspace/create.tsx (1)
166-169: Potential duplicate setValue callThe
setValue("name", event.target.value)call seems redundant sinceonChangealready updates the form value.onChange={(event) => { onChange(event.target.value); - setValue("name", event.target.value); setValue("slug", event.target.value.toLocaleLowerCase().trim().replace(/ /g, "-"), { shouldValidate: true, }); }}
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: 2
🧹 Nitpick comments (1)
apps/api/templates/emails/invitations/project_invitation.html (1)
11-11: Default link color still points to legacy paletteWhile the
.r16-rrule now uses the updated brand colour#006399, the global link style (a, a:link) defined a few lines below still hard-codes#0092ff. This results in mixed branding across the email (links in paragraphs vs. button/border colour). Consider updating that rule to the new token so all hyperlinks follow the refreshed palette.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/api/templates/emails/auth/forgot_password.html(6 hunks)apps/api/templates/emails/auth/magic_signin.html(6 hunks)apps/api/templates/emails/invitations/project_invitation.html(3 hunks)apps/api/templates/emails/invitations/workspace_invitation.html(5 hunks)apps/api/templates/emails/notifications/issue-updates.html(1 hunks)apps/api/templates/emails/notifications/project_addition.html(14 hunks)apps/api/templates/emails/notifications/webhook-deactivate.html(6 hunks)apps/api/templates/emails/user/user_activation.html(14 hunks)apps/api/templates/emails/user/user_deactivation.html(14 hunks)
✅ Files skipped from review due to trivial changes (8)
- apps/api/templates/emails/user/user_activation.html
- apps/api/templates/emails/notifications/issue-updates.html
- apps/api/templates/emails/invitations/workspace_invitation.html
- apps/api/templates/emails/user/user_deactivation.html
- apps/api/templates/emails/notifications/project_addition.html
- apps/api/templates/emails/auth/magic_signin.html
- apps/api/templates/emails/notifications/webhook-deactivate.html
- apps/api/templates/emails/auth/forgot_password.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
- GitHub Check: Build and lint web apps
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
♻️ Duplicate comments (2)
apps/web/core/components/onboarding/steps/team/root.tsx (2)
103-125: This complex email validation logic was flagged in previous reviews but remains unchanged.The
emailOnChangefunction still contains the same nested conditional logic that was identified as complex and hard to maintain in previous review comments. The suggested refactor to extract helper functions was not implemented.
335-335: This hardcoded role value issue was flagged but remains unaddressed.The hardcoded role value
15should be replaced with the enum constantEUserPermissions.MEMBERas noted in previous reviews. Based on the retrieved learnings,EUserPermissions.MEMBER = 15is the correct mapping.Apply this diff to use the enum constant:
const appendField = () => { - append({ email: "", role: 15, role_active: false }); + append({ email: "", role: EUserPermissions.MEMBER, role_active: false }); }; useEffect(() => { if (fields.length === 0) { append( [ - { email: "", role: 15, role_active: false }, - { email: "", role: 15, role_active: false }, - { email: "", role: 15, role_active: false }, + { email: "", role: EUserPermissions.MEMBER, role_active: false }, + { email: "", role: EUserPermissions.MEMBER, role_active: false }, + { email: "", role: EUserPermissions.MEMBER, role_active: false }, ],Also applies to: 342-344
🧹 Nitpick comments (3)
apps/web/core/components/onboarding/steps/team/root.tsx (3)
68-68: Move email regex to constants file for better maintainability.The email regex pattern should be moved to a constants file to promote reusability across the application and centralize validation patterns.
Consider moving this to a validation constants file:
-const emailRegex = /^[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}$/i;Then import it in this file:
import { EMAIL_REGEX } from '@plane/constants';
200-205: Inconsistent color token usage.The ChevronDown icon uses
stroke-onboarding-text-400andstroke-onboarding-text-100which are onboarding-specific tokens, while other elements usetext-custom-text-*tokens. This inconsistency should be addressed for proper theming.Apply this diff to use consistent color tokens:
<ChevronDown className={`size-3 ${ !getValues(`emails.${index}.role_active`) - ? "stroke-onboarding-text-400" - : "stroke-onboarding-text-100" + ? "stroke-custom-text-400" + : "stroke-custom-text-100" }`} />
222-222: Another inconsistent color token usage.The Listbox option uses
bg-onboarding-background-400/40while other elements usebg-custom-background-*tokens.Apply this diff for consistency:
- active || selected ? "bg-onboarding-background-400/40" : "" + active || selected ? "bg-custom-background-400/40" : ""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/(all)/sign-up/page.tsx(1 hunks)apps/web/core/components/onboarding/steps/team/root.tsx(1 hunks)apps/web/core/components/pages/pages-list-main-content.tsx(2 hunks)apps/web/core/components/project/card-list.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/web/core/components/pages/pages-list-main-content.tsx
- apps/web/core/components/project/card-list.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(all)/sign-up/page.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: all role enums in this codebase (euserpermissions, euserworkspaceroles, euserprojectroles) use the s...
Learnt from: prateekshourya29
PR: makeplane/plane#7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: All role enums in this codebase (EUserPermissions, EUserWorkspaceRoles, EUserProjectRoles) use the same consistent numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. None of these enums have a value of 0, so truthy checks work correctly with these enum values.
Applied to files:
apps/web/core/components/onboarding/steps/team/root.tsx
📚 Learning: the `euserpermissions` enum in this codebase uses numeric values: admin = 20, member = 15, guest = 5...
Learnt from: prateekshourya29
PR: makeplane/plane#7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: The `EUserPermissions` enum in this codebase uses numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. All roles in this enum are always numbers, not string values.
Applied to files:
apps/web/core/components/onboarding/steps/team/root.tsx
📚 Learning: in the apps/web/helpers/react-hook-form.helper.ts file, the getnestederror function's fielderror det...
Learnt from: prateekshourya29
PR: makeplane/plane#7363
File: apps/web/helpers/react-hook-form.helper.ts:9-19
Timestamp: 2025-07-08T13:41:19.928Z
Learning: In the apps/web/helpers/react-hook-form.helper.ts file, the getNestedError function's FieldError detection logic using "message" property check is sufficient for their use case, and more robust type checking is not required.
Applied to files:
apps/web/core/components/onboarding/steps/team/root.tsx
📚 Learning: in the plane project's smtp configuration implementation, the email configuration form (email-config...
Learnt from: gakshita
PR: makeplane/plane#7393
File: apps/admin/app/(all)/(dashboard)/email/email-config-form.tsx:104-104
Timestamp: 2025-07-14T11:22:43.964Z
Learning: In the Plane project's SMTP configuration implementation, the email configuration form (email-config-form.tsx) hardcodes ENABLE_SMTP to "1" in form submission because the form is only rendered when SMTP is enabled. The enable/disable functionality is managed at the page level (page.tsx) with a toggle, and the form only handles configuration details when SMTP is already enabled.
Applied to files:
apps/web/core/components/onboarding/steps/team/root.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (4)
apps/web/core/components/onboarding/steps/team/root.tsx (4)
1-34: Import organization and dependencies look good.The imports are well-organized and all necessary dependencies are included for the form management, UI components, and business logic requirements.
357-359: Good form submission prevention.The
onKeyDownhandler properly prevents accidental form submission when Enter is pressed, which is important for multi-input forms like this one.
290-332: Form submission logic is well-structured.The
onSubmitfunction properly filters empty emails, handles success/error cases with appropriate tracking and user feedback through toast notifications. The error handling and event tracking implementation follows good practices.
403-403: Form validation state management is appropriate.The button disable logic correctly combines multiple validation states (
isInvitationDisabled,!isValid,isSubmitting) to prevent invalid submissions while providing clear user feedback.
This PR includes the following updates:
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Documentation
Style
No user-facing functionality was removed; these changes enhance onboarding, authentication, and overall visual consistency.