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. |
📝 WalkthroughWalkthroughAdds a new FAQs feature: a Changes
Sequence DiagramsequenceDiagram
actor User
participant FaqsPage as FaqsPage
participant Accordion as Accordion
participant FaqItem as FaqItem
participant Data as faqData
User->>FaqsPage: Navigate to /faqs
FaqsPage->>FaqsPage: set activeCategory (default)
FaqsPage->>Data: read faqData
FaqsPage->>FaqsPage: filter faqData by activeCategory
FaqsPage->>Accordion: render(filteredItems)
User->>FaqsPage: Click category tab
FaqsPage->>FaqsPage: update activeCategory
FaqsPage->>Accordion: render(updatedFilteredItems)
User->>Accordion: Click question item
Accordion->>Accordion: update openId
Accordion->>FaqItem: set isOpen=true for selected item
FaqItem->>User: reveal answer (animated)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ 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[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.
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.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/data/faqs.ts`:
- Around line 23-25: Fix the typo and extra space in the FAQ entry by updating
the answer property of the relevant FAQ object: change "payments methods" to
"payment methods" and remove the double space before "Apple Pay" so the answer
reads with correct grammar and spacing; locate the object with keys question:
"What payment methods do you accept?" and answer to apply the edit.
🧹 Nitpick comments (3)
src/pages/FaqsPage.tsx (3)
1-1: UnusedReactimport with the new JSX transform.With React 19 and the modern JSX transform, importing
Reactis unnecessary. Only the named hooks are needed.✨ Suggested change
-import React, { useState } from 'react'; +import { useState } from 'react';
9-11:React.FCstill references theReactnamespace — remove if you drop the import.If you remove the
Reactdefault import per the suggestion above, update this line too.✨ Suggested change
-const FaqsPage: React.FC = () => { +const FaqsPage = () => {
13-17:categoriesarray can drift from theCategorytype without a compile-time error.If a fourth category is added to
FAQ['category'], this array won't be updated automatically and that category's FAQs will be unreachable. Consider deriving categories fromfaqDataor using asatisfiesassertion to ensure exhaustiveness.♻️ Option A — derive from data
- const categories: Category[] = [ - 'Technical Support', - 'Shopping & Orders', - 'Payment & billing' - ]; + const categories: Category[] = [...new Set(faqData.map((f) => f.category))];This auto-discovers categories from the data. If ordering matters, you can define a sort/order map separately.
src/data/faqs.ts
Outdated
| question: "What payment methods do you accept?", | ||
| answer: "We accept a variety of payments methods including credit cards, PayPal and Apple Pay for a seamless checkout experience.", | ||
| category: 'Payment & billing' |
There was a problem hiding this comment.
Typo in answer text: "payments methods" → "payment methods", and extra space before "Apple Pay".
Line 24 has a minor grammatical error and a double space.
✏️ Proposed fix
- answer: "We accept a variety of payments methods including credit cards, PayPal and Apple Pay for a seamless checkout experience.",
+ answer: "We accept a variety of payment methods including credit cards, PayPal and Apple Pay for a seamless checkout experience.",📝 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.
| question: "What payment methods do you accept?", | |
| answer: "We accept a variety of payments methods including credit cards, PayPal and Apple Pay for a seamless checkout experience.", | |
| category: 'Payment & billing' | |
| question: "What payment methods do you accept?", | |
| answer: "We accept a variety of payment methods including credit cards, PayPal and Apple Pay for a seamless checkout experience.", | |
| category: 'Payment & billing' |
🤖 Prompt for AI Agents
In `@src/data/faqs.ts` around lines 23 - 25, Fix the typo and extra space in the
FAQ entry by updating the answer property of the relevant FAQ object: change
"payments methods" to "payment methods" and remove the double space before
"Apple Pay" so the answer reads with correct grammar and spacing; locate the
object with keys question: "What payment methods do you accept?" and answer to
apply the edit.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/FaqItem.tsx`:
- Around line 19-22: The toggle button in FaqItem.tsx currently removes the
focus indicator via the class "focus:outline-none", which hides keyboard focus;
update the button's className (the JSX button with props onToggle and
aria-expanded) to retain no outline for mouse but add a visible keyboard focus
ring by replacing the bare outline suppression with a focus-visible ring utility
(e.g., add focus-visible:ring and a color/offset utility) so keyboard users get
a clear focus indicator while mouse users keep the original styling.
🧹 Nitpick comments (1)
src/pages/FaqsPage.tsx (1)
14-18:categoriesarray won't surface a compile error if a new category is added to theFAQunion.If someone adds a fourth category to the
FAQinterface, this array silently omits it. You can use asatisfiesassertion to get exhaustiveness checking while preserving the explicit ordering:- const categories: Category[] = [ + const categories = [ "Technical Support", "Shopping & Orders", "Payment & billing", - ]; + ] as const satisfies readonly Category[];This still lets you control display order but will error if the array contains an invalid category. To also catch missing categories you'd need a helper, but this is a pragmatic improvement.
| <button | ||
| onClick={onToggle} | ||
| aria-expanded={isOpen} | ||
| className="flex w-full items-center justify-between text-left focus:outline-none" |
There was a problem hiding this comment.
focus:outline-none removes the focus indicator without a replacement.
Keyboard users lose all visual focus feedback on the toggle button. Add a focus-visible ring so the outline is only suppressed for mouse clicks:
- className="flex w-full items-center justify-between text-left focus:outline-none"
+ className="flex w-full items-center justify-between text-left focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-2 rounded-md"📝 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.
| <button | |
| onClick={onToggle} | |
| aria-expanded={isOpen} | |
| className="flex w-full items-center justify-between text-left focus:outline-none" | |
| <button | |
| onClick={onToggle} | |
| aria-expanded={isOpen} | |
| className="flex w-full items-center justify-between text-left focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-2 rounded-md" |
🤖 Prompt for AI Agents
In `@src/components/FaqItem.tsx` around lines 19 - 22, The toggle button in
FaqItem.tsx currently removes the focus indicator via the class
"focus:outline-none", which hides keyboard focus; update the button's className
(the JSX button with props onToggle and aria-expanded) to retain no outline for
mouse but add a visible keyboard focus ring by replacing the bare outline
suppression with a focus-visible ring utility (e.g., add focus-visible:ring and
a color/offset utility) so keyboard users get a clear focus indicator while
mouse users keep the original styling.
|
@anastkalas amazing work |
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