feat: add dedicated FAQ page#80
Conversation
🚀 PR Received SuccessfullyHello @saurabhhhcodes, Thank you for taking the initiative to contribute to this project. Please ensure that your PR follows all project guidelines properly before requesting review.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a centralized ChangesFAQ Page Feature
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant FAQPage
participant FAQIndex
participant Accordion
Client->>Router: navigate to /faq
Router->>FAQPage: render FAQPage
FAQPage->>FAQIndex: build index from faqs
FAQPage->>Accordion: render accordion items from faqs
FAQIndex->>FAQPage: toggleFaq(index, scroll=true)
FAQPage->>Accordion: open item (openIdx change) and requestAnimationFrame scroll
Accordion->>Client: reveal answer panel (item.a)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 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)
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 review this entire PR thoroughly and point out every area that can be improved, optimized, or made more logically consistent so the overall implementation becomes cleaner, stronger, and production-ready for the project. |
🧩 Analysis chain🏁 Script executed: Length of output: 24720
1.
|
| Issue | Location | Fix |
|---|---|---|
+ / - <span> not hidden from screen readers |
Accordion toggle button | Add aria-hidden="true" to the span |
No aria-controls on accordion buttons |
Each <button> |
Add aria-controls={item.id} and id={item.id} on the panel <div> |
No id on answer panel |
<div> after button |
Add id={item.id} |
Corrected button:
<button
type="button"
onClick={() => setOpenIdx(isOpen ? null : index)}
className="..."
aria-expanded={isOpen}
aria-controls={`faq-panel-${item.id}`}
>
<span className="...">{item.q}</span>
<span className="text-4xl font-black leading-none" aria-hidden="true">
{isOpen ? "−" : "+"}
</span>
</button>
<div id={`faq-panel-${item.id}`} ...>E. key={item.q} is fragile
Use the stable id proposed above: key={item.id}.
F. supportTopics array is hardcoded and disconnected from data
const supportTopics = [
"Platform tracking",
"Data privacy",
"Open source",
"Account setup",
];These four labels don't map to any FAQ entry. Either derive them from a category field on the FAQ objects, or remove the chips to avoid implying filterable categories that don't work.
G. "Back To Explore" CTA always hardcodes /explore
If a user arrives at /faq from the Footer or any other path, clicking the CTA always sends them to /explore regardless of history. A simple improvement:
import { useNavigate } from "react-router-dom";
const navigate = useNavigate();
// ...
<button onClick={() => navigate(-1)}>Go Back</button>Or at minimum label it "Explore the Platform" to set correct expectations.
H. No document <title> for SEO
The page has no <title> update. Use the React Router <title> element (v7) or a <Helmet> equivalent:
// React Router v7
<title>FAQ – CodeLens</title>3. src/components/explore/FAQSection.jsx
A. type="button" missing on accordion toggle
// ❌ current
<button onClick={() => toggle(index)} className="...">
// ✅ fix
<button type="button" onClick={() => toggle(index)} className="...">Without type="button", if this component is ever nested in a <form>, clicking will submit the form.
B. key={index} is an anti-pattern
// ❌ current
<div key={index} ...>
// ✅ fix — once id is added to faqs.js
<div key={item.id} ...>C. maxHeight: "500px" will clip long answers
style={{ maxHeight: isOpen ? "500px" : "0px" }}If any answer exceeds ~500px of rendered height (especially on mobile), it will be silently truncated with no scroll or indicator. Use CSS Grid animation instead:
<div
className="overflow-hidden transition-all duration-300 ease-in-out"
style={{ maxHeight: isOpen ? "1000px" : "0px" }} // or use a ref-measured value
>Or better, use a CSS Grid approach:
<div
className={`grid transition-all duration-300 ease-in-out ${isOpen ? "grid-rows-[1fr]" : "grid-rows-[0fr]"}`}
>
<div className="overflow-hidden">
<p className="...">{item.a}</p>
</div>
</div>D. "border-3" is not a valid Tailwind CSS class
className="border-3 border-black ..." // ❌ — no such class in default Tailwind configTailwind's border-width scale goes border (1px), border-2 (2px), border-4 (4px). border-3 silently produces no border. Change to border-2 or border-4.
4. src/App.jsx
A. Missing blank line before FAQPage import
Minor but inconsistent with the rest of the file's import style — the FAQPage import is placed directly before export default function App() without a blank line. Group it with the other page imports.
// ❌ current (at bottom, right before export)
import FAQPage from "./pages/FAQPage";
export default function App() {
// ✅ fix — move into the page imports block above
import TermsPage from "./pages/TermsPage";
import PrivacyPage from "./pages/PrivacyPage";
import FAQPage from "./pages/FAQPage"; // ← here
import NotFoundPage from "./pages/NotFoundPage";5. src/components/shared/Footer.jsx
A. /codeforces is a protected route in the public Platform list
{ to: "/dashboard", label: "Dashboard" },
{ to: "/explore", label: "Explore" },
{ to: "/faq", label: "FAQ" }, // ✅ public — correct
{ to: "/codeforces", label: "Codeforces" }, // ⚠️ protected routeUnauthenticated users clicking "Codeforces" from the footer will be redirected to login, with no explanation. Either remove it from the footer, add an auth-aware wrapper, or add a tooltip/badge (e.g., "Login required"). This isn't introduced by this PR but is exposed now that you're auditing footer links.
Summary of Issues by Priority
| Priority | File | Issue |
|---|---|---|
| 🔴 High | FAQPage.jsx |
Sidebar & accordion state logic inconsistency |
| 🔴 High | FAQSection.jsx |
border-3 is not a valid Tailwind class — no border rendered |
| 🔴 High | FAQSection.jsx |
maxHeight: 500px clips long answers |
| 🟠 Medium | FAQPage.jsx |
No scroll-to-item on sidebar click |
| 🟠 Medium | FAQPage.jsx |
supportTopics chips disconnected from FAQ data |
| 🟠 Medium | FAQPage.jsx |
Missing aria-controls / aria-hidden on toggle |
| 🟠 Medium | faqs.js |
No id field — fragile key usage in both components |
| 🟠 Medium | FAQSection.jsx |
Missing type="button" on accordion toggle |
| 🟡 Low | FAQPage.jsx |
openIdx initialized to 0 instead of null |
| 🟡 Low | FAQPage.jsx |
"Back To Explore" ignores navigation history |
| 🟡 Low | FAQPage.jsx |
No page <title> for SEO |
| 🟡 Low | App.jsx |
FAQPage import placement out of group |
✅ Actions performed
Full review triggered.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/src/pages/FAQPage.jsx (1)
80-99: ⚡ Quick winBind each toggle button to its panel with ARIA ids.
Add
aria-controlson the button, andid+aria-labelledbyon the panel; mark the symbol as decorative.♿ Proposed fix
{faqs.map((item, index) => { const isOpen = openIdx === index; + const panelId = `faq-panel-${index}`; + const buttonId = `faq-button-${index}`; return ( <article key={item.q} className="border-4 border-black bg-white shadow-[8px_8px_0_0_rgba(0,0,0,1)]" > <button type="button" onClick={() => setOpenIdx(isOpen ? null : index)} className="flex w-full items-center justify-between gap-6 p-6 text-left sm:p-8" aria-expanded={isOpen} + aria-controls={panelId} + id={buttonId} > <span className="text-2xl font-black uppercase leading-tight tracking-tight sm:text-4xl"> {item.q} </span> - <span className="text-4xl font-black leading-none"> + <span className="text-4xl font-black leading-none" aria-hidden="true"> {isOpen ? "-" : "+"} </span> </button> {isOpen && ( - <div className="border-t-4 border-black px-6 py-6 sm:px-8"> + <div + id={panelId} + aria-labelledby={buttonId} + className="border-t-4 border-black px-6 py-6 sm:px-8" + > <p className="max-w-3xl text-base font-bold leading-relaxed text-black sm:text-lg"> {item.a} </p> </div> )}🤖 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 `@frontend/src/pages/FAQPage.jsx` around lines 80 - 99, The button and panel need ARIA linking: give the toggle button a unique id (e.g., `faq-button-${index}`) and add aria-controls pointing to the panel id; give the panel a matching id (e.g., `faq-panel-${index}`) and aria-labelledby referencing the button id; mark the decorative +/- span as aria-hidden="true". Update the JSX around the toggle (where setOpenIdx, isOpen, index, item.q/item.a are used) to generate these ids so each button-panel pair is uniquely bound.frontend/src/data/faqs.js (1)
1-18: ⚡ Quick winAdd a stable
idfield to each FAQ item.Using mutable text (
q) as identity is actively causing issues across two components.FAQPage.jsxrenders FAQs withkey={item.q}at two locations (lines 56 and 77), andFAQSection.jsxuseskey={index}at line 23. Both patterns are fragile: if question text is edited or the list is reordered, React loses component identity and state association. Add a stableidto each FAQ and consume it downstream.♻️ Proposed change
export const faqs = [ { + id: "what-is-codelens", q: "What is CodeLens?", a: "CodeLens is a platform that helps developers track their coding progress across different platforms like Codeforces, LeetCode, and GitHub in one place.", }, { + id: "problem-codelens-solves", q: "What problem does CodeLens solve?", a: "It brings all your coding activity into one place so you can understand your progress, stay consistent, and know what to improve next.", }, { + id: "repository-data-privacy", q: "Is my repository data private?", a: "CodeLens only uses publicly available data from connected platforms and does not store any sensitive information.", }, { + id: "how-to-contribute", q: "How can I contribute to CodeLens?", a: "You can contribute by solving issues, adding features, improving UI, or fixing bugs. Check the repository and start with beginner-friendly issues.", }, ];🤖 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 `@frontend/src/data/faqs.js` around lines 1 - 18, The faqs array items lack stable IDs which breaks React identity in FAQPage.jsx (uses key={item.q}) and FAQSection.jsx (uses key={index}); add a unique, stable id property to each object in the exported faqs array (e.g., "id": "faq-1", etc.) and update the consuming components to use key={item.id} instead of key={item.q} or key={index} so component identity is preserved when text changes or items reorder.
🤖 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 `@frontend/src/pages/FAQPage.jsx`:
- Around line 55-59: The button's onClick currently always sets open via
setOpenIdx(index) while the accordion header uses toggle logic; change the
button's handler to the same toggle pattern used by the accordion header by
calling setOpenIdx(prev => (prev === index ? null : index)) so it closes when
already open and opens otherwise; locate the onClick on the button that
references setOpenIdx and update it to use the toggle function, keeping openIdx
and index as the comparison keys.
---
Nitpick comments:
In `@frontend/src/data/faqs.js`:
- Around line 1-18: The faqs array items lack stable IDs which breaks React
identity in FAQPage.jsx (uses key={item.q}) and FAQSection.jsx (uses
key={index}); add a unique, stable id property to each object in the exported
faqs array (e.g., "id": "faq-1", etc.) and update the consuming components to
use key={item.id} instead of key={item.q} or key={index} so component identity
is preserved when text changes or items reorder.
In `@frontend/src/pages/FAQPage.jsx`:
- Around line 80-99: The button and panel need ARIA linking: give the toggle
button a unique id (e.g., `faq-button-${index}`) and add aria-controls pointing
to the panel id; give the panel a matching id (e.g., `faq-panel-${index}`) and
aria-labelledby referencing the button id; mark the decorative +/- span as
aria-hidden="true". Update the JSX around the toggle (where setOpenIdx, isOpen,
index, item.q/item.a are used) to generate these ids so each button-panel pair
is uniquely bound.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c9c87f3-447a-453f-a027-cb1b819b5faa
📒 Files selected for processing (5)
frontend/src/App.jsxfrontend/src/components/explore/FAQSection.jsxfrontend/src/components/shared/Footer.jsxfrontend/src/data/faqs.jsfrontend/src/pages/FAQPage.jsx
|
Addressed the CodeRabbit review feedback in commit |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/src/components/explore/FAQSection.jsx`:
- Around line 48-55: The FAQ panel div with id `landing-faq-panel-${item.id}` is
kept mounted for animation but lacks hidden semantics; update that div to
include aria-hidden={isOpen ? "false" : "true"} (or aria-hidden={!isOpen}) and
add accessible region attributes such as role="region" and aria-labelledby
pointing to the question toggle (e.g., `landing-faq-button-${item.id}`), and
ensure the toggle button that controls `isOpen` has the matching id so screen
readers know which heading labels the region.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5551b5b4-74d0-4685-bb30-0029b2c16390
📒 Files selected for processing (4)
frontend/src/App.jsxfrontend/src/components/explore/FAQSection.jsxfrontend/src/data/faqs.jsfrontend/src/pages/FAQPage.jsx
|
Pushed Updated the landing FAQ accordion so each animated panel is now explicitly bound to its toggle:
Validation:
|
|
@coderabbitai please check whether @saurabhhhcodes resolved every level issues you mentioned before or something else |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 26164
Verification Report✅ All High-Priority Issues — Fixed
✅ All Medium-Priority Issues — Fixed
✅ /
|
| Issue | Status |
|---|---|
🟡 openIdx initialized to 0 |
✅ Changed to null |
🟡 No <title> for SEO |
✅ <title>FAQ - CodeLens</title> present |
🟡 FAQPage import out of group |
✅ Now correctly placed before NotFoundPage |
🟡 "Back To Explore" hardcodes /explore |
to="/explore" — not fixed, but low impact |
🆕 New Issue Found — Medium Priority
FAQPage.jsx: aria-controls points to a conditionally-rendered panel
The answer panel in FAQPage is rendered only when isOpen is true:
{isOpen && (
<div id={`faq-panel-${item.id}`} ...> // ← Only exists in DOM when open
...
</div>
)}But the toggle button always has:
aria-controls={`faq-panel-${item.id}`}When the panel is collapsed, the id it references doesn't exist in the DOM, so screen readers cannot find or announce the controlled region. FAQSection.jsx already handles this correctly by always rendering the panel and using grid-rows-[0fr] to collapse it visually.
The fix is to adopt the same pattern used in FAQSection.jsx:
<article key={item.id} ref={(el) => { itemRefs.current[index] = el; }} className="...">
<button
type="button"
onClick={() => toggleFaq(index)}
aria-expanded={isOpen}
aria-controls={`faq-panel-${item.id}`}
className="..."
>
<span className="...">{item.q}</span>
<span aria-hidden="true">{isOpen ? "−" : "+"}</span>
</button>
{/* Always in DOM — collapsed via CSS grid, not conditional rendering */}
<div
id={`faq-panel-${item.id}`}
role="region"
aria-labelledby={`faq-btn-${item.id}`}
aria-hidden={!isOpen}
className={`grid transition-all duration-300 ease-in-out border-t-4 border-black ${
isOpen ? "grid-rows-[1fr]" : "grid-rows-[0fr]"
}`}
>
<div className="overflow-hidden">
<p className="px-6 py-6 sm:px-8 max-w-3xl text-base font-bold leading-relaxed text-black sm:text-lg">
{item.a}
</p>
</div>
</div>
</article>Summary for Merge Decision
The PR is in good shape overall — all high and medium issues from the original review are resolved. Before merging, it's worth asking @saurabhhhcodes to fix the aria-controls / conditional rendering issue above (medium severity, screen-reader regression introduced in the latest accessibility commit). The "Back To Explore" link is low priority and can be deferred if needed.
|
@coderabbitai what do you mean by hardcoding /explore |
|
Tip For best results, initiate chat on the files or code changes.
"Hardcoding // ❌ Hardcoded — always goes to /explore, even if user came from /dashboard or /faq via Footer
<Link to="/explore">Back To Explore</Link>The problem: If a user clicks "FAQ" from the Footer (while on The better alternative — import { useNavigate } from "react-router-dom";
const navigate = useNavigate();
// ✅ Goes back to wherever the user came from
<button type="button" onClick={() => navigate(-1)}>
← Go Back
</button>This uses the browser's history stack, so the user always returns to the page they were on before — whether that was If you want to keep it simple and always go to ʕ •ᴥ•ʔ |
kunalverma2512
added
enhancement
|
@saurabhhhcodes implement the better alternative that coderabbitai mentioned put neccessary screenshot of final result and also put a video also showcasing result (via drive link) |
|
Implemented the requested follow-up in What changed:
Validation:
The PR should be ready for your review again. |
|
Pushed one more small polish commit: What changed:
Validation run after this commit:
CodeRabbit was already green on the prior head, and this latest change is scoped to responsive containment only. The PR should be ready for maintainer review again. |
|
Quick follow-up: this PR is still clean and mergeable from my side after the last polish commit. Since the PR already carries the GSSoC labels and mentor tag, please merge it when you get a chance if there is no further feedback. |
|
@saurabhhhcodes attach screen shot plus working site video for that page and also show browser console while opring faq page |
|
Hi! Checking back on this PR since the branch is still cleanly mergeable on GitHub. If there are any requested adjustments, I can turn them around quickly. |
|
Please perform one final deep and complete review of this PR from a production-quality perspective before merge. Do a holistic review of the entire implementation and verify whether this contribution is genuinely clean, professional, scalable, and well integrated with the existing CodeLens codebase. Please specifically review:
Also verify:
Please review the actual final state carefully instead of only checking incremental diffs so we can confidently decide whether this PR should be merged. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 30148
✅ Issues from All Previous Reviews — Resolution Status
🔍 Final Deep Review — Remaining Findings🟠 Medium —
|
| Area | Verdict |
|---|---|
| Architecture / file layout | ✅ Clean — data in /data, page in /pages, component in /components/explore |
| Route registration | ✅ Public /faq route, correctly ordered before * wildcard |
| Data layer reuse | ✅ Single faqs.js source, consumed by both FAQPage and FAQSection — no duplication |
| Accordion state logic | ✅ Correct toggle, collapsed by default, functional setState update pattern |
| Stable React keys | ✅ item.id used everywhere, no index keys |
| Accessibility (ARIA) | ✅ aria-expanded, aria-controls, aria-labelledby, aria-hidden, role="region" — all correct and consistent |
aria-controls DOM integrity |
✅ Panels always mounted, collapsed visually only |
| ID namespacing | ✅ FAQSection uses landing-faq-* prefix to avoid collision with FAQPage's faq-* IDs |
| Animations | ✅ CSS grid transition — smooth, no JS height measurement needed |
| Mobile responsiveness | ✅ break-words on sidebar labels, stacked chips on mobile, sticky sidebar on desktop |
<title> SEO |
✅ Valid React 19 metadata element, hoisted to <head> |
navigate(-1) |
✅ Correct behavior — but needs fallback (see Medium issue above) |
| Import grouping | ✅ Correct |
No console.log |
✅ Clean |
border-3 |
✅ Replaced with border-2 |
type="button" |
✅ Present on all toggle buttons |
Footer /faq link |
✅ Added and routes correctly |
| React 19 + RR v7 compatibility | ✅ No deprecated APIs used |
Merge Verdict
This PR is mergeable. All high and medium issues from the original review have been cleanly resolved. The two remaining medium items (navigate(-1) fallback and the CTA copy mismatch) are UX polish items — the navigate(-1) fallback in particular is worth a quick fix before merge since a user arriving via a direct link will get a broken "Go Back" button. The low-priority items can be addressed in a follow-up.
Recommended before merge:
- 🟠 Add the
navigate(-1)fallback (2-line fix) - 🟠 Fix the CTA section heading to match the "Go Back" button label
Can be deferred to a follow-up PR:
3. 🟡 Replace width: 100vw with w-full
4. 🟡 Double-rAF scroll timing
5. 🟡 4th FAQ category / more entries
|
Got it — I’m working on the proof bundle you asked for for the FAQ page flow, including the screenshot/browser-console check, and I’ll add that as a follow-up on this PR. |
|
Pushed one more polish commit on this PR just now. It addresses the latest review points by adding a safe fallback for the |
|
Quick follow-up here — I already pushed the FAQ polish pass requested in review. The main remaining ask on my side is visual proof once the local frontend runner cooperates again. |
2 participants
Summary
Adds a dedicated FAQ page for CodeLens and wires the existing FAQ CTA/footer navigation to the new route.
Fixes #31
Changes
/faqroute and a full-page FAQ experiencesrc/data/faqs.jsValidation
Local checks run:
Notes:
npm run buildis blocked in this macOS environment by the Rolldown native binding code-signature error after a fresh install; the failure occurs before app source bundling.Summary by CodeRabbit
New Features
Refactor
Accessibility & UX
Style
Suggested GSSoC labels if accepted:
gssoc:approved,level:beginner,quality:clean,type:feature,type:design.