-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(#775): now we see one a question at a time #785
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Files ignored due to filter (1)
- src/data/faq.json
Files selected for processing (4)
- .husky/commit-msg (1 hunks)
- src/components/FaqAccordian.js (2 hunks)
- src/components/faq.js (1 hunks)
- src/data/faq.js (1 hunks)
Files skipped from review due to trivial changes (1)
- .husky/commit-msg
Additional comments (Suppressed): 9
src/data/faq.js (1)
- 1-36: The data structure and content of the FAQs look good. The
id
field is unique for each FAQ, which is important for identifying each FAQ uniquely in the UI. Thequestions
andanswers
fields are self-explanatory and contain the necessary information.src/components/faq.js (4)
1-4: The import statements have been updated to import
faqData
from "data/faq" instead ofFaqs
from "data/faq.json". This change implies that the data source for FAQs has been updated. Please ensure that the new data source is correctly set up and contains the expected data structure.8-8: The
useState
hook is used to manage thecurrentQuestion
state. This is a new addition to the component and it's used to track the currently selected FAQ question.16-18: The
FaqAccordian
component now receives two additional props:currentQuestion
andsetCurrentQuestion
. Ensure that theFaqAccordian
component has been updated to handle these new props correctly.16-18: The
key
prop for theFaqAccordian
component has been changed fromidx
tofaq.id
. This change assumes that each FAQ object has a uniqueid
property. Please verify that this is the case to avoid potential issues with React's reconciliation process.src/components/FaqAccordian.js (4)
6-13: The
onClickHandler
function has been modified to set thecurrentQuestion
to thefaq.id
of the clicked FAQ item. If the clicked FAQ item is already thecurrentQuestion
, it resets thecurrentQuestion
to0
. This change allows only one FAQ item to be open at a time. Ensure that this behavior is intended and that thecurrentQuestion
state is properly managed in the parent component.22-24: The conditional class assignment based on whether
currentQuestion
equalsfaq.id
has been correctly updated from the previousisOpen
state. This change is consistent with the new behavior of allowing only one FAQ item to be open at a time.29-31: The rotation of the arrow icon is now based on whether
currentQuestion
equalsfaq.id
, which is consistent with the new behavior of allowing only one FAQ item to be open at a time.36-39: The conditional rendering of the answer paragraph is now based on whether
currentQuestion
equalsfaq.id
, which is consistent with the new behavior of allowing only one FAQ item to be open at a time.
onClick={onClickHandler} | ||
onKeyDown={onClickHandler} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The onClickHandler
function is used for both onClick
and onKeyDown
events. This means that any key press will trigger the function, which may not be the desired behavior. Consider checking the key code in the onKeyDown
event to ensure that the function is only triggered by specific keys (e.g., Enter or Space).
- onKeyDown={onClickHandler}
+ onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') onClickHandler() }}
Hey there @AliReza1083, can you check your Linting issues here? There seems to be an error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please run eslint once and check out the comment by @coderabbitai
onClick={() => setIsOpen(!isOpen)} | ||
onKeyDown={() => setIsOpen(!isOpen)} | ||
onClick={onClickHandler} | ||
onKeyDown={onClickHandler} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onKeyDown={onClickHandler} | |
onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') onClickHandler() }} |
@@ -40,7 +46,7 @@ const FaqAccordian = ({ faq }) => { | |||
</AnimatePresence> | |||
</div> | |||
</li> | |||
) | |||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
); | |
) |
const Faq = () => { | ||
const [currentQuestion, setCurrentQuestion] = useState(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const [currentQuestion, setCurrentQuestion] = useState(1); | |
const [currentQuestion, setCurrentQuestion] = useState(1) |
export const faqData = [ | ||
{ | ||
id: 1, | ||
questions: 'What does "4C" stands for?', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
questions: 'What does "4C" stands for?', | |
questions: "What does "4C" stands for?", |
{ | ||
id: 1, | ||
questions: 'What does "4C" stands for?', | ||
answers: '4C stands for "Cool Community of Content Creators".', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
answers: '4C stands for "Cool Community of Content Creators".', | |
answers: "4C stands for \"Cool Community of Content Creators\".", |
}, | ||
{ | ||
id: 2, | ||
questions: 'What is the goal of "4C"? ', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
questions: 'What is the goal of "4C"? ', | |
questions: "What is the goal of "4C"?", |
}, | ||
{ | ||
id: 3, | ||
questions: 'Is "4C" open-source and how can I contribute?', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
questions: 'Is "4C" open-source and how can I contribute?', | |
questions: "Is \"4C\" open-source and how can I contribute?", |
}, | ||
{ | ||
id: 5, | ||
questions: 'Is "4C" only for experienced content creators?', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
questions: 'Is "4C" only for experienced content creators?', | |
questions: "Is \"4C\" only for experienced content creators?", |
}, | ||
{ | ||
id: 6, | ||
questions: 'What are the benefits of the "4C" community?', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
questions: 'What are the benefits of the "4C" community?', | |
questions: 'What are the benefits of the \"4C\" community?', |
answers: | ||
"You get support, activities, collaborations, free dedicated content creation webinars (yes, you got it right ! 😄), coffee chat, and a meme channel! ", | ||
}, | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
]; | |
] |
@AliReza1083 @tobySolutions I am closing this |
Closes #775
Summary by CodeRabbit