feat: I added the locked lesson progression logic #286
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces static topic lists on Grammar, Listening, Reading, and Speaking pages with a client-side per-topic progress model, sequential unlock logic, and localStorage persistence; adds a demo "Mark Complete" control. AudioAssessment API expanded and PronunciationFeedback integrated; minor formatting tweaks elsewhere. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as LessonListPage
participant Storage as localStorage
Note over Page: Initialization
Page->>Storage: getItem(STORAGE_KEY)
alt saved progress exists
Storage-->>Page: progressList
else no saved progress
Page-->>Page: init progressList (first unlocked, others locked, 0%)
end
User->>Page: Click "Mark Complete" on topic i
Page-->>Page: set progress[i]=100
Page-->>Page: if progress[i] == 100 then unlock i+1
Page->>Storage: setItem(STORAGE_KEY, progressList)
Page-->>User: render updated status (check, progress 100%, next unlocked)
sequenceDiagram
participant User
participant AudioComp as AudioAssessment
participant PF as PronunciationFeedback
User->>AudioComp: Load (props include showRecording)
alt showRecording = true
User->>AudioComp: Record & Stop
AudioComp-->>PF: pass recordedAudio, modelAudio
PF-->>AudioComp: analysis/score
AudioComp-->>User: display feedback
AudioComp-->>User: call onAnswer(analysis)
else showRecording = false
AudioComp-->>User: recording UI hidden
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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: 5
🧹 Nitpick comments (21)
src/pages/aurora-site/learning/listening-content.jsx (7)
48-49: Use lazy state initializer to avoid unnecessary work.This avoids recalculating initial progress on every render and pairs well with the SSR-safe initializer.
-const [progressList, setProgressList] = useState(getInitialProgress()); +const [progressList, setProgressList] = useState(() => getInitialProgress());
57-70: Consolidate double setState into a single pass.Two sequential state updates are batched but still perform two array walks. Merge into one update and compute unlocks from the updated array.
- const handleComplete = (idx) => { - setProgressList((prev) => - prev.map((item, i) => (i === idx ? { ...item, progress: 100 } : item)) - ); - - // Unlock next lesson if previous is completed - setProgressList((prev) => { - return prev.map((item, index) => { - if (index === 0) return { ...item, unlocked: true }; - const prevCompleted = prev[index - 1].progress === 100; - return { ...item, unlocked: prevCompleted }; - }); - }); - }; + const handleComplete = (idx) => { + setProgressList((prev) => { + const updated = prev.map((item, i) => + i === idx ? { ...item, progress: 100 } : item + ); + return updated.map((item, i) => ({ + ...item, + unlocked: i === 0 ? true : updated[i - 1].progress === 100, + })); + }); + };
1-1: Drop unused default React import (ESLint no-unused-vars).The JSX runtime doesn’t require
Reactin scope. Keep only the hooks to satisfy linting.-import React, { useEffect, useState } from "react"; +import { useEffect, useState } from "react";
4-33: Remove unused progress/unlocked in topic seeds to prevent confusion.These fields are ignored (state drives UI). Keeping them risks drift and misleads maintainers.
-const listeningTopics = [ - { - id: "basic-conversations", - title: "Basic Conversations", - progress: 100, - unlocked: true, - }, - { - id: "everyday-dialogues", - title: "Everyday Dialogues", - progress: 80, - unlocked: true, - }, - { id: "podcasts", title: "Podcasts", progress: 65, unlocked: true }, - { - id: "news-broadcasts", - title: "News Broadcasts", - progress: 45, - unlocked: true, - }, - { id: "interviews", title: "Interviews", progress: 20, unlocked: true }, - { id: "lectures", title: "Lectures", progress: 0, unlocked: false }, - { id: "movies-tv", title: "Movies & TV", progress: 0, unlocked: false }, - { - id: "advanced-listening", - title: "Advanced Listening", - progress: 0, - unlocked: false, - }, -]; +const listeningTopics = [ + { id: "basic-conversations", title: "Basic Conversations" }, + { id: "everyday-dialogues", title: "Everyday Dialogues" }, + { id: "podcasts", title: "Podcasts" }, + { id: "news-broadcasts", title: "News Broadcasts" }, + { id: "interviews", title: "Interviews" }, + { id: "lectures", title: "Lectures" }, + { id: "movies-tv", title: "Movies & TV" }, + { id: "advanced-listening", title: "Advanced Listening" }, +];
90-97: Minor a11y: announce locked state to assistive tech.Make cards focusable and expose lock state via
aria-disabled; tie the hint witharia-describedby.- <div + <div key={topic.id} - className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${ + className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${ !item.unlocked ? "opacity-60 pointer-events-none" : "" }`} + tabIndex={0} + aria-disabled={!item.unlocked} + aria-describedby={!item.unlocked ? `lock-hint-${topic.id}` : undefined} > @@ - <span className="text-xs text-neutral-4"> + <span id={`lock-hint-${topic.id}`} className="text-xs text-neutral-4"> Complete to unlock </span>Also applies to: 118-124
51-55: Optional: avoid persisting derived “unlocked” state.Since
unlockedis derivable from progress, consider persisting only{id, progress}and recomputingunlockedon load (as in the initializer). Reduces drift.
1-163: Would you like a shared hook to remove duplication across 4 pages?This file shares 90% of logic with Grammar/Reading/Speaking. I can extract a
useSequentialProgress(STORAGE_KEY, topics)hook returning[progressList, handleComplete, reset]and update all four pages.src/pages/aurora-site/learning/reading-content.jsx (5)
53-55: Use lazy initializer for state.-const [progressList, setProgressList] = useState(getInitialProgress()); +const [progressList, setProgressList] = useState(() => getInitialProgress());
61-74: Merge the two state updates into one.- const handleComplete = (idx) => { - setProgressList((prev) => - prev.map((item, i) => (i === idx ? { ...item, progress: 100 } : item)) - ); - - setProgressList((prev) => { - return prev.map((item, index) => { - if (index === 0) return { ...item, unlocked: true }; - const prevCompleted = prev[index - 1].progress === 100; - return { ...item, unlocked: prevCompleted }; - }); - }); - }; + const handleComplete = (idx) => { + setProgressList((prev) => { + const updated = prev.map((item, i) => + i === idx ? { ...item, progress: 100 } : item + ); + return updated.map((item, i) => ({ + ...item, + unlocked: i === 0 ? true : updated[i - 1].progress === 100, + })); + }); + };
1-1: Remove unused default React import (ESLint).-import React, { useEffect, useState } from "react"; +import { useEffect, useState } from "react";
4-38: Prune progress/unlocked from readingTopics seeds.Keep seeds as pure metadata; let state drive progress/unlocked.
-const readingTopics = [ - { - id: "vocabulary-building", - title: "Vocabulary Building", - progress: 100, - unlocked: true, - }, - { id: "short-texts", title: "Short Texts", progress: 95, unlocked: true }, - { - id: "comprehension", - title: "Reading Comprehension", - progress: 70, - unlocked: true, - }, - { id: "articles", title: "Articles & Essays", progress: 45, unlocked: true }, - { id: "literature", title: "Literature", progress: 15, unlocked: true }, - { - id: "technical-reading", - title: "Technical Reading", - progress: 0, - unlocked: false, - }, - { - id: "critical-analysis", - title: "Critical Analysis", - progress: 0, - unlocked: false, - }, - { - id: "research-papers", - title: "Research Papers", - progress: 0, - unlocked: false, - }, -]; +const readingTopics = [ + { id: "vocabulary-building", title: "Vocabulary Building" }, + { id: "short-texts", title: "Short Texts" }, + { id: "comprehension", title: "Reading Comprehension" }, + { id: "articles", title: "Articles & Essays" }, + { id: "literature", title: "Literature" }, + { id: "technical-reading", title: "Technical Reading" }, + { id: "critical-analysis", title: "Critical Analysis" }, + { id: "research-papers", title: "Research Papers" }, +];
86-97: Minor a11y: expose locked status.- <div + <div key={topic.id} className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${ !item.unlocked ? "opacity-60 pointer-events-none" : "" }`} + tabIndex={0} + aria-disabled={!item.unlocked} + aria-describedby={!item.unlocked ? `lock-hint-${topic.id}` : undefined} > @@ - <span className="text-xs text-neutral-4"> + <span id={`lock-hint-${topic.id}`} className="text-xs text-neutral-4"> Complete to unlock </span>Also applies to: 121-126
src/pages/aurora-site/learning/speaking-content.jsx (5)
63-65: Use lazy initializer for state.-const [progressList, setProgressList] = useState(getInitialProgress()); +const [progressList, setProgressList] = useState(() => getInitialProgress());
72-85: Single-pass state update for completion/unlock.- const handleComplete = (idx) => { - setProgressList((prev) => - prev.map((item, i) => (i === idx ? { ...item, progress: 100 } : item)) - ); - - // unlock next lesson if previous is completed - setProgressList((prev) => { - return prev.map((item, index) => { - if (index === 0) return { ...item, unlocked: true }; - const prevCompleted = prev[index - 1].progress === 100; - return { ...item, unlocked: prevCompleted }; - }); - }); - }; + const handleComplete = (idx) => { + setProgressList((prev) => { + const updated = prev.map((item, i) => + i === idx ? { ...item, progress: 100 } : item + ); + return updated.map((item, i) => ({ + ...item, + unlocked: i === 0 ? true : updated[i - 1].progress === 100, + })); + }); + };
1-1: Remove unused default React import (ESLint).-import React, { useEffect, useState } from "react"; +import { useEffect, useState } from "react";
4-48: Trim topic seeds to id/title only.-const speakingTopics = [ - { - id: "basic-pronunciation", - title: "Basic Pronunciation", - progress: 100, - unlocked: true, - }, - { - id: "everyday-phrases", - title: "Everyday Phrases", - progress: 85, - unlocked: true, - }, - { - id: "conversation-skills", - title: "Conversation Skills", - progress: 60, - unlocked: true, - }, - { - id: "fluency-building", - title: "Fluency Building", - progress: 40, - unlocked: true, - }, - { - id: "public-speaking", - title: "Public Speaking", - progress: 0, - unlocked: false, - }, - { - id: "debate-discussion", - title: "Debate & Discussion", - progress: 0, - unlocked: false, - }, - { id: "presentations", title: "Presentations", progress: 0, unlocked: false }, - { - id: "accent-reduction", - title: "Accent Reduction", - progress: 0, - unlocked: false, - }, -]; +const speakingTopics = [ + { id: "basic-pronunciation", title: "Basic Pronunciation" }, + { id: "everyday-phrases", title: "Everyday Phrases" }, + { id: "conversation-skills", title: "Conversation Skills" }, + { id: "fluency-building", title: "Fluency Building" }, + { id: "public-speaking", title: "Public Speaking" }, + { id: "debate-discussion", title: "Debate & Discussion" }, + { id: "presentations", title: "Presentations" }, + { id: "accent-reduction", title: "Accent Reduction" }, +];
105-110: Minor a11y improvement (same as other pages).- <div + <div key={topic.id} className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${ !item.unlocked ? "opacity-60 pointer-events-none" : "" }`} + tabIndex={0} + aria-disabled={!item.unlocked} + aria-describedby={!item.unlocked ? `lock-hint-${topic.id}` : undefined} > @@ - <span className="text-xs text-neutral-4"> + <span id={`lock-hint-${topic.id}`} className="text-xs text-neutral-4"> Complete to unlock </span>Also applies to: 133-139
src/pages/aurora-site/grammar-content.jsx (4)
63-65: Lazy-initialize state.-const [progressList, setProgressList] = useState(getInitialProgress()); +const [progressList, setProgressList] = useState(() => getInitialProgress());
71-85: Single-pass completion/unlock update.- const handleComplete = (idx) => { - setProgressList((prev) => - prev.map((item, i) => (i === idx ? { ...item, progress: 100 } : item)) - ); - - // Unlock next lesson if previous is completed - setProgressList((prev) => { - return prev.map((item, index) => { - if (index === 0) return { ...item, unlocked: true }; - const prevCompleted = prev[index - 1].progress === 100; - return { ...item, unlocked: prevCompleted }; - }); - }); - }; + const handleComplete = (idx) => { + setProgressList((prev) => { + const updated = prev.map((item, i) => + i === idx ? { ...item, progress: 100 } : item + ); + return updated.map((item, i) => ({ + ...item, + unlocked: i === 0 ? true : updated[i - 1].progress === 100, + })); + }); + };
1-1: Remove unused default React import (ESLint).-import React, { useEffect, useState } from "react"; +import { useEffect, useState } from "react";
4-48: Normalize topic seeds; remove misleading progress/unlocked values.Some topics (e.g., “Plural Nouns”) show
unlocked: truewithprogress: 0, which conflicts with the sequential-unlock rule. Keep topics as id/title only.-const grammarTopics = [ - { - id: "present-simple", - title: "Present Simple", - progress: 100, - unlocked: true, - }, - { - id: "present-continuous", - title: "Present Continuous", - progress: 75, - unlocked: true, - }, - { - id: "articles", - title: "Articles (A/An/The)", - progress: 30, - unlocked: true, - }, - { id: "plural-nouns", title: "Plural Nouns", progress: 0, unlocked: true }, - { - id: "basic-pronouns", - title: "Basic Pronouns", - progress: 0, - unlocked: false, - }, - { - id: "subject-verb", - title: "Subject-Verb Agreement", - progress: 0, - unlocked: false, - }, - { - id: "possessive-adj", - title: "Possessive Adjectives", - progress: 0, - unlocked: false, - }, - { - id: "prepositions", - title: "Prepositions of Place", - progress: 0, - unlocked: false, - }, -]; +const grammarTopics = [ + { id: "present-simple", title: "Present Simple" }, + { id: "present-continuous", title: "Present Continuous" }, + { id: "articles", title: "Articles (A/An/The)" }, + { id: "plural-nouns", title: "Plural Nouns" }, + { id: "basic-pronouns", title: "Basic Pronouns" }, + { id: "subject-verb", title: "Subject-Verb Agreement" }, + { id: "possessive-adj", title: "Possessive Adjectives" }, + { id: "prepositions", title: "Prepositions of Place" }, +];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/components/Games/word-matching/game-board.jsx(1 hunks)src/pages/aurora-site/grammar-content.jsx(2 hunks)src/pages/aurora-site/learning/listening-content.jsx(1 hunks)src/pages/aurora-site/learning/reading-content.jsx(1 hunks)src/pages/aurora-site/learning/speaking-content.jsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/pages/aurora-site/learning/listening-content.jsx (3)
src/pages/aurora-site/learning/reading-content.jsx (5)
STORAGE_KEY(40-40)getInitialProgress(42-51)stored(43-43)progressList(54-54)handleComplete(62-74)src/pages/aurora-site/grammar-content.jsx (5)
STORAGE_KEY(50-50)getInitialProgress(52-61)stored(53-53)progressList(64-64)handleComplete(72-85)src/pages/aurora-site/learning/speaking-content.jsx (5)
STORAGE_KEY(50-50)getInitialProgress(52-61)stored(53-53)progressList(64-64)handleComplete(72-85)
src/pages/aurora-site/learning/speaking-content.jsx (3)
src/pages/aurora-site/learning/reading-content.jsx (5)
STORAGE_KEY(40-40)getInitialProgress(42-51)stored(43-43)progressList(54-54)handleComplete(62-74)src/pages/aurora-site/learning/listening-content.jsx (5)
STORAGE_KEY(35-35)getInitialProgress(37-46)stored(38-38)progressList(49-49)handleComplete(57-70)src/pages/aurora-site/grammar-content.jsx (5)
STORAGE_KEY(50-50)getInitialProgress(52-61)stored(53-53)progressList(64-64)handleComplete(72-85)
src/pages/aurora-site/learning/reading-content.jsx (3)
src/pages/aurora-site/learning/listening-content.jsx (5)
STORAGE_KEY(35-35)getInitialProgress(37-46)stored(38-38)progressList(49-49)handleComplete(57-70)src/pages/aurora-site/grammar-content.jsx (5)
STORAGE_KEY(50-50)getInitialProgress(52-61)stored(53-53)progressList(64-64)handleComplete(72-85)src/pages/aurora-site/learning/speaking-content.jsx (5)
STORAGE_KEY(50-50)getInitialProgress(52-61)stored(53-53)progressList(64-64)handleComplete(72-85)
src/pages/aurora-site/grammar-content.jsx (3)
src/pages/aurora-site/learning/reading-content.jsx (5)
STORAGE_KEY(40-40)getInitialProgress(42-51)stored(43-43)progressList(54-54)handleComplete(62-74)src/pages/aurora-site/learning/listening-content.jsx (5)
STORAGE_KEY(35-35)getInitialProgress(37-46)stored(38-38)progressList(49-49)handleComplete(57-70)src/pages/aurora-site/learning/speaking-content.jsx (5)
STORAGE_KEY(50-50)getInitialProgress(52-61)stored(53-53)progressList(64-64)handleComplete(72-85)
🪛 ESLint
src/pages/aurora-site/learning/listening-content.jsx
[error] 1-1: 'React' is defined but never used.
(no-unused-vars)
src/pages/aurora-site/learning/speaking-content.jsx
[error] 1-1: 'React' is defined but never used.
(no-unused-vars)
src/pages/aurora-site/learning/reading-content.jsx
[error] 1-1: 'React' is defined but never used.
(no-unused-vars)
src/pages/aurora-site/grammar-content.jsx
[error] 1-1: 'React' is defined but never used.
(no-unused-vars)
🔇 Additional comments (4)
src/pages/aurora-site/learning/listening-content.jsx (1)
72-156: LGTM on UI/logic integration.Sequential unlock visuals, lock icon messaging, and progress bar behavior align with the PR objectives and work cohesively with the persistence layer (post fixes above).
src/pages/aurora-site/learning/reading-content.jsx (1)
76-162: Overall: meets the DoD post-SSR fix.Sequential unlocking, persistence, and UI messaging meet the linked issue’s requirements.
src/pages/aurora-site/learning/speaking-content.jsx (1)
87-174: Looks good after the SSR/storage fixes.The sequential unlock logic and visuals for Speaking align with the issue requirements.
src/pages/aurora-site/grammar-content.jsx (1)
98-171: Approach aligns with the DoD once SSR/persistence is fixed.The lock visuals, messaging, and sequential enablement meet the feature goals.
|
@benedictfred could you please review and fix the conflicts? |
benedictfred
force-pushed
the
feat/lesson-lock
branch
from
6f8af9e to
fb9e9ed
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements locked lesson progression logic across four learning modules (grammar, speaking, listening, reading) to enforce sequential completion of lessons. The feature includes visual indicators for locked/unlocked states, progress tracking, and local storage persistence.
- Added sequential lesson unlocking where completing one lesson unlocks the next
- Implemented local storage to persist progress across browser sessions
- Added "Mark Complete" buttons for demo purposes to simulate lesson completion
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/aurora-site/learning/speaking-content.jsx | Implements progress tracking state management and sequential unlocking for speaking lessons |
| src/pages/aurora-site/learning/reading-content.jsx | Adds locked lesson progression with completion simulation for reading topics |
| src/pages/aurora-site/learning/listening-content.jsx | Integrates progress state and unlocking logic for listening exercises |
| src/pages/aurora-site/grammar-content.jsx | Implements lesson progression system for grammar topics with completion tracking |
| src/pages/aurora-site/assessment/pronunciation-assessment.jsx | Updates import paths to correct component locations |
| src/components/games/word-matching/game-board.jsx | Removes unnecessary empty lines |
| src/components/assessment/AudioAssessment.jsx | Fixes import path case sensitivity and formatting improvements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return grammarTopics.map((topic, index) => ({ | ||
| id: topic.id, | ||
| progress: index === 0 ? 0 : 0, | ||
| unlocked: index === 0, | ||
| })); |
There was a problem hiding this comment.
The ternary operator index === 0 ? 0 : 0 always returns 0. This can be simplified to just progress: 0.
| return grammarTopics.map((topic, index) => ({ | |
| id: topic.id, | |
| progress: index === 0 ? 0 : 0, | |
| unlocked: index === 0, | |
| })); | |
| return grammarTopics.map((topic, index) => ({ | |
| id: topic.id, | |
| progress: 0, | |
| unlocked: index === 0, | |
| })); |
| {item.progress}% | ||
| </span> | ||
| {/* Only show navigation if unlocked */} | ||
| {topic.unlocked && topic.id === "past-simple" && ( |
There was a problem hiding this comment.
The condition topic.unlocked refers to the original topic data instead of the current progress state. This should be item.unlocked to check the actual unlocked status from the progress state.
|
@ManuelJG1999 |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
src/pages/aurora-site/grammar-content.jsx (1)
54-63: Harden getInitialProgress: SSR guard, parse safety, and id-based merge.Direct localStorage access breaks on SSR; JSON.parse can throw; stored shape may be stale. Adopt a safe base + merge-by-id and recompute unlocked.
-const getInitialProgress = () => { - const stored = localStorage.getItem(STORAGE_KEY); - if (stored) return JSON.parse(stored); - // Default: first lesson unlocked, rest locked, all progress 0 - return grammarTopics.map((topic, index) => ({ - id: topic.id, - progress: index === 0 ? 0 : 0, - unlocked: index === 0, - })); -}; +const getInitialProgress = () => { + const base = grammarTopics.map((t, i) => ({ + id: t.id, + progress: 0, + unlocked: i === 0, + })); + if (typeof window === "undefined") return base; + try { + const raw = window.localStorage.getItem(STORAGE_KEY); + const parsed = raw ? JSON.parse(raw) : null; + if (!Array.isArray(parsed)) return base; + const byId = new Map(parsed.map((p) => [p.id, p])); + const merged = base.map((t) => ({ + id: t.id, + progress: Number.isFinite(byId.get(t.id)?.progress) + ? Math.max(0, Math.min(100, byId.get(t.id).progress)) + : 0, + unlocked: false, + })); + return merged.map((item, i, arr) => ({ + ...item, + unlocked: i === 0 ? true : arr[i - 1].progress === 100, + })); + } catch (e) { + console.warn("Invalid grammar progress in localStorage, resetting.", e); + return base; + } +};src/pages/aurora-site/learning/speaking-content.jsx (1)
86-95: Harden getInitialProgress: SSR guard + parse/shape validation.Same issue as Grammar/Reading: direct localStorage read and unvalidated parse.
-const getInitialProgress = () => { - const stored = localStorage.getItem(STORAGE_KEY); - if (stored) return JSON.parse(stored); - // Default: first lesson unlocked, rest locked, all progress 0 - return speakingTopics.map((t, i) => ({ - id: t.id, - progress: 0, - unlocked: i === 0, - })); -}; +const getInitialProgress = () => { + const base = speakingTopics.map((t, i) => ({ + id: t.id, + progress: 0, + unlocked: i === 0, + })); + if (typeof window === "undefined") return base; + try { + const raw = window.localStorage.getItem(STORAGE_KEY); + const parsed = raw ? JSON.parse(raw) : null; + if (!Array.isArray(parsed)) return base; + const byId = new Map(parsed.map((p) => [p.id, p])); + const merged = base.map((t) => ({ + id: t.id, + progress: Number.isFinite(byId.get(t.id)?.progress) + ? Math.max(0, Math.min(100, byId.get(t.id).progress)) + : 0, + unlocked: false, + })); + return merged.map((item, i, arr) => ({ + ...item, + unlocked: i === 0 ? true : arr[i - 1].progress === 100, + })); + } catch (e) { + console.warn("Invalid speaking progress in localStorage, resetting.", e); + return base; + } +};
🧹 Nitpick comments (11)
src/pages/aurora-site/assessment/pronunciation-assessment.jsx (1)
78-80: Guard against divide-by-zero when maxScore is 0.If the question set ever contains no auto-gradable items, percentage becomes NaN.
Apply this one-liner near the percentage computation:
const percentage = maxScore > 0 ? Math.round((score / maxScore) * 100) : 0;src/components/assessment/AudioAssessment.jsx (4)
95-98: Prefer inline error UI over alert() for mic failures.Keeps UX consistent with the existing error banner.
- console.error("Error starting recording:", error); - alert( - "Could not access microphone. Please ensure microphone permissions are granted." - ); + console.error("Error starting recording:", error); + setAudioError("Could not access microphone. Please ensure microphone permissions are granted.");
1-1: Add useEffect to clean up object URLs (prevent memory leaks).You create object URLs for recordings but never revoke them.
-import { useState, useRef, useCallback } from "react"; +import { useState, useRef, useCallback, useEffect } from "react";Add the cleanup effect (outside changed ranges):
// Revoke previous object URL when replaced/unmounted useEffect(() => { return () => { if (recordedAudio) URL.revokeObjectURL(recordedAudio); }; }, [recordedAudio]);
75-107: Stop microphone tracks after recording to release hardware.Tracks stay live after
MediaRecorder.stop(). Store the stream and stop tracks on end.Add (outside changed ranges):
const streamRef = useRef(null);Patch in start/stop (illustrative):
// in startRecording() const stream = await navigator.mediaDevices.getUserMedia({ audio: true }); streamRef.current = stream; mediaRecorderRef.current = new MediaRecorder(stream); // in stopRecording() if (mediaRecorderRef.current && isRecording) { mediaRecorderRef.current.stop(); if (streamRef.current) { streamRef.current.getTracks().forEach(t => t.stop()); streamRef.current = null; } setIsRecording(false); }
144-146: Minor a11y: add aria-labels to the audio buttons.Improves screen reader UX without changing visuals.
Example (outside changed ranges):
<Button aria-label={isPlaying && selectedAudio === source ? `Pause audio ${String.fromCharCode(65 + index)}` : `Play audio ${String.fromCharCode(65 + index)}`} ... >src/pages/aurora-site/grammar-content.jsx (2)
101-109: Add accessibility hint for locked cards.Provide an aria-disabled state so screen readers understand it’s locked.
- className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${!item.unlocked ? "opacity-60 pointer-events-none" : ""}`} + className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${!item.unlocked ? "opacity-60 pointer-events-none" : ""}`} + aria-disabled={!item.unlocked}
68-71: Optional: debounce persistence to reduce localStorage churn.Not critical, but frequent writes can be noisy; a small debounce helps.
-useEffect(() => { - localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList)); -}, [progressList]); +useEffect(() => { + const id = setTimeout(() => { + window.localStorage?.setItem(STORAGE_KEY, JSON.stringify(progressList)); + }, 150); + return () => clearTimeout(id); +}, [progressList]);src/pages/aurora-site/learning/speaking-content.jsx (4)
216-221: Prefer id-based lookup over index fallback.Index coupling can break if topics reorder. Minor for MVP, but consider mapping progress by id.
-const item = progressList[idx] || { progress: 0, unlocked: idx === 0 }; +const byId = useMemo(() => new Map(progressList.map((p) => [p.id, p])), [progressList]); +const item = byId.get(topic.id) ?? { progress: 0, unlocked: idx === 0 };
223-227: Add aria-disabled on locked cards for a11y.- className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${!item.unlocked ? "opacity-60 pointer-events-none" : ""}`} + className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${!item.unlocked ? "opacity-60 pointer-events-none" : ""}`} + aria-disabled={!item.unlocked}
276-282: Button nit: add type and accessible label.Prevents implicit submit in forms and improves SR clarity.
-<button +<button + type="button" + aria-label={`Mark "${topic.title}" complete`} className="absolute top-4 right-4 text-xs bg-light-blue-1 text-white px-2 py-1 rounded" onClick={() => handleComplete(idx)} >
11-55: Optional: unify topic metadata shape across modules.Speaking topics still embed progress/unlocked in metadata, unlike the intended separation. Consider normalizing to id/title only (same reasoning as Grammar).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/components/assessment/AudioAssessment.jsx(9 hunks)src/components/games/word-matching/game-board.jsx(0 hunks)src/pages/aurora-site/assessment/pronunciation-assessment.jsx(5 hunks)src/pages/aurora-site/grammar-content.jsx(2 hunks)src/pages/aurora-site/learning/listening-content.jsx(1 hunks)src/pages/aurora-site/learning/reading-content.jsx(1 hunks)src/pages/aurora-site/learning/speaking-content.jsx(2 hunks)
💤 Files with no reviewable changes (1)
- src/components/games/word-matching/game-board.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/aurora-site/learning/reading-content.jsx
- src/pages/aurora-site/learning/listening-content.jsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/pages/aurora-site/assessment/pronunciation-assessment.jsx (1)
src/pages/aurora-site/assessment/listening-comprehension.jsx (2)
mockAudioPaths(6-16)results(85-85)
src/pages/aurora-site/grammar-content.jsx (2)
src/pages/aurora-site/learning/reading-content.jsx (5)
STORAGE_KEY(40-40)getInitialProgress(42-51)stored(43-43)progressList(54-54)handleComplete(62-74)src/pages/aurora-site/learning/speaking-content.jsx (5)
STORAGE_KEY(84-84)getInitialProgress(86-95)stored(87-87)progressList(98-98)handleComplete(106-119)
src/pages/aurora-site/learning/speaking-content.jsx (2)
src/pages/aurora-site/grammar-content.jsx (5)
STORAGE_KEY(52-52)getInitialProgress(54-63)stored(55-55)progressList(66-66)handleComplete(74-87)src/pages/aurora-site/learning/reading-content.jsx (5)
STORAGE_KEY(40-40)getInitialProgress(42-51)stored(43-43)progressList(54-54)handleComplete(62-74)
🔇 Additional comments (2)
src/pages/aurora-site/assessment/pronunciation-assessment.jsx (2)
1-3: Imports/path consolidation looks good.Paths and double-quote normalization are consistent with the repo’s component locations.
8-8: Verify audio asset presence and filename for veryVsBerryNo local MP3 was found under
public/audio,src/assets/audio, or anywhere insrc. Ensure that/audio/very.mp3exists in the deployed assets (or rename it to match the intended pair clip, e.g.very-vs-berry.mp3) to prevent runtime 404s.
| audioSources?.forEach((audioUrl) => { | ||
| useAudioQuality(audioUrl, handleQualityCheck(audioUrl)); | ||
| }); |
There was a problem hiding this comment.
Fix Rules of Hooks violation: don’t call hooks in loops.
Calling useAudioQuality inside forEach breaks the Rules of Hooks and will be flagged/buggy.
Apply:
- audioSources?.forEach((audioUrl) => {
- useAudioQuality(audioUrl, handleQualityCheck(audioUrl));
- });
+ // Process only the currently selected source to respect Rules of Hooks
+ useAudioQuality(
+ selectedAudio,
+ selectedAudio ? handleQualityCheck(selectedAudio) : () => {}
+ );📝 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.
| audioSources?.forEach((audioUrl) => { | |
| useAudioQuality(audioUrl, handleQualityCheck(audioUrl)); | |
| }); | |
| // Process only the currently selected source to respect Rules of Hooks | |
| useAudioQuality( | |
| selectedAudio, | |
| selectedAudio ? handleQualityCheck(selectedAudio) : () => {} | |
| ); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 41-41: This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 ESLint
[error] 41-41: React Hook "useAudioQuality" cannot be called inside a callback. React Hooks must be called in a React function component or a custom React Hook function.
(react-hooks/rules-of-hooks)
|
|
||
| mediaRecorderRef.current.onstop = () => { | ||
| const audioBlob = new Blob(chunksRef.current, { type: 'audio/wav' }); | ||
| const audioBlob = new Blob(chunksRef.current, { type: "audio/wav" }); |
There was a problem hiding this comment.
Recorded blob MIME type is mislabeled (can corrupt playback).
MediaRecorder typically produces webm/ogg. Forcing audio/wav without transcoding is incorrect.
- const audioBlob = new Blob(chunksRef.current, { type: "audio/wav" });
+ const mimeType =
+ mediaRecorderRef.current?.mimeType ||
+ (chunksRef.current[0] && chunksRef.current[0].type) ||
+ "audio/webm";
+ const audioBlob = new Blob(chunksRef.current, { type: mimeType });📝 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 audioBlob = new Blob(chunksRef.current, { type: "audio/wav" }); | |
| // Derive the true recording MIME type instead of forcing "audio/wav" | |
| const mimeType = | |
| mediaRecorderRef.current?.mimeType || | |
| (chunksRef.current[0] && chunksRef.current[0].type) || | |
| "audio/webm"; | |
| const audioBlob = new Blob(chunksRef.current, { type: mimeType }); |
🤖 Prompt for AI Agents
In src/components/assessment/AudioAssessment.jsx around line 86, the code
creates a Blob using a forced MIME type "audio/wav" which is incorrect for
MediaRecorder output and can corrupt playback; replace the hardcoded type by
using the actual recorder output type (e.g. recorder.mimeType if available) or
derive it from the first chunk (chunksRef.current[0].type) and pass that into
the Blob constructor, or omit the type entirely so the browser preserves the
original MIME; if WAV is required downstream, transcode the recorded data to WAV
instead of relabelling it.
| const getInitialProgress = () => { | ||
| const stored = localStorage.getItem(STORAGE_KEY); | ||
| if (stored) return JSON.parse(stored); | ||
| // Default: first lesson unlocked, rest locked, all progress 0 | ||
| return grammarTopics.map((topic, index) => ({ | ||
| id: topic.id, | ||
| progress: index === 0 ? 0 : 0, | ||
| unlocked: index === 0, | ||
| })); | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Repo-wide check for similar patterns.
Other lesson pages show the same initializer and double-setState. Run this to find and fix consistently.
🏁 Script executed:
#!/bin/bash
# Find unsafe localStorage reads and topic.unlocked gating
rg -nP --type=js --type=jsx -C2 'localStorage\.getItem\('
rg -nP --type=js --type=jsx -C2 '\btopic\.unlocked\b'
# Find handleComplete with multiple setState calls
rg -nP --type=jsx -C3 'const handleComplete.*?\n(?:.|\n)*?setProgressList\('Length of output: 278
The previous patterns failed due to unrecognized file types—these commands use glob filters instead.
🏁 Script executed:
#!/bin/bash
# Re-run searches without relying on ripgrep file-type aliases
# 1. Find all localStorage.getItem usages in .js/.jsx
rg -n 'localStorage\.getItem\(' -g '*.js' -g '*.jsx'
# 2. Find all occurrences of topic.unlocked in .js/.jsx
rg -n '\btopic\.unlocked\b' -g '*.js' -g '*.jsx'
# 3. Locate handleComplete definitions and check for multiple setProgressList calls
rg -n 'const handleComplete' -g '*.js' -g '*.jsx' -C3 | rg 'setProgressList\('Length of output: 2120
🏁 Script executed:
#!/bin/bash
# Show handleComplete implementation to verify multiple setProgressList calls
rg -n -C5 'const handleComplete' -g 'src/pages/aurora-site/grammar-content.jsx'Length of output: 949
Extract shared progress logic into a utility or hook
The getInitialProgress initializer is duplicated across grammar-content.jsx, reading-content.jsx, speaking-content.jsx and listening-content.jsx. Consolidate this (and any similar handleComplete logic) into a shared function or custom hook to avoid repetition.
| const STORAGE_KEY = "speakingProgress"; | ||
|
|
||
| const getInitialProgress = () => { | ||
| const stored = localStorage.getItem(STORAGE_KEY); | ||
| if (stored) return JSON.parse(stored); | ||
| // Default: first lesson unlocked, rest locked, all progress 0 | ||
| return speakingTopics.map((t, i) => ({ | ||
| id: t.id, | ||
| progress: 0, | ||
| unlocked: i === 0, | ||
| })); | ||
| }; | ||
|
|
||
| const SpeakingPage = () => { | ||
| const speakingTopics = [ | ||
| { | ||
| id: "basic-pronunciation", | ||
| title: "Basic Pronunciation", | ||
| progress: 100, | ||
| unlocked: true, | ||
| }, | ||
| { | ||
| id: "everyday-phrases", | ||
| title: "Everyday Phrases", | ||
| progress: 85, | ||
| unlocked: true, | ||
| }, | ||
| { | ||
| id: "conversation-skills", | ||
| title: "Conversation Skills", | ||
| progress: 60, | ||
| unlocked: true, | ||
| }, | ||
| { | ||
| id: "fluency-building", | ||
| title: "Fluency Building", | ||
| progress: 40, | ||
| unlocked: true, | ||
| }, | ||
| { | ||
| id: "public-speaking", | ||
| title: "Public Speaking", | ||
| progress: 0, | ||
| unlocked: false, | ||
| }, | ||
| { | ||
| id: "debate-discussion", | ||
| title: "Debate & Discussion", | ||
| progress: 0, | ||
| unlocked: false, | ||
| }, | ||
| { | ||
| id: "presentations", | ||
| title: "Presentations", | ||
| progress: 0, | ||
| unlocked: false, | ||
| }, | ||
| { | ||
| id: "accent-reduction", | ||
| title: "Accent Reduction", | ||
| progress: 0, | ||
| unlocked: false, | ||
| }, | ||
| ]; | ||
| const [progressList, setProgressList] = useState(getInitialProgress()); | ||
|
|
||
| // Persist progress to localStorage | ||
| useEffect(() => { | ||
| localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList)); | ||
| }, [progressList]); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Repo-wide verification to align pages.
Ensure Listening/Reading also get the SSR-safe initializer and single setState.
🏁 Script executed:
#!/bin/bash
# Check all learning pages for direct localStorage access and duplicate setState
fd -t f '.jsx$' src/pages/aurora-site | rg -n 'localStorage\.getItem\(' -n
rg -nP --type=jsx -C3 'const handleComplete\W' src/pages/aurora-site | rg -n 'setProgressList\(' -nLength of output: 206
🏁 Script executed:
#!/bin/bash
# List all .jsx files under the learning pages
fd --extension jsx src/pages/aurora-site/learning
# Check for direct localStorage access in each
rg -n 'localStorage\.getItem' src/pages/aurora-site/learning
rg -n 'localStorage\.setItem' src/pages/aurora-site/learning
# Locate state initialization patterns
rg -n 'useState' src/pages/aurora-site/learning
rg -n 'getInitialProgress' -C3 src/pages/aurora-site/learning
# Inspect progress update handlers for multiple setState calls
rg -n 'setProgressList' -C3 src/pages/aurora-site/learningLength of output: 11853
Consolidate duplicate setProgressList calls into a single state update
- In
reading-content.jsx(≈ lines 62–70), merge the twosetProgressListcalls inhandleCompleteinto one updater that applies both the progress and unlocking logic. - Apply the same refactor in
listening-content.jsx(≈ lines 57–64) andspeaking-content.jsx(≈ lines 106–113). - No changes needed for SSR‐safe initializer—
getInitialProgressis already passed as the lazy initializer.
🤖 Prompt for AI Agents
In src/pages/aurora-site/learning/speaking-content.jsx around lines 84 to 104,
there are duplicate setProgressList calls in the completion handler; replace
them with a single functional state update: call setProgressList(prev => { const
next = prev.map(item => item.id === targetId ? { ...item, progress: newProgress
} : item); if (newProgress >= 100) unlock the next topic by index in that same
map; return next; }); use the functional updater to avoid stale state and apply
both progress change and unlocking logic in one setProgressList call.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/pages/aurora-site/learning/reading-content.jsx (1)
42-51: Fix SSR crash, validate JSON, and reconcile by id (same issue previously flagged).Accessing localStorage during render will crash under SSR; also JSON is unvalidated and index-based reconciliation is brittle.
Apply:
-const getInitialProgress = () => { - const stored = localStorage.getItem(STORAGE_KEY); - if (stored) return JSON.parse(stored); - // Default: first lesson unlocked, rest locked, all progress 0 - return readingTopics.map((t, i) => ({ - id: t.id, - progress: 0, - unlocked: i === 0, - })); -}; +const getInitialProgress = () => { + const base = readingTopics.map((t) => ({ id: t.id, progress: 0 })); + if (typeof window === "undefined") { + return base.map((t, i) => ({ ...t, unlocked: i === 0 })); + } + try { + const stored = window.localStorage.getItem(STORAGE_KEY); + const parsed = stored ? JSON.parse(stored) : null; + const byId = Array.isArray(parsed) + ? new Map(parsed.map((p) => [p.id, p])) + : new Map(); + + const merged = base.map((t) => { + const s = byId.get(t.id); + const progress = Number.isFinite(s?.progress) + ? Math.min(100, Math.max(0, Number(s.progress))) + : 0; + return { id: t.id, progress }; + }); + return merged.map((item, i, arr) => ({ + ...item, + unlocked: i === 0 ? true : arr[i - 1].progress === 100, + })); + } catch (e) { + console.warn("Invalid reading progress in localStorage, resetting.", e); + return base.map((t, i) => ({ ...t, unlocked: i === 0 })); + } +};
🧹 Nitpick comments (5)
src/pages/aurora-site/learning/reading-content.jsx (5)
54-54: Use lazy initializer for useState.Avoids evaluating initialization logic on re-renders.
-const [progressList, setProgressList] = useState(getInitialProgress()); +const [progressList, setProgressList] = useState(() => getInitialProgress());
5-38: Remove progress/unlocked from readingTopics to avoid duplicating source of truth.These fields are ignored after you introduced state; keeping them is misleading.
- { - id: "vocabulary-building", - title: "Vocabulary Building", - progress: 100, - unlocked: true, - }, - { id: "short-texts", title: "Short Texts", progress: 95, unlocked: true }, - { - id: "comprehension", - title: "Reading Comprehension", - progress: 70, - unlocked: true, - }, - { id: "articles", title: "Articles & Essays", progress: 45, unlocked: true }, - { id: "literature", title: "Literature", progress: 15, unlocked: true }, - { - id: "technical-reading", - title: "Technical Reading", - progress: 0, - unlocked: false, - }, - { - id: "critical-analysis", - title: "Critical Analysis", - progress: 0, - unlocked: false, - }, - { - id: "research-papers", - title: "Research Papers", - progress: 0, - unlocked: false, - }, + { id: "vocabulary-building", title: "Vocabulary Building" }, + { id: "short-texts", title: "Short Texts" }, + { id: "comprehension", title: "Reading Comprehension" }, + { id: "articles", title: "Articles & Essays" }, + { id: "literature", title: "Literature" }, + { id: "technical-reading", title: "Technical Reading" }, + { id: "critical-analysis", title: "Critical Analysis" }, + { id: "research-papers", title: "Research Papers" },
92-97: Improve basic a11y: mark locked cards as disabled; hide decorative icons from SR.Adds aria-disabled on locked cards and aria-hidden on icons.
- <div - key={topic.id} + <div + key={topic.id} + aria-disabled={!item.unlocked} className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${ !item.unlocked ? "opacity-60 pointer-events-none" : "" }`} > @@ - <BookOpen className="w-5 h-5 text-green-1" /> + <BookOpen aria-hidden="true" className="w-5 h-5 text-green-1" /> @@ - <CheckCircle + <CheckCircle + aria-hidden="true" className={`w-5 h-5 ${ item.progress === 100 ? "text-green" : "text-green-1" }`} /> @@ - <Lock className="w-5 h-5 text-neutral-4" /> + <Lock aria-hidden="true" className="w-5 h-5 text-neutral-4" />Also applies to: 101-102, 113-118, 121-125
56-60: Optional: guard and try/catch on write to localStorage.Prevents noisy errors on quota/blocked storage.
- useEffect(() => { - localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList)); - }, [progressList]); + useEffect(() => { + try { + if (typeof window !== "undefined") { + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList)); + } + } catch {} + }, [progressList]);
1-3: Consider extracting a reusable hook (useLessonProgress) shared by grammar/listening/speaking/reading.Centralizes id-based reconciliation, SSR guards, and unlock logic; reduces duplication across pages.
Also applies to: 75-161
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/pages/aurora-site/grammar-content.jsx(2 hunks)src/pages/aurora-site/learning/listening-content.jsx(1 hunks)src/pages/aurora-site/learning/reading-content.jsx(1 hunks)src/pages/aurora-site/learning/speaking-content.jsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/pages/aurora-site/learning/speaking-content.jsx
- src/pages/aurora-site/learning/listening-content.jsx
- src/pages/aurora-site/grammar-content.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/aurora-site/learning/reading-content.jsx (2)
src/pages/aurora-site/grammar-content.jsx (5)
STORAGE_KEY(52-52)getInitialProgress(54-63)stored(55-55)progressList(66-66)handleComplete(74-84)src/pages/aurora-site/learning/speaking-content.jsx (5)
STORAGE_KEY(84-84)getInitialProgress(86-95)stored(87-87)progressList(98-98)handleComplete(106-117)
| // Simulate marking a lesson complete (for demo) | ||
| const handleComplete = (idx) => { | ||
| setProgressList((prev) => { | ||
| const updated = prev.map((item, i) => | ||
| i === idx ? { ...item, progress: 100 } : item | ||
| ); | ||
|
|
||
| <div className="space-y-3"> | ||
| {readingTopics.map((topic) => ( | ||
| <div key={topic.id} className="bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937]"> | ||
| <div className="flex items-center justify-between mb-2"> | ||
| <div className="flex items-center gap-3"> | ||
| <div className="w-8 h-8 rounded-md bg-dark-blue-4 flex items-center justify-center"> | ||
| <BookOpen className="w-5 h-5 text-green-1" /> | ||
| </div> | ||
| <span className="text-neutral-1 font-medium">{topic.title}</span> | ||
| </div> | ||
| {topic.unlocked ? ( | ||
| <div className={`w-8 h-8 rounded-full flex items-center justify-center ${ | ||
| topic.progress === 100 | ||
| ? "bg-green/20" | ||
| : "bg-green-1/20" | ||
| }`}> | ||
| <CheckCircle className={`w-5 h-5 ${topic.progress === 100 ? 'text-green' : 'text-green-1'}`} /> | ||
| </div> | ||
| ) : ( | ||
| <Lock className="w-5 h-5 text-neutral-4" /> | ||
| )} | ||
| </div> | ||
| <div className="h-2 w-full bg-neutral-1/10 rounded-full overflow-hidden"> | ||
| <div | ||
| className={`h-full ${ | ||
| topic.progress === 100 | ||
| ? "bg-green" | ||
| : topic.progress > 0 | ||
| ? "bg-green-1" | ||
| : "" | ||
| }`} | ||
| style={{ width: `${topic.progress}%` }} | ||
| /> | ||
| </div> | ||
| <div className="text-right mt-1"> | ||
| <span className="text-xs text-neutral-2">{topic.progress}%</span> | ||
| </div> | ||
| </div> | ||
| ))} | ||
| return updated.map((item, i, arr) => ({ | ||
| ...item, | ||
| unlocked: i === 0 ? true : arr[i - 1].progress === 100, | ||
| })); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Switch completion updates from index-based to id-based to prevent mismatches after content changes.
Index coupling can update the wrong lesson when topics are re-ordered/inserted.
-const handleComplete = (idx) => {
- setProgressList((prev) => {
- const updated = prev.map((item, i) =>
- i === idx ? { ...item, progress: 100 } : item
- );
-
- return updated.map((item, i, arr) => ({
- ...item,
- unlocked: i === 0 ? true : arr[i - 1].progress === 100,
- }));
- });
-};
+const handleComplete = (topicId) => {
+ setProgressList((prev) => {
+ const byId = new Map(prev.map((p) => [p.id, p]));
+ byId.set(topicId, {
+ ...(byId.get(topicId) ?? { id: topicId, progress: 0, unlocked: false }),
+ progress: 100,
+ });
+ const rebuilt = readingTopics.map((t) => ({
+ id: t.id,
+ progress: byId.get(t.id)?.progress ?? 0,
+ unlocked: false,
+ }));
+ return rebuilt.map((item, i, arr) => ({
+ ...item,
+ unlocked: i === 0 ? true : arr[i - 1].progress === 100,
+ }));
+ });
+};- onClick={() => handleComplete(idx)}
+ onClick={() => handleComplete(topic.id)}Also applies to: 147-151
🤖 Prompt for AI Agents
In src/pages/aurora-site/learning/reading-content.jsx around lines 61-73 (and
also apply the same change at lines 147-151), the completion handler currently
uses an index param which can mismatch when items are reordered/inserted; change
the handler to accept a lesson id instead, update state by mapping items and
setting progress where item.id === passedId, then compute unlocked flags by
locating each item's current index (e.g., via findIndex) and setting unlocked to
true for index 0 or when the previous item's progress === 100; update any
callers to pass the lesson id rather than the index.
| return ( | ||
| <div className="min-h-screen bg-[#111827] text-neutral-1 p-6"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Derive UI state by id and previous progress; stop relying on array position.
Prevents stale/incorrect lock states when list changes or storage is partial.
- return (
+ const progressById = new Map(progressList.map((p) => [p.id, p.progress]));
+ return (
@@
- {readingTopics.map((topic, idx) => {
- const item = progressList[idx] || {
- progress: 0,
- unlocked: idx === 0,
- };
+ {readingTopics.map((topic, idx) => {
+ const prevId = readingTopics[idx - 1]?.id;
+ const prevProgress = idx === 0 ? 100 : (progressById.get(prevId) ?? 0);
+ const item = {
+ progress: progressById.get(topic.id) ?? 0,
+ unlocked: idx === 0 ? true : prevProgress === 100,
+ };Also applies to: 85-91
🤖 Prompt for AI Agents
In src/pages/aurora-site/learning/reading-content.jsx around lines 75-76 (and
also update logic at 85-91), stop deriving lock/UI state from item array
positions; instead compute state by content ID and any previous progress data.
Build a lookup (map) from savedProgress entries keyed by contentId (handle
missing/partial storage), then for each content item find its progress by id
(not index), derive locked/unlocked/completion from that found entry and prior
state, and update setState calls to use the id-derived result so UI remains
correct when list order or partial storage changes.
| item.progress === 100 ? "bg-green/20" : "bg-green-1/20" | ||
| }`} | ||
| > | ||
| <CheckCircle | ||
| className={`w-5 h-5 ${ | ||
| item.progress === 100 ? "text-green" : "text-green-1" | ||
| }`} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Tailwind tokens exist ("text-green" vs "text-green-1", "bg-green" vs "bg-green-1").
Mixed token families may not both be defined in your theme.
Run:
🏁 Script executed:
#!/bin/bash
# Inspect Tailwind config and scan usage of green tokens
fd -HI 'tailwind.config.*' -0 | xargs -0 -I{} sh -c 'echo "== {} =="; sed -n "1,200p" "{}"'
rg -n --no-heading -C1 'text-green(\b|-)|bg-green(\b|-)' srcLength of output: 45558
Fix undefined ‘green-1’ tokens in src/pages/aurora-site/learning/reading-content.jsx (lines 110–116). Your Tailwind config only defines a single green color, so bg-green-1/20 and text-green-1 won’t be generated—either switch to bg-green/20/text-green or add a nested 1 shade under green in tailwind.config.js.
🤖 Prompt for AI Agents
In src/pages/aurora-site/learning/reading-content.jsx around lines 110 to 116,
the component uses non-existent Tailwind tokens `bg-green-1/20` and
`text-green-1`; update these to the defined tokens `bg-green/20` and
`text-green` (replace both occurrences) OR alternatively add a `1` shade under
`green` in tailwind.config.js (e.g., green: { DEFAULT: '<hex>', 1: '<hex>' })
and rebuild so the `green-1` utilities are generated; choose one consistent
approach and ensure class names match the tailwind config.
|
@benedictfred please fix the conflict. |
benedictfred
force-pushed
the
feat/lesson-lock
branch
from
4fddb9f to
15e51b5
Compare
|
@ManuelJG1999 |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/pages/aurora-site/learning/listening-content.jsx (1)
38-47: Fix SSR crash and persist only minimal state.Accessing localStorage during render will throw on SSR and tests; also persisting derived
unlockedcauses stale states. Guard forwindow, validate/parse safely, reconcile by id, and recomputeunlocked. Persist only{id, progress}.-const getInitialProgress = () => { - const stored = localStorage.getItem(STORAGE_KEY); - if (stored) return JSON.parse(stored); - // Default: first lesson unlocked, rest locked, all progress 0 - return listeningTopics.map((topic, index) => ({ - id: topic.id, - progress: 0, - unlocked: index === 0, - })); -}; +const getInitialProgress = () => { + const base = listeningTopics.map((t, i) => ({ + id: t.id, + progress: 0, + // derived below; first is always unlocked + unlocked: i === 0, + })); + + if (typeof window === "undefined") return base; + + try { + const raw = window.localStorage.getItem(STORAGE_KEY); + const parsed = raw ? JSON.parse(raw) : null; + const byId = new Map( + Array.isArray(parsed) ? parsed.map((p) => [p.id, p]) : [] + ); + const next = base.map((t) => ({ + id: t.id, + progress: byId.get(t.id)?.progress ?? 0, + unlocked: false, + })); + return next.map((item, i, arr) => ({ + ...item, + unlocked: i === 0 ? true : arr[i - 1].progress === 100, + })); + } catch (e) { + console.warn("Invalid listening progress in localStorage, resetting.", e); + return base; + } +};// Persist progress to localStorage useEffect(() => { - localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList)); + if (typeof window === "undefined") return; + const minimal = progressList.map(({ id, progress }) => ({ id, progress })); + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(minimal)); }, [progressList]);Also applies to: 52-55
src/pages/aurora-site/learning/speaking-content.jsx (1)
86-95: SSR-safe initializer + robust storage merge/validationGuard against SSR, validate/merge by id, and recompute
unlockedfrom progress to avoid stale/malformed localStorage state. This mirrors the earlier bot suggestion and prevents crashes/data drift.-const getInitialProgress = () => { - const stored = localStorage.getItem(STORAGE_KEY); - if (stored) return JSON.parse(stored); - // Default: first lesson unlocked, rest locked, all progress 0 - return speakingTopics.map((t, i) => ({ - id: t.id, - progress: 0, - unlocked: i === 0, - })); -}; +const getInitialProgress = () => { + const base = speakingTopics.map((t, i) => ({ + id: t.id, + progress: 0, + unlocked: i === 0, + })); + if (typeof window === "undefined") return base; + try { + const raw = window.localStorage.getItem(STORAGE_KEY); + if (!raw) return base; + const parsed = JSON.parse(raw); + if (!Array.isArray(parsed)) return base; + const byId = new Map( + parsed + .filter((p) => p && typeof p.id === "string" && typeof p.progress === "number") + .map((p) => [p.id, p.progress]) + ); + const merged = base.map((item) => ({ + id: item.id, + progress: byId.get(item.id) ?? 0, + unlocked: false, + })); + return merged.map((item, i, arr) => ({ + ...item, + unlocked: i === 0 ? true : arr[i - 1].progress === 100, + })); + } catch (e) { + console.warn("Invalid speaking progress in localStorage, resetting.", e); + return base; + } +};
🧹 Nitpick comments (9)
src/pages/aurora-site/learning/listening-content.jsx (3)
5-34: Keep topic data pure; remove presentational/derived fields.Store only ids/titles in
listeningTopics. Progress/unlocked belong to state, not static data.const listeningTopics = [ { id: "basic-conversations", - title: "Basic Conversations", - progress: 100, - unlocked: true, + title: "Basic Conversations", }, { id: "everyday-dialogues", - title: "Everyday Dialogues", - progress: 80, - unlocked: true, + title: "Everyday Dialogues", }, - { id: "podcasts", title: "Podcasts", progress: 65, unlocked: true }, + { id: "podcasts", title: "Podcasts" }, { id: "news-broadcasts", - title: "News Broadcasts", - progress: 45, - unlocked: true, + title: "News Broadcasts", }, - { id: "interviews", title: "Interviews", progress: 20, unlocked: true }, - { id: "lectures", title: "Lectures", progress: 0, unlocked: false }, - { id: "movies-tv", title: "Movies & TV", progress: 0, unlocked: false }, + { id: "interviews", title: "Interviews" }, + { id: "lectures", title: "Lectures" }, + { id: "movies-tv", title: "Movies & TV" }, { id: "advanced-listening", - title: "Advanced Listening", - progress: 0, - unlocked: false, + title: "Advanced Listening", }, ];
145-153: Fix token inconsistency: use the same green scale.
bg-green/20andtext-greenlikely don't exist in your palette (you use*-1). Unify togreen-1.- className={`w-8 h-8 rounded-full flex items-center justify-center ${ - item.progress === 100 ? "bg-green/20" : "bg-green-1/20" - }`} + className="w-8 h-8 rounded-full flex items-center justify-center bg-green-1/20" > <CheckCircle - className={`w-5 h-5 ${ - item.progress === 100 ? "text-green" : "text-green-1" - }`} + className="w-5 h-5 text-green-1" />- item.progress === 100 - ? "bg-green" - : item.progress > 0 - ? "bg-green-1" - : "" + item.progress > 0 ? "bg-green-1" : ""Also applies to: 167-171
36-47: Consider extracting a reusable progress hook used by all four pages.Reduce duplication and guarantee consistent behavior across Grammar/Reading/Speaking/Listening.
- Hook shape:
- Input: topics[], storageKey
- Output: progressList, handleComplete, resetProgress
Example (outline):
// useSequentialProgress.ts import { useEffect, useMemo, useState } from "react"; export function useSequentialProgress(topics, storageKey) { const getInitial = /* robust impl as above */; const [progressList, setProgressList] = useState(getInitial); useEffect(() => { /* persist minimal */ }, [progressList]); const byId = useMemo(() => new Map(progressList.map(p => [p.id, p])), [progressList]); const handleComplete = (idx) => { /* same logic */ }; const reset = () => setProgressList(getInitial()); return { progressList, handleComplete, reset, byId }; }Also applies to: 52-68
src/pages/aurora-site/learning/speaking-content.jsx (6)
98-98: Use lazyuseStateinitializerAvoid recomputing
getInitialProgress()on every render and double-invoke in React 18 StrictMode.- const [progressList, setProgressList] = useState(getInitialProgress()); + const [progressList, setProgressList] = useState(getInitialProgress);
100-103: Harden persistence with guardsWrap
localStorage.setItemwithwindowcheck and try/catch to avoid crashes in private mode/quota errors.- useEffect(() => { - localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList)); - }, [progressList]); + useEffect(() => { + try { + if (typeof window !== "undefined") { + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList)); + } + } catch (e) { + console.warn("Failed to persist speaking progress.", e); + } + }, [progressList]);
11-55: Remove unused progress/unlocked fromspeakingTopicsThese fields are ignored (state drives UI). Keeping them risks confusion and drift.
-const speakingTopics = [ - { - id: "basic-pronunciation", - title: "Basic Pronunciation", - progress: 100, - unlocked: true, - }, - { - id: "everyday-phrases", - title: "Everyday Phrases", - progress: 85, - unlocked: true, - }, - { - id: "conversation-skills", - title: "Conversation Skills", - progress: 60, - unlocked: true, - }, - { - id: "fluency-building", - title: "Fluency Building", - progress: 40, - unlocked: true, - }, - { - id: "public-speaking", - title: "Public Speaking", - progress: 0, - unlocked: false, - }, - { - id: "debate-discussion", - title: "Debate & Discussion", - progress: 0, - unlocked: false, - }, - { id: "presentations", title: "Presentations", progress: 0, unlocked: false }, - { - id: "accent-reduction", - title: "Accent Reduction", - progress: 0, - unlocked: false, - }, -]; +const speakingTopics = [ + { id: "basic-pronunciation", title: "Basic Pronunciation" }, + { id: "everyday-phrases", title: "Everyday Phrases" }, + { id: "conversation-skills", title: "Conversation Skills" }, + { id: "fluency-building", title: "Fluency Building" }, + { id: "public-speaking", title: "Public Speaking" }, + { id: "debate-discussion", title: "Debate & Discussion" }, + { id: "presentations", title: "Presentations" }, + { id: "accent-reduction", title: "Accent Reduction" }, +];
106-117: Prefer id-based updates inhandleCompleteKeeps state ops consistent with id-based rendering and future-proof against reordering.
- const handleComplete = (idx) => { - setProgressList((prev) => { - const updated = prev.map((item, i) => - i === idx ? { ...item, progress: 100 } : item - ); + const handleComplete = (id) => { + setProgressList((prev) => { + const updated = prev.map((item) => + item.id === id ? { ...item, progress: 100 } : item + ); return updated.map((item, i, arr) => ({ ...item, unlocked: i === 0 ? true : arr[i - 1].progress === 100, })); }); }; @@ - onClick={() => handleComplete(idx)} + onClick={() => handleComplete(topic.id)}Also applies to: 275-280
151-209: Non-blocking: static “Basic Conversation” blockIf this list will later adopt locking, consider reusing the same progress model to avoid divergent UX.
248-253: Copy tweak (optional)Consider “Complete the previous lesson to unlock” for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/components/assessment/AudioAssessment.jsx(9 hunks)src/components/games/word-matching/game-board.jsx(0 hunks)src/pages/aurora-site/assessment/pronunciation-assessment.jsx(5 hunks)src/pages/aurora-site/grammar-content.jsx(2 hunks)src/pages/aurora-site/learning/listening-content.jsx(1 hunks)src/pages/aurora-site/learning/reading-content.jsx(1 hunks)src/pages/aurora-site/learning/speaking-content.jsx(2 hunks)
💤 Files with no reviewable changes (1)
- src/components/games/word-matching/game-board.jsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/assessment/AudioAssessment.jsx
- src/pages/aurora-site/grammar-content.jsx
- src/pages/aurora-site/learning/reading-content.jsx
- src/pages/aurora-site/assessment/pronunciation-assessment.jsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/aurora-site/learning/listening-content.jsx (3)
src/pages/aurora-site/grammar-content.jsx (5)
STORAGE_KEY(52-52)getInitialProgress(54-63)stored(55-55)progressList(66-66)handleComplete(74-84)src/pages/aurora-site/learning/reading-content.jsx (5)
STORAGE_KEY(40-40)getInitialProgress(42-51)stored(43-43)progressList(54-54)handleComplete(62-73)src/pages/aurora-site/learning/speaking-content.jsx (5)
STORAGE_KEY(84-84)getInitialProgress(86-95)stored(87-87)progressList(98-98)handleComplete(106-117)
src/pages/aurora-site/learning/speaking-content.jsx (2)
src/pages/aurora-site/grammar-content.jsx (5)
STORAGE_KEY(52-52)getInitialProgress(54-63)stored(55-55)progressList(66-66)handleComplete(74-84)src/pages/aurora-site/learning/reading-content.jsx (5)
STORAGE_KEY(40-40)getInitialProgress(42-51)stored(43-43)progressList(54-54)handleComplete(62-73)
🔇 Additional comments (4)
src/pages/aurora-site/learning/listening-content.jsx (2)
58-68: LGTM: sequential unlock logic is correct.Marks current complete and derives
unlockedfrom previous progress; idempotent and pure.
111-117: Link target/listening-courseis defined
The/listening-courseroute is registered insrc/App.jsx, so this CTA will not 404.src/pages/aurora-site/learning/speaking-content.jsx (2)
106-116: Consolidated state update looks goodSingle
setProgressListwith derived unlocks keeps updates atomic and avoids stale reads.
84-95: Drop this recommendation: all three pages (speaking, listening, reading) currently readlocalStoragedirectly—there is no SSR-safe initializer pattern to align to.Likely an incorrect or invalid review comment.
| import { Link } from 'react-router-dom'; | ||
| import { Headphones, CheckCircle, Lock, BookOpen } from "lucide-react"; | ||
| import { Link } from "react-router-dom"; | ||
| import { useEffect, useState } from "react"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reconcile by id, not by index (stable across reorders).
Using indexes can mismatch progress when topics are inserted/reordered. Build an id->state map and consume by id.
-import { useEffect, useState } from "react";
+import { useEffect, useMemo, useState } from "react";- const [progressList, setProgressList] = useState(getInitialProgress());
+ const [progressList, setProgressList] = useState(getInitialProgress());
+ const listById = useMemo(
+ () => new Map(progressList.map((p) => [p.id, p])),
+ [progressList]
+ );- const item = progressList[idx] || {
+ const item = listById.get(topic.id) || {
progress: 0,
unlocked: idx === 0,
};Also applies to: 50-50, 121-127
🤖 Prompt for AI Agents
In src/pages/aurora-site/learning/listening-content.jsx around line 3 (and also
apply to areas around lines 50-50 and 121-127), the component currently
reconciles progress/state using array indexes which breaks when topics are
inserted or reordered; instead create a stable id->state map (e.g., from the
existing progress array) and when rendering or updating items use each item's
unique id to lookup/update its state; populate the map once from current state,
read values by item.id during render, and when updating produce a new
array/state by merging changes keyed by id so order changes won’t misalign
progress.
15e51b5 to
bfbba1d
Compare
|
GM Sire @ManuelJG1999 |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/pages/aurora-site/learning/listening-content.jsx (3)
38-47: Fix SSR/localStorage crash + stale data; deriveunlockedand reconcile by id.Guard
localStoragefor non-browser contexts, validate JSON, rebuild state by topic id, and recomputeunlockedfrom progress (don’t trust persisted derived fields).-const getInitialProgress = () => { - const stored = localStorage.getItem(STORAGE_KEY); - if (stored) return JSON.parse(stored); - // Default: first lesson unlocked, rest locked, all progress 0 - return listeningTopics.map((topic, index) => ({ - id: topic.id, - progress: 0, - unlocked: index === 0, - })); -}; +const getInitialProgress = () => { + const base = listeningTopics.map((t, i) => ({ + id: t.id, + progress: 0, + unlocked: i === 0, + })); + + if (typeof window === "undefined") return base; + + try { + const stored = window.localStorage.getItem(STORAGE_KEY); + const parsed = stored ? JSON.parse(stored) : null; + const byId = Array.isArray(parsed) + ? new Map(parsed.map((p) => [p.id, p])) + : new Map(); + + const next = listeningTopics.map((t) => ({ + id: t.id, + progress: byId.get(t.id)?.progress ?? 0, + unlocked: false, + })); + + return next.map((item, i, arr) => ({ + ...item, + unlocked: i === 0 ? true : arr[i - 1].progress === 100, + })); + } catch (e) { + console.warn("Invalid listening progress in localStorage, resetting.", e); + return base; + } +};
121-127: Render by id, not index (prevents progress misalignment on insert/reorder).- {listeningTopics.map((topic, idx) => { - const item = progressList[idx] || { - progress: 0, - unlocked: idx === 0, - }; + {listeningTopics.map((topic, idx) => { + const item = + listById.get(topic.id) || { + id: topic.id, + progress: 0, + unlocked: idx === 0, + };
3-3: AdduseMemoimport (needed for id-based reconciliation).-import { useEffect, useState } from "react"; +import { useEffect, useMemo, useState } from "react";
🧹 Nitpick comments (5)
src/pages/aurora-site/learning/listening-content.jsx (5)
146-153: Invalid/undefined Tailwind tokens (bg-green,text-green). Use existing design tokens.- className={`w-8 h-8 rounded-full flex items-center justify-center ${ - item.progress === 100 ? "bg-green/20" : "bg-green-1/20" - }`} + className={`w-8 h-8 rounded-full flex items-center justify-center ${ + item.progress === 100 ? "bg-green-1/20" : "bg-green-1/20" + }`} ... - <CheckCircle - className={`w-5 h-5 ${ - item.progress === 100 ? "text-green" : "text-green-1" - }`} - /> + <CheckCircle + className={`w-5 h-5 ${"text-green-1"}`} + />If your theme defines different shades (e.g.,
green-500), adjust accordingly.
167-171: Fix progress bar color class; remove non-existentbg-green.- item.progress === 100 - ? "bg-green" - : item.progress > 0 - ? "bg-green-1" - : "" + item.progress === 100 + ? "bg-green-1" + : item.progress > 0 + ? "bg-green-1/60" + : ""
130-133: Add basic a11y: aria-disabled and progressbar semantics.- <div + <div key={topic.id} className={`bg-dark-blue-5 rounded-lg p-4 border-2 border-[#1f2937] relative ${ !item.unlocked ? "opacity-60 pointer-events-none" : "" }`} + aria-disabled={!item.unlocked} > ... - <div className="h-2 w-full bg-neutral-1/10 rounded-full overflow-hidden"> + <div + className="h-2 w-full bg-neutral-1/10 rounded-full overflow-hidden" + role="progressbar" + aria-label={`${topic.title} progress`} + aria-valuemin={0} + aria-valuemax={100} + aria-valuenow={item.progress} + > <div className={`h-full ${ item.progress === 100 ? "bg-green-1" : item.progress > 0 ? "bg-green-1/60" : "" }`} style={{ width: `${item.progress}%` }} /> - </div> + </div>Also applies to: 164-175
5-34: Optional: removeprogress/unlockedfromlisteningTopicsto avoid drift with state.const listeningTopics = [ { id: "basic-conversations", title: "Basic Conversations", - progress: 100, - unlocked: true, }, { id: "everyday-dialogues", title: "Everyday Dialogues", - progress: 80, - unlocked: true, }, - { id: "podcasts", title: "Podcasts", progress: 65, unlocked: true }, + { id: "podcasts", title: "Podcasts" }, { id: "news-broadcasts", title: "News Broadcasts", - progress: 45, - unlocked: true, }, - { id: "interviews", title: "Interviews", progress: 20, unlocked: true }, - { id: "lectures", title: "Lectures", progress: 0, unlocked: false }, - { id: "movies-tv", title: "Movies & TV", progress: 0, unlocked: false }, + { id: "interviews", title: "Interviews" }, + { id: "lectures", title: "Lectures" }, + { id: "movies-tv", title: "Movies & TV" }, { id: "advanced-listening", title: "Advanced Listening", - progress: 0, - unlocked: false, }, ];
36-47: DRY across pages with a small hook.Given identical logic in grammar/reading/speaking, consider a
useSequentialProgress(storageKey, topics)hook that returns[state, complete]. Cuts duplication and centralizes JSON/SSR handling.Also applies to: 50-68, 121-127, 184-187
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/assessment/AudioAssessment.jsx(5 hunks)src/components/games/word-matching/game-board.jsx(0 hunks)src/pages/aurora-site/assessment/pronunciation-assessment.jsx(5 hunks)src/pages/aurora-site/grammar-content.jsx(2 hunks)src/pages/aurora-site/learning/listening-content.jsx(1 hunks)src/pages/aurora-site/learning/reading-content.jsx(1 hunks)src/pages/aurora-site/learning/speaking-content.jsx(2 hunks)
💤 Files with no reviewable changes (1)
- src/components/games/word-matching/game-board.jsx
🚧 Files skipped from review as they are similar to previous changes (5)
- src/pages/aurora-site/learning/reading-content.jsx
- src/pages/aurora-site/grammar-content.jsx
- src/pages/aurora-site/assessment/pronunciation-assessment.jsx
- src/components/assessment/AudioAssessment.jsx
- src/pages/aurora-site/learning/speaking-content.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/aurora-site/learning/listening-content.jsx (3)
src/pages/aurora-site/grammar-content.jsx (5)
STORAGE_KEY(52-52)getInitialProgress(54-63)stored(55-55)progressList(66-66)handleComplete(74-84)src/pages/aurora-site/learning/reading-content.jsx (5)
STORAGE_KEY(40-40)getInitialProgress(42-51)stored(43-43)progressList(54-54)handleComplete(62-73)src/pages/aurora-site/learning/speaking-content.jsx (5)
STORAGE_KEY(84-84)getInitialProgress(86-95)stored(87-87)progressList(98-98)handleComplete(106-117)
| const [progressList, setProgressList] = useState(getInitialProgress()); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use lazy state init and build an id->state map once.
Avoid running getInitialProgress() every render; memoize lookup map.
- const [progressList, setProgressList] = useState(getInitialProgress());
+ const [progressList, setProgressList] = useState(getInitialProgress);
+ const listById = useMemo(
+ () => new Map(progressList.map((p) => [p.id, p])),
+ [progressList]
+ );📝 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 [progressList, setProgressList] = useState(getInitialProgress()); | |
| const [progressList, setProgressList] = useState(getInitialProgress); | |
| const listById = useMemo( | |
| () => new Map(progressList.map((p) => [p.id, p])), | |
| [progressList] | |
| ); |
🤖 Prompt for AI Agents
In src/pages/aurora-site/learning/listening-content.jsx around lines 50-51,
change the state initialization to use lazy init so getInitialProgress() only
runs once by passing a function to useState (e.g. useState(() =>
getInitialProgress())), and create a single id->state lookup map rather than
recomputing on each render — for example, build the map once into a ref (useRef)
from the initial progressList and on any updates keep the ref in sync inside
setProgressList (or update the map where you change items); then replace any
array scans with O(1) lookups against that map.
| // Persist progress to localStorage | ||
| useEffect(() => { | ||
| localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList)); | ||
| }, [progressList]); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Persist only minimal, non-derived state.
Store {id, progress} only; keep unlocked derived in-memory.
- useEffect(() => {
- localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList));
- }, [progressList]);
+ useEffect(() => {
+ window.localStorage.setItem(
+ STORAGE_KEY,
+ JSON.stringify(progressList.map(({ id, progress }) => ({ id, progress })))
+ );
+ }, [progressList]);📝 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.
| // Persist progress to localStorage | |
| useEffect(() => { | |
| localStorage.setItem(STORAGE_KEY, JSON.stringify(progressList)); | |
| }, [progressList]); | |
| // Persist only minimal, non-derived state to localStorage | |
| useEffect(() => { | |
| window.localStorage.setItem( | |
| STORAGE_KEY, | |
| JSON.stringify( | |
| progressList.map(({ id, progress }) => ({ id, progress })) | |
| ) | |
| ); | |
| }, [progressList]); |
🤖 Prompt for AI Agents
In src/pages/aurora-site/learning/listening-content.jsx around lines 52-56, the
useEffect persists the full progressList to localStorage including derived
fields like unlocked; change persistence to save only minimal state by mapping
progressList to an array of { id, progress } before JSON.stringify and storing
that under STORAGE_KEY. Also update the load/rehydration logic elsewhere to read
that minimal array and merge it back into the in-memory progressList, computing
unlocked in-memory (not from storage). Ensure any future writes follow the same
mapping so only id and progress are ever persisted.
| // Simulate marking a lesson complete (for demo) | ||
| const handleComplete = (idx) => { | ||
| setProgressList((prev) => { | ||
| const updated = prev.map((item, i) => | ||
| i === idx ? { ...item, progress: 100 } : item | ||
| ); | ||
| return updated.map((item, i, arr) => ({ | ||
| ...item, | ||
| unlocked: i === 0 ? true : arr[i - 1].progress === 100, | ||
| })); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Index-based updates break on reorders; update by id and then recompute unlocks.
- const handleComplete = (idx) => {
- setProgressList((prev) => {
- const updated = prev.map((item, i) =>
- i === idx ? { ...item, progress: 100 } : item
- );
- return updated.map((item, i, arr) => ({
- ...item,
- unlocked: i === 0 ? true : arr[i - 1].progress === 100,
- }));
- });
- };
+ const handleComplete = (id) => {
+ setProgressList((prev) => {
+ const byId = new Map(prev.map((p) => [p.id, p]));
+ const current = byId.get(id) || { id, progress: 0, unlocked: false };
+ byId.set(id, { ...current, progress: 100 });
+
+ const next = listeningTopics.map((t) => ({
+ id: t.id,
+ progress: byId.get(t.id)?.progress ?? 0,
+ unlocked: false,
+ }));
+ return next.map((item, i, arr) => ({
+ ...item,
+ unlocked: i === 0 ? true : arr[i - 1].progress === 100,
+ }));
+ });
+ };📝 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.
| // Simulate marking a lesson complete (for demo) | |
| const handleComplete = (idx) => { | |
| setProgressList((prev) => { | |
| const updated = prev.map((item, i) => | |
| i === idx ? { ...item, progress: 100 } : item | |
| ); | |
| return updated.map((item, i, arr) => ({ | |
| ...item, | |
| unlocked: i === 0 ? true : arr[i - 1].progress === 100, | |
| })); | |
| }); | |
| }; | |
| // Simulate marking a lesson complete (for demo) | |
| const handleComplete = (id) => { | |
| setProgressList((prev) => { | |
| // Build a map of existing progress by ID | |
| const byId = new Map(prev.map((p) => [p.id, p])); | |
| // Fetch or initialize the current item, then mark it complete | |
| const current = byId.get(id) || { id, progress: 0, unlocked: false }; | |
| byId.set(id, { ...current, progress: 100 }); | |
| // Reconstruct the full list in topic order, reconciling progress | |
| const next = listeningTopics.map((t) => ({ | |
| id: t.id, | |
| progress: byId.get(t.id)?.progress ?? 0, | |
| unlocked: false, | |
| })); | |
| // Recompute unlock states based on completed progress | |
| return next.map((item, i, arr) => ({ | |
| ...item, | |
| unlocked: i === 0 ? true : arr[i - 1].progress === 100, | |
| })); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In src/pages/aurora-site/learning/listening-content.jsx around lines 57 to 68,
the handleComplete updater uses the array index to mark progress and compute
unlocked flags which breaks if the list is reordered; change it to accept an
item id, map the list to set progress where item.id === id, then recompute
unlocked in a stable order by iterating the resulting array in sequence and
setting unlocked = true for the first item or when the previous item's progress
=== 100; update any callers to pass the lesson id instead of index.
| <button | ||
| className="absolute top-4 right-4 text-xs bg-light-blue-1 text-white px-2 py-1 rounded" | ||
| onClick={() => handleComplete(idx)} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Click handler should pass a stable id, not an index.
- onClick={() => handleComplete(idx)}
+ onClick={() => handleComplete(topic.id)}📝 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 | |
| className="absolute top-4 right-4 text-xs bg-light-blue-1 text-white px-2 py-1 rounded" | |
| onClick={() => handleComplete(idx)} | |
| > | |
| <button | |
| className="absolute top-4 right-4 text-xs bg-light-blue-1 text-white px-2 py-1 rounded" | |
| onClick={() => handleComplete(topic.id)} | |
| > |
🤖 Prompt for AI Agents
In src/pages/aurora-site/learning/listening-content.jsx around lines 184-187 the
button onClick is passing the loop index (idx) to handleComplete which is
unstable; change the call to pass the item’s stable identifier (e.g., item.id or
listening.id) instead, update the handleComplete signature to accept an id (not
an index) and use that id for state updates/requests, and ensure any places that
call or rely on handleComplete are updated to use the stable id; also confirm
the map uses that same id as the React key.
2 participants
🚀 Pull Request
Description
/grammer,/speaking,/listening,/reading.mark completebutton to each lesson card to simulate completion of a lesson and the unlocking of the next lessonRelated Issue
Closes #263
Type of Change
Proof of Completion
For speaking lessons
Screencast from 2025-08-20 20-33-23.webm
For grammar lessons
Screencast from 2025-08-20 20-38-02.webm
For listening lessons
Screencast from 2025-08-20 20-39-22.webm
For reading lessons
Screencast from 2025-08-20 20-40-18.webm
Checklist
@zleypner
Please review. Thanks.
Summary by CodeRabbit
New Features
New Features
Public API
Style