Conversation
anastkalas
requested review from
adithyanmkd,
ansifmk and
naheel0
as code owners
|
@anastkalas is attempting to deploy a commit to the naheel0's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA new FAQ feature is introduced comprising a dedicated page with category filtering, reusable Accordion and FaqItem components for displaying collapsible FAQ items, a TypeScript data structure containing FAQ definitions with metadata, and app routing configuration exposing the feature at Changes
Sequence DiagramsequenceDiagram
actor User
participant FaqsPage
participant Accordion
participant FaqItem
User->>FaqsPage: Load /faqs
FaqsPage->>FaqsPage: Initialize activeCategory = "Payment & billing"
FaqsPage->>FaqsPage: Filter faqData by category
FaqsPage->>Accordion: Render with filtered items
User->>FaqsPage: Click category button
FaqsPage->>FaqsPage: Update activeCategory state
FaqsPage->>FaqsPage: Filter faqData by new category
FaqsPage->>Accordion: Re-render with updated items
User->>Accordion: Click FAQ item
Accordion->>Accordion: Update openId state
Accordion->>FaqItem: Pass isOpen=true
FaqItem->>FaqItem: Animate answer section reveal
FaqItem->>User: Display answer with icon toggle
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/components/Accordion.tsx`:
- Around line 9-29: Accordion's openId state persists across category switches
because the component instance is reused; either reset openId when the items
prop changes or force remount from the parent. Add a useEffect inside Accordion
that watches the items array (or items.length and item ids) and calls
setOpenId(null) when it changes to clear any stale openId (refer to Accordion,
useState/openId/setOpenId, items, handleToggle), or alternatively in FaqsPage
provide a unique key on the Accordion (e.g., key={selectedCategoryId}) so React
remounts it when the category changes.
In `@src/data/faqs.ts`:
- Around line 1-21: The faqData array currently only contains FAQs with category
'Technical Support' (see FAQ and faqData) causing empty default views; add
representative entries for the missing categories 'Payment & billing' and
'Shopping & Orders' (or change one or more existing FAQ.category values
accordingly) so each declared category has at least one FAQ; update the faqData
array with new objects including id, question, answer, and the correct category
strings to ensure FaqsPage renders content for all categories.
In `@src/pages/FaqsPage.tsx`:
- Around line 8-20: FaqsPage initializes activeCategory to a hardcoded 'Payment
& billing' which is empty in faqData, causing an empty page; change the
initialization to pick a category that actually has items by computing the first
category present in faqData (e.g., derive initialActive = categories.find(c =>
faqData.some(f => f.category === c)) || categories[0]) or set activeCategory
after mount (useEffect) using faqData, and ensure filteredFaqs uses that
computed value; update references to activeCategory/setActiveCategory and the
filteredFaqs computation accordingly so the page defaults to a non-empty
category.
- Line 7: Replace the manual Category union with a derived type from the FAQ
model: change the Category declaration to derive from the FAQ interface
(FAQ['category']) so it always matches the source-of-truth, and update any uses
such as the categories array in FaqsPage to be typed/constructed from that
derived type; locate the Category type, the FAQ interface reference (FAQ) and
the categories array in this file to make the substitution and ensure no
duplicate manual unions remain.
🧹 Nitpick comments (2)
src/pages/FaqsPage.tsx (1)
50-50: Redundantmax-w-3xl mx-autowrapper.This
<div>appliesmax-w-3xl mx-auto, butAccordion.tsx(Line 18) already wraps its content with the same constraint. The double nesting is harmless but redundant — consider removing one.src/components/FaqItem.tsx (1)
14-34: Minor accessibility gap: missingaria-controls/idpairing.The button has
aria-expandedwhich is good, but for full WAI-ARIA accordion compliance, the button should reference the controlled panel viaaria-controls, and the panel should have a correspondingid. Consider accepting the FAQidas a prop to generate these attributes.Sketch
interface FaqItemProps { + id: number; question: string; answer: string; isOpen: boolean; onToggle: () => void; } -export const FaqItem: React.FC<FaqItemProps> = ({ question, answer, isOpen, onToggle }) => { +export const FaqItem: React.FC<FaqItemProps> = ({ id, question, answer, isOpen, onToggle }) => { + const panelId = `faq-panel-${id}`; + const headerId = `faq-header-${id}`; ... <button + id={headerId} + aria-controls={panelId} aria-expanded={isOpen} ... > ... <div + id={panelId} + role="region" + aria-labelledby={headerId} className={...} >
| const Accordion: React.FC<AccordionProps> = ({ items }) => { | ||
| // We store the ID of the open FAQ. If null, all are closed. | ||
| const [openId, setOpenId] = useState<number | null>(null); | ||
|
|
||
| const handleToggle = (id: number) => { | ||
| setOpenId(prevId => (prevId === id ? null : id)); | ||
| }; | ||
|
|
||
| return ( | ||
| <div className="max-w-3xl mx-auto"> | ||
| {items.map((item) => ( | ||
| <FaqItem | ||
| key={item.id} | ||
| question={item.question} | ||
| answer={item.answer} | ||
| isOpen={openId === item.id} | ||
| onToggle={() => handleToggle(item.id)} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Stale openId persists across category switches.
When the user changes categories in FaqsPage, the Accordion instance is reused (same component, no key), so openId retains its previous value. If an FAQ in the new category happens to share the same id, it will appear pre-expanded unexpectedly.
Fix either here (reset openId when items changes) or in FaqsPage.tsx by adding a key on the Accordion:
Option A: Add key in FaqsPage.tsx (simpler)
- <Accordion items={filteredFaqs} />
+ <Accordion key={activeCategory} items={filteredFaqs} />Option B: Reset state inside Accordion via useEffect
+import React, { useState, useEffect } from 'react';
...
const Accordion: React.FC<AccordionProps> = ({ items }) => {
const [openId, setOpenId] = useState<number | null>(null);
+
+ useEffect(() => {
+ setOpenId(null);
+ }, [items]);📝 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 Accordion: React.FC<AccordionProps> = ({ items }) => { | |
| // We store the ID of the open FAQ. If null, all are closed. | |
| const [openId, setOpenId] = useState<number | null>(null); | |
| const handleToggle = (id: number) => { | |
| setOpenId(prevId => (prevId === id ? null : id)); | |
| }; | |
| return ( | |
| <div className="max-w-3xl mx-auto"> | |
| {items.map((item) => ( | |
| <FaqItem | |
| key={item.id} | |
| question={item.question} | |
| answer={item.answer} | |
| isOpen={openId === item.id} | |
| onToggle={() => handleToggle(item.id)} | |
| /> | |
| ))} | |
| </div> | |
| ); | |
| const Accordion: React.FC<AccordionProps> = ({ items }) => { | |
| // We store the ID of the open FAQ. If null, all are closed. | |
| const [openId, setOpenId] = useState<number | null>(null); | |
| useEffect(() => { | |
| setOpenId(null); | |
| }, [items]); | |
| const handleToggle = (id: number) => { | |
| setOpenId(prevId => (prevId === id ? null : id)); | |
| }; | |
| return ( | |
| <div className="max-w-3xl mx-auto"> | |
| {items.map((item) => ( | |
| <FaqItem | |
| key={item.id} | |
| question={item.question} | |
| answer={item.answer} | |
| isOpen={openId === item.id} | |
| onToggle={() => handleToggle(item.id)} | |
| /> | |
| ))} | |
| </div> | |
| ); |
🤖 Prompt for AI Agents
In `@src/components/Accordion.tsx` around lines 9 - 29, Accordion's openId state
persists across category switches because the component instance is reused;
either reset openId when the items prop changes or force remount from the
parent. Add a useEffect inside Accordion that watches the items array (or
items.length and item ids) and calls setOpenId(null) when it changes to clear
any stale openId (refer to Accordion, useState/openId/setOpenId, items,
handleToggle), or alternatively in FaqsPage provide a unique key on the
Accordion (e.g., key={selectedCategoryId}) so React remounts it when the
category changes.
| export interface FAQ { | ||
| id: number; | ||
| question: string; | ||
| answer: string; | ||
| category: 'Payment & billing' | 'Shopping & Orders' | 'Technical Support'; | ||
| } | ||
|
|
||
| export const faqData: FAQ[] = [ | ||
| { | ||
| id: 1, | ||
| question: "What materials are ZAYQ cases made from?", | ||
| answer: "ZAYQ cases are made from high-quality, durable materials including polycarbonate and TPU (Thermoplastic Polyurethane) for maximum protection and flexibility.", | ||
| category: 'Technical Support' | ||
| }, | ||
| { | ||
| id: 2, | ||
| question: "Are ZAYQ cases compatible with wireless charging?", | ||
| answer: "Yes, all ZAYQ cases are designed to be compatible with wireless charging, allowing you to charge your device without removing the case.", | ||
| category: 'Technical Support' | ||
| } | ||
| ]; No newline at end of file |
There was a problem hiding this comment.
Data only covers one of three categories.
Both entries are 'Technical Support'. The 'Payment & billing' and 'Shopping & Orders' categories are declared but have no content, which causes the default view in FaqsPage to render empty (flagged separately). This is likely placeholder data — just noting for awareness so it's populated before shipping to production.
🤖 Prompt for AI Agents
In `@src/data/faqs.ts` around lines 1 - 21, The faqData array currently only
contains FAQs with category 'Technical Support' (see FAQ and faqData) causing
empty default views; add representative entries for the missing categories
'Payment & billing' and 'Shopping & Orders' (or change one or more existing
FAQ.category values accordingly) so each declared category has at least one FAQ;
update the faqData array with new objects including id, question, answer, and
the correct category strings to ensure FaqsPage renders content for all
categories.
| import type { FAQ } from '../data/faqs'; | ||
|
|
||
| // Define the literal types to match your faqs.ts exactly | ||
| type Category = 'Payment & billing' | 'Shopping & Orders' | 'Technical Support'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Category type is duplicated from FAQ['category'] — derive it instead.
This union is manually kept in sync with the category field in src/data/faqs.ts. If a new category is added to the FAQ interface, this type and the categories array on Line 13 won't update, causing a silent mismatch.
Suggested fix
-type Category = 'Payment & billing' | 'Shopping & Orders' | 'Technical Support';
+type Category = FAQ['category'];🤖 Prompt for AI Agents
In `@src/pages/FaqsPage.tsx` at line 7, Replace the manual Category union with a
derived type from the FAQ model: change the Category declaration to derive from
the FAQ interface (FAQ['category']) so it always matches the source-of-truth,
and update any uses such as the categories array in FaqsPage to be
typed/constructed from that derived type; locate the Category type, the FAQ
interface reference (FAQ) and the categories array in this file to make the
substitution and ensure no duplicate manual unions remain.
|
|
||
| const FaqsPage: React.FC = () => { | ||
| // 1. State to track the active category | ||
| const [activeCategory, setActiveCategory] = useState<Category>('Payment & billing'); | ||
|
|
||
| const categories: Category[] = [ | ||
| 'Payment & billing', | ||
| 'Shopping & Orders', | ||
| 'Technical Support' | ||
| ]; | ||
|
|
||
| // 2. Filter the data based on the state | ||
| const filteredFaqs = faqData.filter((faq: FAQ) => faq.category === activeCategory); |
There was a problem hiding this comment.
Default category shows an empty page on first load.
activeCategory defaults to 'Payment & billing', but the current faqData only contains 'Technical Support' entries. Users will land on an empty FAQ page with "No questions found in this category." Consider defaulting to a category that has content, or dynamically picking the first category that has items.
🤖 Prompt for AI Agents
In `@src/pages/FaqsPage.tsx` around lines 8 - 20, FaqsPage initializes
activeCategory to a hardcoded 'Payment & billing' which is empty in faqData,
causing an empty page; change the initialization to pick a category that
actually has items by computing the first category present in faqData (e.g.,
derive initialActive = categories.find(c => faqData.some(f => f.category === c))
|| categories[0]) or set activeCategory after mount (useEffect) using faqData,
and ensure filteredFaqs uses that computed value; update references to
activeCategory/setActiveCategory and the filteredFaqs computation accordingly so
the page defaults to a non-empty category.
naheel0
left a comment
naheel0
left a comment
There was a problem hiding this comment.
Thanks for the implementation — good structure overall 👍
Requesting changes before merge:
Accordion state bug:
openId persists when switching categories. Reset the state when items change or add a key to based on activeCategory.
Duplicate Category type:
Don’t manually define the Category union in FaqsPage.tsx.
Derive it from FAQ['category'] to avoid future mismatch.
Empty default category:
The page defaults to 'Payment & billing', but there’s no data for it.
Either populate all categories or default to one that has items.
Incomplete data:
Currently only Technical Support has FAQs. Add entries for the other categories before production.
Minor:
Add a newline at the end of faqs.ts.
Please address these and push an update. I’ll review again after.
2 participants
🚀 BΞYTΞFLʘW | Pull Request Protocol
PR Type:
featIssue Link: #16
📝 System Summary
Introduced a centralized, data-driven FAQ system to the application. This implementation provides a scalable way to handle common user queries through a modular Accordion architecture, ensuring that content and logic remain decoupled.
🛠️ Technical Changes
Created a toggle-state logic within the Accordion to manage expanding/collapsing of FAQ items.FaqsPage.tsx: Main view container. Accordion.tsx: Wrapper component for the list. FaqItem.tsx: Individual question/answer unit with transition logic.Externalized content to src/data/faqs.ts for easy text updates without redeploying logic changes.🧪 Quality Assurance (QA)
🖼️ Visual Evidence
If this PR affects the UI, drop a screenshot or GIF below:
capture_20260211163254910.bmp
📡 Developer Authorization
Authorized by: @anastkalas
Timestamp: {{ 2/11/2026 }}
Summary by CodeRabbit