Enhance Terms of Service Page - Better Navigation & Readability#339
Enhance Terms of Service Page - Better Navigation & Readability#339ArshiBansal wants to merge 3 commits into
Conversation
|
@ArshiBansal is attempting to deploy a commit to the vishnukothakapu's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Terms of Service page is converted from a static server component to a client component. A reading progress bar, ChangesInteractive Terms of Service Page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/terms/page.tsx (1)
85-86: ⚡ Quick winUse a passive scroll listener for smoother scrolling.
Line 85 attaches a high-frequency scroll handler without
{ passive: true }, which can hurt scrolling performance on slower devices.Suggested fix
- window.addEventListener("scroll", handleScroll); - return () => window.removeEventListener("scroll", handleScroll); + window.addEventListener("scroll", handleScroll, { passive: true }); + return () => window.removeEventListener("scroll", handleScroll);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/terms/page.tsx` around lines 85 - 86, The scroll event listener attached with addEventListener for the handleScroll function is missing the passive option, which can degrade scrolling performance on slower devices. Modify both the addEventListener call and the corresponding removeEventListener call in the cleanup function to include { passive: true } as the options object (third parameter), which tells the browser the handler won't call preventDefault and allows for smoother scrolling performance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/terms/page.tsx`:
- Around line 168-169: Replace unescaped double quote characters `"` with their
HTML entity equivalent `"` in the JSX text content to resolve the
react/no-unescaped-entities lint errors. In app/terms/page.tsx at lines 168-169
(the location in the Welcome to LinkID text), escape the quote characters in the
JSX text. Also apply the same fix at line 391 where unescaped quotes appear in
JSX text content. Ensure all quote characters within JSX text nodes are properly
escaped using HTML entities.
- Around line 78-82: The progress calculation at lines 78-82 does not guard
against the case where totalHeight is zero or negative, which causes the
progressPercentage calculation to produce NaN or Infinity values. Add a guard
condition to check that totalHeight is greater than 0 before calculating
progressPercentage. If totalHeight is not positive (indicating the page is not
scrollable), set progress to 0 directly to avoid passing invalid values to
setProgress.
- Line 2: The file marks itself as a Client Component with the "use client"
directive at the top, but also exports metadata at lines 11-15, which is a
server-only export in the App Router. Remove the "use client" directive to
convert this file back to a Server Component so that the metadata export becomes
valid. If client-side features are required in this page, create a separate
client component for the interactive parts and import it into this server
component, keeping the metadata export in the server component.
---
Nitpick comments:
In `@app/terms/page.tsx`:
- Around line 85-86: The scroll event listener attached with addEventListener
for the handleScroll function is missing the passive option, which can degrade
scrolling performance on slower devices. Modify both the addEventListener call
and the corresponding removeEventListener call in the cleanup function to
include { passive: true } as the options object (third parameter), which tells
the browser the handler won't call preventDefault and allows for smoother
scrolling performance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Hi @ArshiBansal , The build is currently failing because In Next.js App Router,
Please fix the build issue and push the changes. Thanks! |
|
@vishnukothakapu @Anushreebasics looking into the issues |
|
@vishnukothakapu my code doesnot use "Use client" directive. already follows server component |
Description
Improved the Terms of Service page (
app/terms/page.tsx) by adding modern documentation features for better user experience.Key Improvements
Technical Changes
'use client'for interactive scroll featuresChecklist
Closes #333
Summary by CodeRabbit