Refactor(eduaid_web): Clean up QA parsing and utilities, reduce duplication#640
Open
ashi2004 wants to merge 1 commit into
Open
Refactor(eduaid_web): Clean up QA parsing and utilities, reduce duplication#640ashi2004 wants to merge 1 commit into
ashi2004 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR extracts repeated QA parsing, shuffling, and API handling logic from individual page components into centralized utility modules ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
eduaid_web/src/pages/Question_Type.jsx (1)
38-76:⚠️ Potential issue | 🔴 CriticalCritical: Broken JSX syntax will prevent compilation.
The JSX structure is completely malformed. The
<button>opening tag (lines 38-39),aria-pressedattribute (line 45), and</button>closing tag (line 76) are scattered in wrong locations instead of properly wrapping each option element.Static analysis confirms:
- Line 40: "expected
...but instead found}"- Line 41: "expected
>but instead found<"- Lines 38-40: "Expected corresponding JSX closing tag for 'button'"
The intent appears to be converting each clickable
<div>option into a semantic<button>, but the edits were applied incorrectly. The button tags and attributes need to be inside the.map()loop, replacing the<div>wrapper.🐛 Proposed fix to correct the JSX structure
Remove the misplaced lines 38-39, 45, and 76, then properly convert each option
<div>to a<button>:- <button - type="button" {/* Title */} <div className="text-center"> <h2 className="text-white text-3xl sm:text-4xl font-extrabold"> What's on your Mind? </h2> - aria-pressed={selectedOption === option.id} {/* Options */} <div className="flex flex-col items-center mt-8 gap-4 w-full"> {[ { id: "get_shortq", label: "Short-Answer Type Questions" }, { id: "get_mcq", label: "Multiple Choice Questions" }, { id: "get_boolq", label: "True/False Questions" }, { id: "get_problems", label: "All Questions" }, ].map((option) => ( - <div + <button + type="button" key={option.id} onClick={() => handleOptionClick(option.id)} className={`w-full max-w-xl flex items-center gap-6 px-6 py-5 rounded-xl cursor-pointer bg-[`#202838`] bg-opacity-50 hover:bg-opacity-70 transition-all duration-200 ${ selectedOption === option.id ? "ring-2 ring-[`#00CBE7`]" : "" }`} - role="button" - tabIndex={0} - onKeyDown={(e) => { - if (e.key === "Enter") handleOptionClick(option.id); - }} + aria-pressed={selectedOption === option.id} > <div className={`w-10 h-10 rounded-full flex-shrink-0 ${ selectedOption === option.id ? "bg-gradient-to-b from-[`#405EED`] to-[`#01CBE7`]" : "bg-[`#999C9D`]" }`} ></div> <div className="text-white text-xl sm:text-2xl font-medium"> {option.label} </div> - </button> + </button> ))}Note: When using a semantic
<button>,role="button",tabIndex={0}, andonKeyDownhandlers are no longer needed—the button provides these behaviors natively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/pages/Question_Type.jsx` around lines 38 - 76, The JSX is broken because a stray opening <button> and attributes were inserted outside the .map() loop and a closing </button> ended up after the options; fix by removing the misplaced opening tag/attributes (the lines that start the button before the Title and the stray aria-pressed and final </button>) and instead convert the option wrapper <div key={option.id} ...> inside the .map() into a semantic <button key={option.id} onClick={() => handleOptionClick(option.id)} aria-pressed={selectedOption === option.id} className={...}> (drop role="button", tabIndex, and onKeyDown since <button> is native). Keep the inner color circle and label markup, and preserve the selectedOption conditional class logic and onClick handler in the Button so compilation and accessibility are correct in Question_Type.jsx.
🧹 Nitpick comments (2)
eduaid_web/src/pages/Question_Type.jsx (1)
83-89: Potential race condition:localStorage.setItemmay not complete before navigation.The
onClickhandler callshandleSaveToLocalStorage()synchronously, but<Link>navigation may proceed immediately. WhilelocalStorage.setItemis synchronous in practice, this pattern can be fragile if the handler ever becomes async or if additional logic is added.Consider using
useNavigatefor programmatic navigation after the save completes:♻️ Suggested refactor for explicit navigation control
+import { Link, useNavigate } from "react-router-dom"; const Question_Type = () => { const [selectedOption, setSelectedOption] = useState(null); + const navigate = useNavigate(); - const handleSaveToLocalStorage = () => { + const handleSaveAndNavigate = () => { if (selectedOption) { localStorage.setItem("selectedQuestionType", selectedOption); + navigate("/input"); } };Then replace the
<Link>with a button:- <Link - to="/input" - onClick={handleSaveToLocalStorage} - className="inline-block rounded-2xl ..." - > + <button + type="button" + onClick={handleSaveAndNavigate} + className="rounded-2xl ..." + > Fire Up 🚀 - </Link> + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/pages/Question_Type.jsx` around lines 83 - 89, The Link's onClick may navigate before storage logic completes; change to programmatic navigation by importing useNavigate and call navigate('/input') only after handleSaveToLocalStorage finishes (or after await if you make it async). Replace the <Link> with a button that calls an async wrapper (e.g., saveThenNavigate) which invokes handleSaveToLocalStorage and then calls navigate('/input'); reference the existing handleSaveToLocalStorage function and useNavigate hook to locate where to add the wrapper and swap the Link component.eduaid_web/src/utils/quizUtils.js (1)
32-33: Harden history parsing against malformed localStorage JSON.Line 32 can throw if
last5Quizzescontains invalid JSON, which breaks the post-success flow even after questions are generated.Resilient parsing fallback
- const last5Quizzes = JSON.parse(localStorage.getItem("last5Quizzes") || "[]"); + let last5Quizzes = []; + try { + last5Quizzes = JSON.parse(localStorage.getItem("last5Quizzes") || "[]"); + if (!Array.isArray(last5Quizzes)) last5Quizzes = []; + } catch { + last5Quizzes = []; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/utils/quizUtils.js` around lines 32 - 33, The JSON.parse of localStorage.getItem("last5Quizzes") can throw on malformed data; wrap the parse in a try/catch (or use a safeParse helper) to fall back to an empty array when parsing fails, then push quizDetails into that safe array (variable last5Quizzes) and continue to update localStorage; reference the existing last5Quizzes variable and quizDetails so you only change the retrieval/parse logic and preserve subsequent push/set operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eduaid_web/src/pages/Text_Input.jsx`:
- Line 168: The helper text shown to users is out of sync with the file input's
accepted types: the input element (the <input ... ref={fileInputRef}
onChange={handleFileUpload} accept=".pdf,.mp3,.wav,.txt" />) accepts .wav and
.txt but the UI copy still says only “PDF, MP3 supported”; update the visible
helper/caption text where fileInputRef/handleFileUpload are referenced so it
lists all accepted types (PDF, MP3, WAV, TXT) or otherwise matches the accept
attribute, and ensure any accessible hints (aria-describedby or labels) are
updated as well.
In `@eduaid_web/src/utils/githubStars.js`:
- Around line 1-2: The constants STARS_KEY and FETCH_TIME_KEY are too generic
and collide with the extension; rename them to namespaced keys (e.g., STARS_KEY
-> "eduaid_web:stars" and FETCH_TIME_KEY -> "eduaid_web:fetchTime") by updating
the constants in githubStars.js and ensuring every usage of STARS_KEY and
FETCH_TIME_KEY (reads/writes/removals) in this module or imports that reference
these symbols uses the new names so localStorage entries for the web app are
isolated from the extension.
In `@eduaid_web/src/utils/quizUtils.js`:
- Around line 69-98: The current logic pushes the same qaPairsFromStorage.output
twice (once as MCQ when output_mcq exists or questionType === "get_mcq", and
again as Short), causing duplicates in combinedQaPairs; change the branching so
output_mcq and output are treated as distinct sources: if
qaPairsFromStorage.output_mcq exists iterate that array for MCQs; only treat
qaPairsFromStorage.output as MCQ when questionType === "get_mcq" AND no
output_mcq is present; ensure the final else-if that pushes Short entries
explicitly excludes cases where questionType === "get_mcq" or output_mcq is
present so the same array is not parsed twice into combinedQaPairs.
---
Outside diff comments:
In `@eduaid_web/src/pages/Question_Type.jsx`:
- Around line 38-76: The JSX is broken because a stray opening <button> and
attributes were inserted outside the .map() loop and a closing </button> ended
up after the options; fix by removing the misplaced opening tag/attributes (the
lines that start the button before the Title and the stray aria-pressed and
final </button>) and instead convert the option wrapper <div key={option.id}
...> inside the .map() into a semantic <button key={option.id} onClick={() =>
handleOptionClick(option.id)} aria-pressed={selectedOption === option.id}
className={...}> (drop role="button", tabIndex, and onKeyDown since <button> is
native). Keep the inner color circle and label markup, and preserve the
selectedOption conditional class logic and onClick handler in the Button so
compilation and accessibility are correct in Question_Type.jsx.
---
Nitpick comments:
In `@eduaid_web/src/pages/Question_Type.jsx`:
- Around line 83-89: The Link's onClick may navigate before storage logic
completes; change to programmatic navigation by importing useNavigate and call
navigate('/input') only after handleSaveToLocalStorage finishes (or after await
if you make it async). Replace the <Link> with a button that calls an async
wrapper (e.g., saveThenNavigate) which invokes handleSaveToLocalStorage and then
calls navigate('/input'); reference the existing handleSaveToLocalStorage
function and useNavigate hook to locate where to add the wrapper and swap the
Link component.
In `@eduaid_web/src/utils/quizUtils.js`:
- Around line 32-33: The JSON.parse of localStorage.getItem("last5Quizzes") can
throw on malformed data; wrap the parse in a try/catch (or use a safeParse
helper) to fall back to an empty array when parsing fails, then push quizDetails
into that safe array (variable last5Quizzes) and continue to update
localStorage; reference the existing last5Quizzes variable and quizDetails so
you only change the retrieval/parse logic and preserve subsequent push/set
operations.
🪄 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: 08fa0d55-388c-4e83-ad44-672fadc8fbb3
📒 Files selected for processing (6)
eduaid_web/src/pages/Home.jsxeduaid_web/src/pages/Output.jsxeduaid_web/src/pages/Question_Type.jsxeduaid_web/src/pages/Text_Input.jsxeduaid_web/src/utils/githubStars.jseduaid_web/src/utils/quizUtils.js
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.
Fixes #639
Summary:
This PR cleans up repeated logic across the project by moving common functionality into shared utility files. It mainly focuses on QA parsing, shuffling, API handling, and some small accessibility improvements.
What’s changed:
Why this matters:
Files involved:
Notes:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor