-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Apply dataroom accent color to otp screen #1902
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
Apply dataroom accent color to otp screen #1902
Conversation
Co-authored-by: marcftone <[email protected]>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces accent color theming for OTP inputs and the email verification flow. Adds an optional accentColor to InputOTP via a new context for dynamic styling. Extends EmailVerificationMessage to accept a brand prop and apply computed colors. Parent views pass brand to EmailVerificationMessage. Changes
Pre-merge checks❌ Failed checks (1 warning)
✅ 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 |
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: 1
🧹 Nitpick comments (4)
components/ui/input-otp.tsx (1)
59-67: Focus ring not themed — can be invisible on some brand backgroundsUse the computed
textColorfor the ring to maintain visibility.Apply:
- "relative flex h-10 w-10 items-center justify-center border-y border-r border-input text-sm transition-all first:rounded-l-md first:border-l last:rounded-r-md", - isActive && "z-10 ring-2 ring-ring", + "relative flex h-10 w-10 items-center justify-center border-y border-r border-input text-sm transition-all first:rounded-l-md first:border-l last:rounded-r-md", + isActive && + (textColor === "white" + ? "z-10 ring-2 ring-white" + : "z-10 ring-2 ring-black"),components/view/access-form/email-verification-form.tsx (3)
54-61: Background color fallback whenaccentColoris invalidIf
brand.accentColoris a truthy but invalid CSS color, the background will not apply. Add a simple hex guard.Example:
- style={{ - backgroundColor: - brand && brand.accentColor ? brand.accentColor : "black", - }} + style={{ + backgroundColor: + brand?.accentColor?.match(/^#(?:[0-9a-fA-F]{3}){1,2}$/) + ? (brand!.accentColor as string) + : "black", + }}Optionally extract a small
isHexColorutil for reuse.
62-75: Avoid repeateddetermineTextColorcallsCompute once and reuse for headings, helper text, button, and links.
Apply:
- return ( + const textColor = determineTextColor(brand?.accentColor); + return ( ... - <h2 - className="mt-10 text-2xl font-bold leading-9 tracking-tight" - style={{ - color: determineTextColor(brand?.accentColor), - }} - > + <h2 + className="mt-10 text-2xl font-bold leading-9 tracking-tight" + style={{ color: textColor }} + > ... - <p - className="text-pretty text-sm leading-6" - style={{ - color: determineTextColor(brand?.accentColor), - }} - > + <p + className="text-pretty text-sm leading-6" + style={{ color: textColor }} + >Also update below sections to use
textColor(Button and footer texts). If desired, memoize:const textColor = useMemo(() => determineTextColor(brand?.accentColor), [brand?.accentColor]).
123-131: Footer text/link grayscale choice may under-contrast on dark accentsHardcoded grays (
rgb(75,85,99)/rgb(107,114,128)/rgb(156,163,175)) can be low‑contrast against some dark brand backgrounds. Prefer deriving fromtextColor(e.g.,opacityorcolor-mix) to guarantee contrast.Example:
- style={{ color: determineTextColor(brand?.accentColor) === "white" ? "rgb(75, 85, 99)" : "rgb(107, 114, 128)" }} + style={{ color: textColor, opacity: 0.75 }}And for the link:
- style={{ color: determineTextColor(brand?.accentColor) === "white" ? "rgb(107, 114, 128)" : "rgb(156, 163, 175)" }} + style={{ color: textColor, opacity: 0.6 }}Also applies to: 137-143
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/ui/input-otp.tsx(2 hunks)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 (2)
components/ui/input-otp.tsx (2)
lib/utils.ts (1)
cn(18-20)lib/utils/determine-text-color.ts (1)
determineTextColor(24-28)
components/view/access-form/email-verification-form.tsx (1)
lib/utils/determine-text-color.ts (1)
determineTextColor(24-28)
🔇 Additional comments (4)
components/ui/input-otp.tsx (2)
16-33: Prop + context wiring looks goodThe new accentColor prop and provider wrapping OTPInput are clean and non-invasive.
46-49: All<InputOTPSlot>usages include the requiredindexprop
Verified no instances missingindex.components/view/access-form/email-verification-form.tsx (2)
91-93: Passing accentColor to OTP — good integrationPropagating
brand?.accentColordown toInputOTPaligns the OTP theming with the page.
111-115: Button contrast on all brand backgroundsCurrent choice uses
backgroundColor = textColorandcolor = accentColor. This usually works, but some dark accents may reduce text contrast on black. Consider clamping the accent’s lightness for button text, or swap tobackgroundColor: accentColorandcolor: textColorwhen the page background isn’t accent-colored.Please visually verify contrast (WCAG AA) with a few dark/light brand colors. If needed, I can provide a small
ensureContrast(fg, bg, ratio)helper.
| const accentColor = React.useContext(AccentColorContext); | ||
| const textColor = determineTextColor(accentColor); | ||
|
|
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.
Guard against invalid hex input
If accentColor is a malformed hex, determineTextColor may throw via hexToRgb. Add a safe fallback.
Apply:
- const accentColor = React.useContext(AccentColorContext);
- const textColor = determineTextColor(accentColor);
+ const accentColor = React.useContext(AccentColorContext);
+ const textColor = React.useMemo(() => {
+ try {
+ return determineTextColor(accentColor);
+ } catch {
+ return "white";
+ }
+ }, [accentColor]);📝 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.
| const accentColor = React.useContext(AccentColorContext); | |
| const textColor = determineTextColor(accentColor); | |
| const accentColor = React.useContext(AccentColorContext); | |
| const textColor = React.useMemo(() => { | |
| try { | |
| return determineTextColor(accentColor); | |
| } catch { | |
| return "white"; | |
| } | |
| }, [accentColor]); |
🤖 Prompt for AI Agents
In components/ui/input-otp.tsx around lines 52 to 54, the call to
determineTextColor(accentColor) can throw if accentColor is a malformed hex;
validate/accommodate bad input by checking that accentColor is a valid hex (or
wrap determineTextColor in try/catch) and fall back to a safe default accent
(e.g. a known hex like "#ffffff" or "#000000") or default text color when
invalid; update the code so accentColor is sanitized/validated before passing to
determineTextColor and ensure a deterministic fallback textColor is assigned on
failure.
Apply dataroom accent color to the OTP passcode screen for visual consistency with branding.
Slack Thread
Summary by CodeRabbit
New Features
Style