Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughUpdated login page layout with increased left column width to 55%, redesigned right panel featuring security/compliance icons (SOC2, GDPR, HIPAA, CCPA), and updated testimonial content. Refactored LogoCloud component from static SVGs to animated image-based progressive reveal with staggered timing effects. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
components/shared/logo-cloud.tsx (1)
42-54: Prefer CSS delays over timer-driven state for this reveal.This mount path schedules six timers and six React state updates, and every tile stays hidden until the effect starts. A CSS-based stagger would avoid the extra render churn, keep the content visible immediately, and make a reduced-motion fallback easier.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/shared/logo-cloud.tsx` around lines 42 - 54, The current useEffect/timers + visibleCount state should be removed and replaced with a CSS-based stagger: stop using visibleCount/setVisibleCount and the timers array in useEffect; instead render all logos visible immediately and assign each tile a style or data attribute containing its index (e.g., map logos and render <div className="logo-tile" style={{ '--delay': `${i * 150}ms` }} ...>), then implement the reveal purely in CSS using transition/animation with animation-delay: var(--delay) and a prefers-reduced-motion media query to disable animations; ensure no JS timers remain and the component still mounts with content visible while the CSS handles the staggered reveal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(auth)/login/page-client.tsx:
- Around line 181-184: The primary email CTA uses a className built via cn that
includes "disabled:opacity-100", which makes disabled buttons look active;
update the class list in the button's className (where cn(...) is used in
page-client.tsx) to show a visible disabled state—replace "disabled:opacity-100"
with a lower opacity like "disabled:opacity-50" and add
"disabled:cursor-not-allowed" (and optionally "disabled:pointer-events-none") so
the button appears and behaves disabled when the email is invalid or another
auth flow is running.
- Around line 63-69: The external Link around the Papermark logo uses
target="_blank" but lacks rel="noopener noreferrer"; update the Link element in
app/(auth)/login/page-client.tsx (the Link wrapping the <img> with
src="/_static/papermark-logo.svg") to include rel="noopener noreferrer" so the
opener is disabled for that external link while keeping target="_blank".
In `@components/shared/logo-cloud.tsx`:
- Line 59: Replace the explicit single row class on the logo grid with an auto
row track so all tiles align: in the div whose className currently contains
"grid grid-cols-2 grid-rows-[50px] gap-x-1 gap-y-3 md:grid-cols-3" (in
components/shared/logo-cloud.tsx) change "grid-rows-[50px]" to
"auto-rows-[50px]" so every implicit row uses a 50px height and the six logos
line up consistently.
---
Nitpick comments:
In `@components/shared/logo-cloud.tsx`:
- Around line 42-54: The current useEffect/timers + visibleCount state should be
removed and replaced with a CSS-based stagger: stop using
visibleCount/setVisibleCount and the timers array in useEffect; instead render
all logos visible immediately and assign each tile a style or data attribute
containing its index (e.g., map logos and render <div className="logo-tile"
style={{ '--delay': `${i * 150}ms` }} ...>), then implement the reveal purely in
CSS using transition/animation with animation-delay: var(--delay) and a
prefers-reduced-motion media query to disable animations; ensure no JS timers
remain and the component still mounts with content visible while the CSS handles
the staggered reveal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f1b83348-ffc9-4bfd-8c53-4a529985d684
⛔ Files ignored due to path filters (9)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/_static/logos/brevo.svgis excluded by!**/*.svgpublic/_static/logos/dp-world.svgis excluded by!**/*.svgpublic/_static/logos/mistral-logo-color-black.svgis excluded by!**/*.svgpublic/_static/security-icons/CCPA.svgis excluded by!**/*.svgpublic/_static/security-icons/GDPR.svgis excluded by!**/*.svgpublic/_static/security-icons/HIPAA.svgis excluded by!**/*.svgpublic/_static/security-icons/soc2-dark-new3.pngis excluded by!**/*.pngpublic/_static/security-icons/soc2new3.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
app/(auth)/login/page-client.tsxcomponents/shared/logo-cloud.tsx
Comment on lines
+63
to
69
| <div className="items-left flex flex-col space-y-3 px-4 py-6 pt-5 sm:px-12 sm:pt-6"> | ||
| <Link href="https://www.papermark.com" target="_blank"> | ||
| <img | ||
| src="/_static/papermark-logo.svg" | ||
| alt="Papermark Logo" | ||
| className="md:mb-48s -mt-8 mb-36 h-7 w-auto self-start sm:mb-32" | ||
| className="mb-24 h-7 w-auto self-start sm:mb-20" | ||
| /> |
Contributor
There was a problem hiding this comment.
Harden the external logo link the same way as the legal links.
This target="_blank" navigation is still missing rel="noopener noreferrer", so it is the one opener-enabled external link left on the page.
Suggested fix
- <Link href="https://www.papermark.com" target="_blank">
+ <Link
+ href="https://www.papermark.com"
+ target="_blank"
+ rel="noopener noreferrer"
+ >📝 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.
Suggested change
| <div className="items-left flex flex-col space-y-3 px-4 py-6 pt-5 sm:px-12 sm:pt-6"> | |
| <Link href="https://www.papermark.com" target="_blank"> | |
| <img | |
| src="/_static/papermark-logo.svg" | |
| alt="Papermark Logo" | |
| className="md:mb-48s -mt-8 mb-36 h-7 w-auto self-start sm:mb-32" | |
| className="mb-24 h-7 w-auto self-start sm:mb-20" | |
| /> | |
| <div className="items-left flex flex-col space-y-3 px-4 py-6 pt-5 sm:px-12 sm:pt-6"> | |
| <Link | |
| href="https://www.papermark.com" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| > | |
| <img | |
| src="/_static/papermark-logo.svg" | |
| alt="Papermark Logo" | |
| className="mb-24 h-7 w-auto self-start sm:mb-20" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(auth)/login/page-client.tsx around lines 63 - 69, The external Link
around the Papermark logo uses target="_blank" but lacks rel="noopener
noreferrer"; update the Link element in app/(auth)/login/page-client.tsx (the
Link wrapping the <img> with src="/_static/papermark-logo.svg") to include
rel="noopener noreferrer" so the opener is disabled for that external link while
keeping target="_blank".
Comment on lines
181
to
184
| className={cn( | ||
| "focus:shadow-outline w-full transform rounded px-4 py-2 text-white transition-colors duration-300 ease-in-out focus:outline-none", | ||
| clickedMethod === "email" | ||
| ? "bg-black" | ||
| : "bg-gray-800 hover:bg-gray-900", | ||
| "focus:shadow-outline w-full transform rounded px-4 py-2 text-white transition-colors duration-300 ease-in-out focus:outline-none disabled:opacity-100", | ||
| "bg-black hover:bg-gray-900", | ||
| )} |
Contributor
There was a problem hiding this comment.
Keep a visible disabled state on the primary email CTA.
disabled:opacity-100 makes the button look active even when the email is invalid or another auth flow is running.
Suggested fix
- "focus:shadow-outline w-full transform rounded px-4 py-2 text-white transition-colors duration-300 ease-in-out focus:outline-none disabled:opacity-100",
+ "focus:shadow-outline w-full transform rounded px-4 py-2 text-white transition-colors duration-300 ease-in-out focus:outline-none",📝 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.
Suggested change
| className={cn( | |
| "focus:shadow-outline w-full transform rounded px-4 py-2 text-white transition-colors duration-300 ease-in-out focus:outline-none", | |
| clickedMethod === "email" | |
| ? "bg-black" | |
| : "bg-gray-800 hover:bg-gray-900", | |
| "focus:shadow-outline w-full transform rounded px-4 py-2 text-white transition-colors duration-300 ease-in-out focus:outline-none disabled:opacity-100", | |
| "bg-black hover:bg-gray-900", | |
| )} | |
| className={cn( | |
| "focus:shadow-outline w-full transform rounded px-4 py-2 text-white transition-colors duration-300 ease-in-out focus:outline-none", | |
| "bg-black hover:bg-gray-900", | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(auth)/login/page-client.tsx around lines 181 - 184, The primary email
CTA uses a className built via cn that includes "disabled:opacity-100", which
makes disabled buttons look active; update the class list in the button's
className (where cn(...) is used in page-client.tsx) to show a visible disabled
state—replace "disabled:opacity-100" with a lower opacity like
"disabled:opacity-50" and add "disabled:cursor-not-allowed" (and optionally
"disabled:pointer-events-none") so the button appears and behaves disabled when
the email is invalid or another auth flow is running.
| rel="noopener noreferrer" | ||
| className="group col-span-1 flex items-center justify-center bg-white p-8 font-bold text-[#249edc] transition-colors" | ||
| href="https://snowflake.com" | ||
| <div className="grid grid-cols-2 grid-rows-[50px] gap-x-1 gap-y-3 md:grid-cols-3"> |
Contributor
There was a problem hiding this comment.
Use auto-rows-[50px] if these tiles are meant to align.
grid-rows-[50px] creates one explicit 50px row. The remaining rows in this 6-item grid fall back to implicit auto tracks, so their height now depends on whichever logo lands there.
Suggested fix
- <div className="grid grid-cols-2 grid-rows-[50px] gap-x-1 gap-y-3 md:grid-cols-3">
+ <div className="grid auto-rows-[50px] grid-cols-2 gap-x-1 gap-y-3 md:grid-cols-3">📝 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.
Suggested change
| <div className="grid grid-cols-2 grid-rows-[50px] gap-x-1 gap-y-3 md:grid-cols-3"> | |
| <div className="grid auto-rows-[50px] grid-cols-2 gap-x-1 gap-y-3 md:grid-cols-3"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/shared/logo-cloud.tsx` at line 59, Replace the explicit single row
class on the logo grid with an auto row track so all tiles align: in the div
whose className currently contains "grid grid-cols-2 grid-rows-[50px] gap-x-1
gap-y-3 md:grid-cols-3" (in components/shared/logo-cloud.tsx) change
"grid-rows-[50px]" to "auto-rows-[50px]" so every implicit row uses a 50px
height and the six logos line up consistently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Style