-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Small update of color(verify email) #1869
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
base: main
Are you sure you want to change the base?
Conversation
@AshishViradiya153 is attempting to deploy a commit to the mftsio Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughIntroduces a new brand prop to EmailVerificationMessage for theming, adds client-side 6-digit code validation, and updates related views to pass brand through. Adjusts UI colors based on brand.accentColor, adds isEmptyCode state, and updates resend/submit handlers to manage validation state. Changes
Sequence Diagram(s)sequenceDiagram
actor U as User
participant E as EmailVerificationMessage
participant H as onSubmitHandler
U->>E: Enter code (onChange)
E->>E: Clear isInvalidCode, isEmptyCode
U->>E: Click "Verify"
alt Code missing or not 6 digits
E->>E: Set isEmptyCode = true<br/>Prevent submit
E-->>U: Show "Please enter the 6-digit verification code."
else Code is 6 digits
E->>E: Clear isEmptyCode
E->>H: Submit code
H-->>E: Resolve/Reject (unchanged flow)
end
U->>E: Click "Resend Code"
E->>E: Clear isEmptyCode (and related flags)
E-->>U: Proceed with resend flow (existing)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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 |
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
🧹 Nitpick comments (6)
components/view/access-form/email-verification-form.tsx (6)
2-4
: Use type-only imports to avoid pulling Prisma client into the browser bundle.Switch to import type for Brand and DataroomBrand (or remove entirely if you adopt the narrower prop type below).
-import { Brand, DataroomBrand } from "@prisma/client"; +import type { Brand, DataroomBrand } from "@prisma/client";
24-34
: Narrow the brand prop to the fields you actually use.You only read accentColor. Narrowing the prop reduces coupling and sidesteps union-property issues.
- brand?: Partial<Brand> | Partial<DataroomBrand> | null; + // Narrow contract: any object with optional accentColor is acceptable + brand?: { accentColor?: string | null } | null;Call sites remain valid via structural typing.
41-49
: Timer re-creates an interval every tick.Prefer a functional state update to avoid needless interval churn.
- useEffect(() => { - if (delaySeconds > 0) { - const interval = setInterval( - () => setDelaySeconds(delaySeconds - 1), - 1000, - ); - return () => clearInterval(interval); - } - }, [delaySeconds]); + useEffect(() => { + if (delaySeconds === 0) return; + const id = setInterval(() => setDelaySeconds((s) => s - 1), 1000); + return () => clearInterval(id); + }, [delaySeconds]);
81-91
: Avoid passing mismatched event types through onSubmitHandler.Resend uses onClick but casts to FormEvent to reuse onSubmitHandler. This is type-unsafe and brittle.
- Option A (preferred): Change onSubmitHandler to a zero-arg callback (e.g., onSubmit: () => Promise) and handle preventDefault locally in the form.
- Option B: Add a dedicated onResend() prop to trigger resend without fabricating a form event.
I can draft the minimal API change if you want.
156-174
: Resend state never resets; label logic can show “Resending...” for 60s.isResendLoading is set to true but never cleared; combined with the current label precedence, users may see “Resending code...” instead of the countdown after the request completes.
Apply both fixes:
- Reset isResendLoading when loading ends.
- Prioritize the countdown label over “Resending...” text.
@@ const [isResendLoading, setIsResendLoading] = useState(false); const [delaySeconds, setDelaySeconds] = useState(60); const [isEmptyCode, setIsEmptyCode] = useState(false); + + // Reset resend loading once the network call finishes + useEffect(() => { + if (!isLoading && isResendLoading) setIsResendLoading(false); + }, [isLoading, isResendLoading]); @@ - > - {isResendLoading && !isLoading - ? "Resending code..." - : delaySeconds > 0 - ? `Resend Code (${delaySeconds}s)` - : "Resend Code"} + > + {delaySeconds > 0 + ? `Resend Code (${delaySeconds}s)` + : isResendLoading && isLoading + ? "Resending code..." + : "Resend Code"}
139-156
: Minor a11y: ensure text remains legible across brand colors.Using determineTextColor for these texts is good. Consider adding aria-live="polite" for the countdown region to improve feedback for screen readers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/view/access-form/email-verification-form.tsx
(6 hunks)components/view/dataroom/dataroom-document-view.tsx
(1 hunks)components/view/dataroom/dataroom-view.tsx
(1 hunks)components/view/document-view.tsx
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/view/access-form/email-verification-form.tsx (1)
lib/utils/determine-text-color.ts (1)
determineTextColor
(24-28)
🔇 Additional comments (4)
components/view/document-view.tsx (1)
249-250
: Prop passthrough looks correct.brand is forwarded to EmailVerificationMessage consistently with the new theming API. Please confirm that both Brand and DataroomBrand expose accentColor to avoid union-property pitfalls in TS.
components/view/dataroom/dataroom-document-view.tsx (1)
291-292
: Consistent theming propagation.Passing brand to EmailVerificationMessage aligns this view with the new theming. No functional changes introduced here.
components/view/dataroom/dataroom-view.tsx (1)
234-235
: LGTM: brand forwarded for verification step.Keeps the verification UI visually consistent with the rest of the dataroom flow.
components/view/access-form/email-verification-form.tsx (1)
123-132
: Button theming inversion: double-check contrast.You’re rendering the button with background = determineTextColor(accentColor) and text = accentColor. This inverts the page theme and generally reads well, but please verify WCAG contrast for light accent colors.
Summary by CodeRabbit