feat: DR-7746 legal pages (terms, SLA, privacy, code of conduct, partner TOS)#7712
Open
AmanVarshney01 wants to merge 3 commits intomainfrom
Open
feat: DR-7746 legal pages (terms, SLA, privacy, code of conduct, partner TOS)#7712AmanVarshney01 wants to merge 3 commits intomainfrom
AmanVarshney01 wants to merge 3 commits intomainfrom
Conversation
Add static terms of service page to apps/site migrated from the old website. Uses Eclipse accordion UI primitives with expand all / print controls in a sticky sidebar layout matching the original design.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds five Next.js legal pages, five corresponding data modules containing static JSX legal content, a new client-side Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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)
apps/site/src/data/terms.tsx (1)
5-8: Consider exportingTermsSectiontype for reuse.The
TermsSectiontype is duplicated interms-accordion.tsxasSection. Exporting it from here would establish a single source of truth and ensure the accordion component stays in sync with the data structure.♻️ Proposed change
-type TermsSection = { +export type TermsSection = { title: string; content: ReactNode; };Then in
terms-accordion.tsx, import and useTermsSectioninstead of defining a localSectiontype.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/data/terms.tsx` around lines 5 - 8, Export the existing TermsSection type so it can be reused by the accordion component; change the declaration of TermsSection (type TermsSection = { title: string; content: ReactNode; };) to an exported type and update the consumer file (where a local Section type is defined in terms-accordion.tsx) to import and use TermsSection instead of its local Section definition to remove duplication and keep a single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/site/src/app/terms/_components/terms-accordion.tsx`:
- Around line 68-71: The printPage function currently sets expandAll then uses a
50ms timeout which is a race; instead update printPage to only call
setExpandAll(true) and move the window.print() call into a useEffect that
watches expandAll so printing occurs after React commits the state change;
locate printPage and the expandAll state (setExpandAll, expandAll) in
terms-accordion.tsx and implement a useEffect that triggers window.print() when
expandAll becomes true, then reset expandAll if needed.
- Around line 25-45: The toggle button (onToggle, aria-expanded={isOpen}) needs
an aria-controls that points to the content panel id and the content panel needs
a matching id so screen readers can link them; create a stable panelId (e.g.,
derived from section.id or a generated string) and set aria-controls={panelId}
on the button and id={panelId} on the content div, render the panel always (do
not remove it from the DOM) and hide it when closed using a CSS utility like
hidden or aria-hidden to preserve the relationship, and ensure you update
aria-hidden to reflect isOpen.
In `@apps/site/src/app/terms/page.tsx`:
- Line 13: Replace the hardcoded hex in the main element's className in the
Terms page (the <main> with className containing
bg-[linear-gradient(180deg,`#171937_0`%,transparent_20%)]) with a theme-aware
gradient that uses the app's CSS custom property (e.g.
var(--color-background-default)) or Tailwind dark: utilities so it adapts to
light/dark themes; update the gradient to reference
var(--color-background-default) (or a dark: variant) instead of `#171937` so the
background matches ThemeProvider-driven colors and remains legible in both
modes.
---
Nitpick comments:
In `@apps/site/src/data/terms.tsx`:
- Around line 5-8: Export the existing TermsSection type so it can be reused by
the accordion component; change the declaration of TermsSection (type
TermsSection = { title: string; content: ReactNode; };) to an exported type and
update the consumer file (where a local Section type is defined in
terms-accordion.tsx) to import and use TermsSection instead of its local Section
definition to remove duplication and keep a single source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 21307fcf-d120-4ef4-8d0d-9c47ad4c4907
📒 Files selected for processing (3)
apps/site/src/app/terms/_components/terms-accordion.tsxapps/site/src/app/terms/page.tsxapps/site/src/data/terms.tsx
Comment on lines
+25
to
+45
| <button | ||
| type="button" | ||
| className="flex w-full items-center justify-between py-3 text-left cursor-pointer" | ||
| onClick={onToggle} | ||
| aria-expanded={isOpen} | ||
| > | ||
| <span className="text-lg font-bold leading-[25px] text-foreground-neutral"> | ||
| {section.title} | ||
| </span> | ||
| <i | ||
| className={cn( | ||
| "fa-regular text-foreground-neutral-weaker text-lg", | ||
| isOpen ? "fa-chevron-up" : "fa-chevron-down", | ||
| )} | ||
| /> | ||
| </button> | ||
| {isOpen && ( | ||
| <div className="pb-4 text-foreground-neutral-weak text-left [&_p]:my-4 [&_a]:underline [&_a]:transition-colors [&_a]:duration-150 hover:[&_a]:text-foreground-neutral [&_ul]:list-revert [&_ul]:m-revert [&_ul]:p-revert [&_ol]:list-revert [&_ol]:m-revert [&_ol]:p-revert [&_li]:my-2 print:text-foreground-neutral"> | ||
| {section.content} | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
Accessibility: Add aria-controls and content panel id for screen reader support.
The button has aria-expanded (good!), but it's missing aria-controls to indicate which element it controls. Additionally, the content panel needs an id that matches. This helps screen reader users understand the relationship between the toggle and the content.
♿ Proposed fix
function AccordionItem({
section,
isOpen,
onToggle,
+ index,
}: {
section: Section;
isOpen: boolean;
onToggle: () => void;
+ index: number;
}) {
const anchorId = section.title.trim().toLowerCase().replace(/\s+/g, "-");
+ const contentId = `${anchorId}-content`;
return (
<div className="scroll-mt-16 md:scroll-mt-24 border-t border-stroke-neutral" id={anchorId}>
<button
type="button"
className="flex w-full items-center justify-between py-3 text-left cursor-pointer"
onClick={onToggle}
aria-expanded={isOpen}
+ aria-controls={contentId}
>
<span className="text-lg font-bold leading-[25px] text-foreground-neutral">
{section.title}
</span>
<i
className={cn(
"fa-regular text-foreground-neutral-weaker text-lg",
isOpen ? "fa-chevron-up" : "fa-chevron-down",
)}
/>
</button>
- {isOpen && (
- <div className="pb-4 text-foreground-neutral-weak ...">
+ <div
+ id={contentId}
+ className="pb-4 text-foreground-neutral-weak ..."
+ hidden={!isOpen}
+ >
{section.content}
- </div>
- )}
+ </div>
</div>
);
}Using hidden instead of conditional rendering keeps the element in the DOM for aria-controls to reference while still hiding it visually and from assistive tech.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/app/terms/_components/terms-accordion.tsx` around lines 25 -
45, The toggle button (onToggle, aria-expanded={isOpen}) needs an aria-controls
that points to the content panel id and the content panel needs a matching id so
screen readers can link them; create a stable panelId (e.g., derived from
section.id or a generated string) and set aria-controls={panelId} on the button
and id={panelId} on the content div, render the panel always (do not remove it
from the DOM) and hide it when closed using a CSS utility like hidden or
aria-hidden to preserve the relationship, and ensure you update aria-hidden to
reflect isOpen.
Comment on lines
+68
to
+71
| const printPage = () => { | ||
| setExpandAll(true); | ||
| setTimeout(() => window.print(), 50); | ||
| }; |
Contributor
There was a problem hiding this comment.
The 50ms timeout before printing is a race condition.
React's state update and subsequent re-render aren't guaranteed to complete within 50ms, especially on slower devices. This could result in printing with sections still collapsed.
🔧 Proposed fix using useEffect
+import { useState, useEffect } from "react";
+
export function TermsAccordion({ sections }: { sections: Section[] }) {
const [expandAll, setExpandAll] = useState(false);
const [selected, setSelected] = useState(0);
+ const [isPrinting, setIsPrinting] = useState(false);
+
+ useEffect(() => {
+ if (isPrinting && expandAll) {
+ window.print();
+ setIsPrinting(false);
+ }
+ }, [isPrinting, expandAll]);
const printPage = () => {
setExpandAll(true);
- setTimeout(() => window.print(), 50);
+ setIsPrinting(true);
};This ensures window.print() is called only after React has committed the state change where expandAll is true.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/app/terms/_components/terms-accordion.tsx` around lines 68 -
71, The printPage function currently sets expandAll then uses a 50ms timeout
which is a race; instead update printPage to only call setExpandAll(true) and
move the window.print() call into a useEffect that watches expandAll so printing
occurs after React commits the state change; locate printPage and the expandAll
state (setExpandAll, expandAll) in terms-accordion.tsx and implement a useEffect
that triggers window.print() when expandAll becomes true, then reset expandAll
if needed.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
- Extract shared accordion into @/components/legal-accordion - Fix accordion state to track individual items via Set<number> - Add fixed-height gradient for consistent look across pages - All pages start collapsed by default - /sla, /privacy, /event-code-of-conduct, /partners/tos
AmanVarshney01
changed the title
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/site/src/app/privacy/page.tsx (1)
11-35: Consider extracting a shared layout component.The page structure (hero with title/last-updated, separator, content grid) is nearly identical across
/privacy,/sla,/terms, and/event-code-of-conduct. A sharedLegalPageLayoutcomponent acceptingtitle,lastUpdated,description?, andchildrenwould reduce duplication and ensure consistency.💡 Example abstraction
// apps/site/src/components/legal-page-layout.tsx export function LegalPageLayout({ title, lastUpdated, description, children, }: { title: string; lastUpdated: string; description?: ReactNode; children: ReactNode; }) { return ( <main className="flex-1 w-full z-1 -mt-24 pt-24 relative before:absolute ..."> <div className="text-center py-16"> <h1 className="text-5xl font-bold ...">{title}</h1> {description && <p className="...">{description}</p>} <p className="..."><b>Last updated:</b> {lastUpdated}</p> </div> <div className="max-w-[1248px] mx-auto ..."> <hr className="border-stroke-neutral" /> </div> <div className="mx-auto w-full max-w-[1248px] ..."> {children} </div> </main> ); }Then pages become:
export default function PrivacyPage() { return ( <LegalPageLayout title="Privacy Policy" lastUpdated={privacyLastUpdated}> <LegalAccordion sections={privacySections} /> </LegalPageLayout> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/privacy/page.tsx` around lines 11 - 35, The PrivacyPage repeats shared layout markup used by other legal pages; extract a new reusable component (e.g., LegalPageLayout) that accepts props title, lastUpdated, optional description, and children, move the hero / separator / container markup into that component, then update PrivacyPage (replace the current main JSX) to render <LegalPageLayout title="Privacy Policy" lastUpdated={privacyLastUpdated}>{/* body */}</LegalPageLayout> and pass the existing body (LegalAccordion with privacySections) as children; ensure references to privacyLastUpdated and privacySections remain unchanged and other legal pages (SLA, Terms, EventCodeOfConduct) are refactored to use the same LegalPageLayout for consistency.apps/site/src/components/legal-accordion.tsx (1)
21-21: Consider a more robust anchor ID generation.The current approach retains characters like periods and colons (e.g.,
"1.-commitment-to-service"). While technically valid, a more thorough sanitization could improve URL fragment aesthetics and prevent potential edge cases:- const anchorId = section.title.trim().toLowerCase().replace(/\s+/g, "-"); + const anchorId = section.title + .trim() + .toLowerCase() + .replace(/[^\w\s-]/g, "") // Remove special chars + .replace(/\s+/g, "-");Alternatively, accept an explicit
idin theSectiontype for full control.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/components/legal-accordion.tsx` at line 21, The anchor generation using const anchorId = section.title.trim().toLowerCase().replace(/\s+/g, "-") is fragile; update legal-accordion to prefer an explicit section.id if provided on the Section type and otherwise generate a robust slug from section.title by: lowercasing, replacing all non-alphanumeric characters with hyphens, collapsing multiple hyphens into one, and trimming leading/trailing hyphens (e.g., use a regex to replace [^a-z0-9]+ -> "-" then .replace(/-+/g,"-") and .replace(/^-|-$/g,"")). Modify the Section type to include an optional id, change the anchorId computation in legal-accordion.tsx to use section.id ?? generatedSlug, and ensure any consumers that create Section objects can pass explicit ids when needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/site/src/app/privacy/page.tsx`:
- Around line 11-35: The PrivacyPage repeats shared layout markup used by other
legal pages; extract a new reusable component (e.g., LegalPageLayout) that
accepts props title, lastUpdated, optional description, and children, move the
hero / separator / container markup into that component, then update PrivacyPage
(replace the current main JSX) to render <LegalPageLayout title="Privacy Policy"
lastUpdated={privacyLastUpdated}>{/* body */}</LegalPageLayout> and pass the
existing body (LegalAccordion with privacySections) as children; ensure
references to privacyLastUpdated and privacySections remain unchanged and other
legal pages (SLA, Terms, EventCodeOfConduct) are refactored to use the same
LegalPageLayout for consistency.
In `@apps/site/src/components/legal-accordion.tsx`:
- Line 21: The anchor generation using const anchorId =
section.title.trim().toLowerCase().replace(/\s+/g, "-") is fragile; update
legal-accordion to prefer an explicit section.id if provided on the Section type
and otherwise generate a robust slug from section.title by: lowercasing,
replacing all non-alphanumeric characters with hyphens, collapsing multiple
hyphens into one, and trimming leading/trailing hyphens (e.g., use a regex to
replace [^a-z0-9]+ -> "-" then .replace(/-+/g,"-") and .replace(/^-|-$/g,"")).
Modify the Section type to include an optional id, change the anchorId
computation in legal-accordion.tsx to use section.id ?? generatedSlug, and
ensure any consumers that create Section objects can pass explicit ids when
needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ea6e71d-085f-4925-921e-d72a0deeaabf
📒 Files selected for processing (10)
apps/site/src/app/event-code-of-conduct/page.tsxapps/site/src/app/partners/tos/page.tsxapps/site/src/app/privacy/page.tsxapps/site/src/app/sla/page.tsxapps/site/src/app/terms/page.tsxapps/site/src/components/legal-accordion.tsxapps/site/src/data/event-code-of-conduct.tsxapps/site/src/data/partners-tos.tsxapps/site/src/data/privacy.tsxapps/site/src/data/sla.tsx
✅ Files skipped from review due to trivial changes (4)
- apps/site/src/app/partners/tos/page.tsx
- apps/site/src/data/event-code-of-conduct.tsx
- apps/site/src/data/partners-tos.tsx
- apps/site/src/data/privacy.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/site/src/app/terms/page.tsx
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
/terms,/sla,/privacy,/event-code-of-conduct, and/partners/tospages toapps/siteLegalAccordioncomponent in@/components/legal-accordion.tsxwith expand all / collapse all / print controlsSummary by CodeRabbit