QF-2639 & QF-239 - Fix RepeatAudioModal to correctly load settings #2548
Conversation
* Use values stored in the database when logged in * Use values from Redux storage when not logged in * Fallback to default values otherwise
|
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. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const onPlayClick = () => { | ||
| logButtonClick('start_repeat_play'); | ||
| onSettingsChangeWithoutDispatch('repeatSettings', verseRepetition, PreferenceGroup.AUDIO, play); | ||
| const normalizedRange = normalizeVerseRange(verseRepetition.from, verseRepetition.to); | ||
| const normalizedSettings = { | ||
| ...verseRepetition, | ||
| ...normalizedRange, | ||
| }; | ||
| setVerseRepetition(normalizedSettings); | ||
| onSettingsChangeWithoutDispatch( | ||
| 'repeatSettings', | ||
| normalizedSettings, | ||
| PreferenceGroup.AUDIO, | ||
| () => play(normalizedSettings), |
There was a problem hiding this comment.
The modal now reads audioPlayerState.repeatSettings to populate defaults once the repeat actor is gone, but onPlayClick persists changes via onSettingsChangeWithoutDispatch, which only calls the backend and invokes the callback. No Redux action updates audioPlayerState.repeatSettings, so for anonymous users (and until a server sync occurs for authenticated ones) the slice remains undefined and the modal still resets to defaults after closing. Dispatch a local update when saving so the newly added Redux-based fallback actually stores the settings.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (3)
434-442: Fix analytics key typo and avoid assigning possibly undefined verse keys.
- Typo:
repitition_mode→repetition_mode.- When Single mode and
selectedVerseKeyis undefined,from/tobecome undefined (state type says string). Use safe fallbacks and normalize.- logValueChange('repitition_mode', repetitionMode, mode); - setVerseRepetition((prevVerseRepetition) => ({ - ...prevVerseRepetition, - from: mode === RepetitionMode.Single ? selectedVerseKey : firstVerseKeyInThisChapter, - to: mode === RepetitionMode.Single ? selectedVerseKey : lastVerseKeyInThisChapter, - })); + logValueChange('repetition_mode', repetitionMode, mode); + setVerseRepetition((prev) => { + const singleKey = selectedVerseKey ?? prev.from; + const nextFrom = + mode === RepetitionMode.Single + ? singleKey + : firstVerseKeyInThisChapter ?? prev.from; + const nextTo = + mode === RepetitionMode.Single + ? singleKey + : lastVerseKeyInThisChapter ?? prev.to; + const normalized = normalizeVerseRange(nextFrom, nextTo); + return { ...prev, ...normalized }; + });This prevents undefined state and preserves correct ordering.
444-448: Analytics old value is wrong dimension.Logging uses
repeatRangeas the “old value” when changing a single verse. Use the previous verse key.- logValueChange('repeat_single_verse', verseRepetition.repeatRange, verseKey); + logValueChange('repeat_single_verse', verseRepetition.from, verseKey);This keeps analytics consistent. As per coding guidelines.
465-478: Add explicit parameter types to handlers.Tighten types per guidelines; avoid implicit
any.-const onSingleVerseChange = (verseKey) => { +const onSingleVerseChange = (verseKey: string) => { ... }; -const onRangeChange = (range) => { +const onRangeChange = (range: Partial<{ from: string; to: string }>) => { ... }; -const onRepeatRangeChange = (val) => { +const onRepeatRangeChange = (val: number) => { ... }; -const onRepeatEachVerseChange = (val) => { +const onRepeatEachVerseChange = (val: number) => { ... }; -const onDelayMultiplierChange = (val) => { +const onDelayMultiplierChange = (val: number) => { ... };As per coding guidelines (TypeScript strictness).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx(7 hunks)src/redux/types/AudioState.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (17)
**/*.*
📄 CodeRabbit inference engine (.cursor/rules/bug-handling-with-todo-comments.mdc)
If you encounter a bug in existing code, or the instructions lead to suboptimal or buggy code, add comments starting with "TODO:" outlining the problems.
**/*.*: Utilize Early Returns: Use early returns to avoid nested conditions and improve readability.
Conditional Classes: Prefer conditional classes over ternary operators for class attributes.
**/*.*: Use comments sparingly, and when you do, make them meaningful.
Don't comment on obvious things. Excessive or unclear comments can clutter the codebase and become outdated.
Use comments to convey the 'why' behind specific actions or explain unusual behavior and potential pitfalls.
Provide meaningful information about the function's behavior and explain unusual behavior and potential pitfalls.
**/*.*: Write short functions that only do one thing.
Follow the single responsibility principle (SRP), which means that a function should have one purpose and perform it effectively.
If a function becomes too long or complex, consider breaking it into smaller, more manageable functions.Order functions with those that are composing other functions appearing earlier in the file. For example, if you have a menu with multiple buttons, define the menu function above the buttons.
**/*.*: Always add helpful comments to the code explaining what you are doing.
Never delete old comments, unless they are no longer relevant because the code has been rewritten or deleted.
**/*.*: Choose names for variables, functions, and classes that reflect their purpose and behavior.
A name should tell you why it exists, what it does, and how it is used. If a name requires a comment, then the name does not reveal its intent.
Use specific names that provide a clearer understanding of what the variables represent and how they are used.
Files:
src/redux/types/AudioState.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/next-js-conventions.mdc)
**/*.{js,jsx,ts,tsx}: Rely on Next.js Pages Router for state changes.
Minimize 'use client' usage: Prefer server components and Next.js SSR features.
Minimize 'use client' usage: Use 'use client' only for Web API access in small components.
Minimize 'use client' usage: Avoid using 'use client' for data fetching or state management.
**/*.{js,jsx,ts,tsx}: Optimize Web Vitals (LCP, CLS, FID)
Use dynamic loading for non-critical components using @src/components/dls/Spinner/Spinner.tsx
Files:
src/redux/types/AudioState.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/{store,redux}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Implement Redux Toolkit for efficient Redux development for medium-complex state logic
Files:
src/redux/types/AudioState.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use xstate for complex state logic
**/*.{ts,tsx}: TypeScript: prefer interfaces over type aliases for object shapes; avoid any; add explicit return types for public functions
Error handling: define and use custom error types; use async/await correctly; show user-friendly error messages
Use Redux Toolkit for state management and XState for complex state machines
Implement i18n with next-translate in components and utilities
Files:
src/redux/types/AudioState.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
src/redux/**/*
📄 CodeRabbit inference engine (.cursor/rules/redux-folder-structure.mdc)
Follow this folder structure: src/redux/actions/, src/redux/slices/, src/redux/RootState.ts, src/redux/store.ts, src/redux/migrations.ts
Files:
src/redux/types/AudioState.ts
src/redux/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/redux-toolkit-best-practices.mdc)
src/redux/**/*.ts: Use Redux Toolkit for efficient Redux development.
Implement slice pattern for organizing Redux code.
Use selectors for accessing state in components.
Use Redux hooks (useSelector, useDispatch) in components.
Follow Redux style guide for naming conventionsUse Redux Toolkit for state management and persistence (redux-persist) within the redux layer
Files:
src/redux/types/AudioState.ts
**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx,d.ts}: Prefer interfaces over types for object definitions
Use type for unions, intersections, and mapped types
Avoid usingany, preferunknownfor unknown types
Leverage TypeScript's built-in utility types
Use generics for reusable type patterns
Use PascalCase for type names and interfaces
Use camelCase for variables and functions
Use UPPER_CASE for constants
Use descriptive names with auxiliary verbs (e.g., isLoading, hasError)
Use explicit return types for public functions
Use arrow functions for callbacks and methods
Implement proper error handling with custom error types
Use function overloads for complex type scenarios
Prefer async/await over Promises
Use readonly for immutable properties
Leverage discriminated unions for type safety
Use type guards for runtime type checking
Implement proper null checking
Avoid type assertions unless necessary
Create custom error types for domain-specific errors
Use Result types for operations that can fail
Use try-catch blocks with typed catch clauses
Handle Promise rejections properly
Files:
src/redux/types/AudioState.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/types/**/*.{ts,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/types/**/*.{ts,d.ts}: Export types and interfaces from dedicated type files when shared
Place shared types in atypesdirectory
Files:
src/redux/types/AudioState.ts
{src,types}/**/*.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript throughout the codebase with strict configuration
Files:
src/redux/types/AudioState.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
{src,types}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use path aliases @/ for src and @/dls/* for design system imports
Files:
src/redux/types/AudioState.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Order imports: React first, then external, then internal; alphabetize within groups with blank lines between groups
Naming: camelCase for variables/functions; PascalCase for types, interfaces, and React components; UPPER_CASE for constants
Functions should be <= 30 lines, prefer early returns, single responsibility, and descriptive names
Files:
src/redux/types/AudioState.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{ts,tsx,js,jsx,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier formatting: single quotes, 2-space indentation, 100-character line width, and trailing commas
Files:
src/redux/types/AudioState.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react-functional-components.mdc)
src/components/**/*.tsx: Always use React functional components with hooks.
Use React.FC for functional components with props.
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use functional components over class components
Keep components small and focused
Extract reusable logic into custom hooks
Use composition over inheritance
Split large components into smaller, focused ones
Follow the Rules of Hooks
Use custom hooks for reusable logic
Use appropriate dependency arrays in useEffect
Implement cleanup in useEffect when needed
Avoid nested hooks
Use useState for local component state
Avoid prop drilling through proper state management
Implement proper memoization (useMemo, useCallback)
Use React.memo for expensive components
Avoid unnecessary re-renders
Implement proper lazy loading
Use proper key props in lists
Profile and optimize render performance
Show appropriate loading and error states
Handle async errors properly
Show user-friendly error messages
Implement proper fallback UI
Log errors appropriately
Handle edge cases gracefully
Use semantic HTML elements
Implement proper ARIA attributes
Ensure keyboard navigation
Handle focus management
Provide proper alt text for images
Use proper imports/exports
Document complex component logic
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Implement proper prop types with TypeScript
**/*.tsx: Prefix interfaces for React props with 'Props' (e.g., ButtonProps)
Implement proper error boundariesReact: use functional components only; extract reusable logic into custom hooks; apply proper memoization (React.memo/useMemo/useCallback)
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer React functional components for UI implementation
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Organize React components by feature and place DLS components under src/components/dls
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
🧬 Code graph analysis (1)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (2)
src/utils/verse.ts (3)
getChapterNumberFromKey(44-45)makeVerseKey(217-227)getVerseNumberFromKey(54-55)src/redux/slices/AudioPlayer/state.ts (1)
selectAudioPlayerState(12-12)
🔇 Additional comments (3)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (3)
172-209: Range normalization looks solid. LGTM.
347-383: Memo default builder looks correct and dependency list is comprehensive. LGTM.
294-315: Selector usage looks correct; actor context reads are guarded. LGTM.
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
Outdated
Show resolved
Hide resolved
| export interface RepeatSettingsPreference { | ||
| from?: string; | ||
| to?: string; | ||
| repeatRange?: number | string | null; | ||
| repeatEachVerse?: number | string | null; | ||
| delayMultiplier?: number | string | null; | ||
| } |
There was a problem hiding this comment.
Clarify verse key format and allowed string values.
Add brief docs stating from/to use the chapter:verse format (e.g., "2:255") and that strings for numeric fields may include the literal "Infinity" from persisted JSON. This improves DX and prevents misuse.
TODO: Document the verse key shape and why string unions are needed (JSON persistence of numbers and "Infinity"). As per coding guidelines.
🤖 Prompt for AI Agents
In src/redux/types/AudioState.ts around lines 1-7, the RepeatSettingsPreference
interface lacks documentation for the shape of verse keys and allowed string
values for numeric fields; add JSDoc comments above the interface and for the
from/to fields noting they use "chapter:verse" format (e.g., "2:255"), and for
repeatRange, repeatEachVerse, and delayMultiplier document that they accept
number | string | null and that persisted JSON may supply numeric strings
including the literal "Infinity"; update the field comments to exactly list the
allowed formats/values so consumers and linters understand the string union
usage.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (4)
525-533: Fix logging key typo.Use the correct event name.
- logValueChange('repitition_mode', repetitionMode, mode); + logValueChange('repetition_mode', repetitionMode, mode);
535-538: Log the previous verse key, not the repeat count.Old value should be the prior selected verse key.
- const onSingleVerseChange = (verseKey) => { - logValueChange('repeat_single_verse', verseRepetition.repeatRange, verseKey); + const onSingleVerseChange = (verseKey: string) => { + logValueChange('repeat_single_verse', verseRepetition.from, verseKey); setVerseRepetition({ ...verseRepetition, from: verseKey, to: verseKey }); };
540-569: Tighten handler types.Add explicit param types for clarity and safety.
- const onRangeChange = (range) => { + const onRangeChange = (range: { from?: string; to?: string }) => { @@ - const onRepeatRangeChange = (val) => { + const onRepeatRangeChange = (val: number) => { @@ - const onRepeatEachVerseChange = (val) => { + const onRepeatEachVerseChange = (val: number) => { @@ - const onDelayMultiplierChange = (val) => { + const onDelayMultiplierChange = (val: number) => {
358-364: Annotate component with React.FC and props.Aligns with project guideline to use React.FC for components with props.
-const RepeatAudioModal = ({ +const RepeatAudioModal: React.FC<RepeatAudioModalProps> = ({ chapterId, isOpen, onClose, defaultRepetitionMode, selectedVerseKey, }: RepeatAudioModalProps) => {src/utils/auth/preferencesMapper.ts (1)
38-47: Include repeatSettings in AUDIO group preference mapping to prevent data loss.The code currently excludes
repeatSettingsfrom the AUDIO_PLAYER_STATE preference mapping (lines 38–47 ofsrc/utils/auth/preferencesMapper.ts), even though it's defined inAudioStateand managed in Redux. If the server uses a full replace (not merge) strategy for preference groups, a complete sync viastateToPreferenceGroupswould clear user repeat settings. Additionally, serialization logic for handlingInfinityvalues exists only inRepeatAudioModaland should be extracted to avoid duplication.Suggested fix:
- Add
repeatSettingsextraction and normalizeInfinityvalues for API compatibility (as shown in the review's diff)- Extract the serialization logic from
RepeatAudioModalinto a shared utility to prevent duplication acrosspreferencesMapper.tsand repeat settings persistencesrc/redux/slices/AudioPlayer/state.ts (1)
62-78: REHYDRATE handler doesn't fix Infinity→null; both branches return identical state.The code checks if
repeatRange === nullbut returns the same spread in both branches, making the condition ineffective. All three numeric fields inrepeatSettingsare affected by JSON's Infinity→null conversion (per type definition insrc/redux/types/AudioState.ts), yet onlyrepeatRangeis mentioned in the comment and no actual conversion occurs.builder.addCase(REHYDRATE, (state, action) => { // @ts-ignore redux-persist types const { key, payload } = action; if (!payload) return state; // Support both root-shaped and slice-shaped payloads const slicePayload = payload[SliceName.AUDIO_PLAYER_STATE] ?? payload; if (!slicePayload) return state; const fixNull = (v: unknown) => (v === null ? 'Infinity' : v); const nextRepeat = slicePayload.repeatSettings ? { ...slicePayload.repeatSettings, repeatRange: fixNull(slicePayload.repeatSettings.repeatRange), repeatEachVerse: fixNull(slicePayload.repeatSettings.repeatEachVerse), delayMultiplier: fixNull(slicePayload.repeatSettings.delayMultiplier), } : undefined; return { ...state, ...slicePayload, ...(nextRepeat && { repeatSettings: nextRepeat }), }; });
♻️ Duplicate comments (2)
src/redux/types/AudioState.ts (2)
3-9: Document verse key format and JSON-safe numeric strings.Add concise JSDoc so callers know from/to shape and why string "Infinity" is allowed.
export interface RepeatSettingsPreference { - from?: string; - to?: string; + /** + * Verse keys use "chapter:verse" (e.g., "2:255"). Ranges are not accepted here. + */ + from?: string; + /** + * Verse keys use "chapter:verse" (e.g., "2:257"). + */ + to?: string; repeatRange?: number | JsonNumberString | null; repeatEachVerse?: number | JsonNumberString | null; delayMultiplier?: number | JsonNumberString | null; }
11-16: Prefer interface for object shape.Use an interface for AudioState to align with project guidelines and improve declaration merging flexibility.
-export type AudioState = { +export interface AudioState { enableAutoScrolling: boolean; isDownloadingAudio: boolean; showTooltipWhenPlayingAudio: boolean; repeatSettings?: RepeatSettingsPreference; -}; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx(6 hunks)src/hooks/auth/usePersistPreferenceGroup.ts(2 hunks)src/redux/defaultSettings/defaultSettings.ts(1 hunks)src/redux/defaultSettings/util.ts(1 hunks)src/redux/slices/AudioPlayer/state.ts(3 hunks)src/redux/types/AudioState.ts(1 hunks)src/utils/auth/preferencesMapper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (17)
**/*.*
📄 CodeRabbit inference engine (.cursor/rules/bug-handling-with-todo-comments.mdc)
If you encounter a bug in existing code, or the instructions lead to suboptimal or buggy code, add comments starting with "TODO:" outlining the problems.
**/*.*: Utilize Early Returns: Use early returns to avoid nested conditions and improve readability.
Conditional Classes: Prefer conditional classes over ternary operators for class attributes.
**/*.*: Use comments sparingly, and when you do, make them meaningful.
Don't comment on obvious things. Excessive or unclear comments can clutter the codebase and become outdated.
Use comments to convey the 'why' behind specific actions or explain unusual behavior and potential pitfalls.
Provide meaningful information about the function's behavior and explain unusual behavior and potential pitfalls.
**/*.*: Write short functions that only do one thing.
Follow the single responsibility principle (SRP), which means that a function should have one purpose and perform it effectively.
If a function becomes too long or complex, consider breaking it into smaller, more manageable functions.Order functions with those that are composing other functions appearing earlier in the file. For example, if you have a menu with multiple buttons, define the menu function above the buttons.
**/*.*: Always add helpful comments to the code explaining what you are doing.
Never delete old comments, unless they are no longer relevant because the code has been rewritten or deleted.
**/*.*: Choose names for variables, functions, and classes that reflect their purpose and behavior.
A name should tell you why it exists, what it does, and how it is used. If a name requires a comment, then the name does not reveal its intent.
Use specific names that provide a clearer understanding of what the variables represent and how they are used.
Files:
src/redux/slices/AudioPlayer/state.tssrc/hooks/auth/usePersistPreferenceGroup.tssrc/utils/auth/preferencesMapper.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsxsrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/next-js-conventions.mdc)
**/*.{js,jsx,ts,tsx}: Rely on Next.js Pages Router for state changes.
Minimize 'use client' usage: Prefer server components and Next.js SSR features.
Minimize 'use client' usage: Use 'use client' only for Web API access in small components.
Minimize 'use client' usage: Avoid using 'use client' for data fetching or state management.
**/*.{js,jsx,ts,tsx}: Optimize Web Vitals (LCP, CLS, FID)
Use dynamic loading for non-critical components using @src/components/dls/Spinner/Spinner.tsx
Files:
src/redux/slices/AudioPlayer/state.tssrc/hooks/auth/usePersistPreferenceGroup.tssrc/utils/auth/preferencesMapper.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsxsrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
**/{store,redux}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Implement Redux Toolkit for efficient Redux development for medium-complex state logic
Files:
src/redux/slices/AudioPlayer/state.tssrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use xstate for complex state logic
**/*.{ts,tsx}: TypeScript: prefer interfaces over type aliases for object shapes; avoid any; add explicit return types for public functions
Error handling: define and use custom error types; use async/await correctly; show user-friendly error messages
Use Redux Toolkit for state management and XState for complex state machines
Implement i18n with next-translate in components and utilities
Files:
src/redux/slices/AudioPlayer/state.tssrc/hooks/auth/usePersistPreferenceGroup.tssrc/utils/auth/preferencesMapper.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsxsrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
src/redux/**/*
📄 CodeRabbit inference engine (.cursor/rules/redux-folder-structure.mdc)
Follow this folder structure: src/redux/actions/, src/redux/slices/, src/redux/RootState.ts, src/redux/store.ts, src/redux/migrations.ts
Files:
src/redux/slices/AudioPlayer/state.tssrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
src/redux/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/redux-toolkit-best-practices.mdc)
src/redux/**/*.ts: Use Redux Toolkit for efficient Redux development.
Implement slice pattern for organizing Redux code.
Use selectors for accessing state in components.
Use Redux hooks (useSelector, useDispatch) in components.
Follow Redux style guide for naming conventionsUse Redux Toolkit for state management and persistence (redux-persist) within the redux layer
Files:
src/redux/slices/AudioPlayer/state.tssrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx,d.ts}: Prefer interfaces over types for object definitions
Use type for unions, intersections, and mapped types
Avoid usingany, preferunknownfor unknown types
Leverage TypeScript's built-in utility types
Use generics for reusable type patterns
Use PascalCase for type names and interfaces
Use camelCase for variables and functions
Use UPPER_CASE for constants
Use descriptive names with auxiliary verbs (e.g., isLoading, hasError)
Use explicit return types for public functions
Use arrow functions for callbacks and methods
Implement proper error handling with custom error types
Use function overloads for complex type scenarios
Prefer async/await over Promises
Use readonly for immutable properties
Leverage discriminated unions for type safety
Use type guards for runtime type checking
Implement proper null checking
Avoid type assertions unless necessary
Create custom error types for domain-specific errors
Use Result types for operations that can fail
Use try-catch blocks with typed catch clauses
Handle Promise rejections properly
Files:
src/redux/slices/AudioPlayer/state.tssrc/hooks/auth/usePersistPreferenceGroup.tssrc/utils/auth/preferencesMapper.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsxsrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
{src,types}/**/*.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript throughout the codebase with strict configuration
Files:
src/redux/slices/AudioPlayer/state.tssrc/hooks/auth/usePersistPreferenceGroup.tssrc/utils/auth/preferencesMapper.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsxsrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
{src,types}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use path aliases @/ for src and @/dls/* for design system imports
Files:
src/redux/slices/AudioPlayer/state.tssrc/hooks/auth/usePersistPreferenceGroup.tssrc/utils/auth/preferencesMapper.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsxsrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Order imports: React first, then external, then internal; alphabetize within groups with blank lines between groups
Naming: camelCase for variables/functions; PascalCase for types, interfaces, and React components; UPPER_CASE for constants
Functions should be <= 30 lines, prefer early returns, single responsibility, and descriptive names
Files:
src/redux/slices/AudioPlayer/state.tssrc/hooks/auth/usePersistPreferenceGroup.tssrc/utils/auth/preferencesMapper.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsxsrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
**/*.{ts,tsx,js,jsx,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier formatting: single quotes, 2-space indentation, 100-character line width, and trailing commas
Files:
src/redux/slices/AudioPlayer/state.tssrc/hooks/auth/usePersistPreferenceGroup.tssrc/utils/auth/preferencesMapper.tssrc/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsxsrc/redux/defaultSettings/defaultSettings.tssrc/redux/defaultSettings/util.tssrc/redux/types/AudioState.ts
src/components/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react-functional-components.mdc)
src/components/**/*.tsx: Always use React functional components with hooks.
Use React.FC for functional components with props.
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use functional components over class components
Keep components small and focused
Extract reusable logic into custom hooks
Use composition over inheritance
Split large components into smaller, focused ones
Follow the Rules of Hooks
Use custom hooks for reusable logic
Use appropriate dependency arrays in useEffect
Implement cleanup in useEffect when needed
Avoid nested hooks
Use useState for local component state
Avoid prop drilling through proper state management
Implement proper memoization (useMemo, useCallback)
Use React.memo for expensive components
Avoid unnecessary re-renders
Implement proper lazy loading
Use proper key props in lists
Profile and optimize render performance
Show appropriate loading and error states
Handle async errors properly
Show user-friendly error messages
Implement proper fallback UI
Log errors appropriately
Handle edge cases gracefully
Use semantic HTML elements
Implement proper ARIA attributes
Ensure keyboard navigation
Handle focus management
Provide proper alt text for images
Use proper imports/exports
Document complex component logic
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Implement proper prop types with TypeScript
**/*.tsx: Prefix interfaces for React props with 'Props' (e.g., ButtonProps)
Implement proper error boundariesReact: use functional components only; extract reusable logic into custom hooks; apply proper memoization (React.memo/useMemo/useCallback)
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer React functional components for UI implementation
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Organize React components by feature and place DLS components under src/components/dls
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/types/**/*.{ts,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/types/**/*.{ts,d.ts}: Export types and interfaces from dedicated type files when shared
Place shared types in atypesdirectory
Files:
src/redux/types/AudioState.ts
🧠 Learnings (1)
📚 Learning: 2025-10-07T08:38:28.343Z
Learnt from: CR
PR: quran/quran.com-frontend-next#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T08:38:28.343Z
Learning: Applies to **/*.{ts,tsx} : TypeScript: prefer interfaces over type aliases for object shapes; avoid any; add explicit return types for public functions
Applied to files:
src/redux/types/AudioState.ts
🧬 Code graph analysis (2)
src/redux/slices/AudioPlayer/state.ts (1)
src/redux/types/AudioState.ts (1)
RepeatSettingsPreference(3-9)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (4)
src/utils/verse.ts (3)
getChapterNumberFromKey(44-45)makeVerseKey(217-227)getVerseNumberFromKey(54-55)src/redux/types/AudioState.ts (2)
JsonNumberString(1-1)RepeatSettingsPreference(3-9)src/redux/slices/AudioPlayer/state.ts (1)
selectAudioPlayerState(12-12)src/utils/eventLogger.ts (1)
logButtonClick(44-46)
🔇 Additional comments (10)
src/hooks/auth/usePersistPreferenceGroup.ts (2)
166-168: LGTM! Consistency improvement for non-logged-in users.This change ensures that
successCallbackis invoked for non-logged-in users, mirroring the behavior for logged-in users (lines 134-136). This fixes a potential bug where callbacks would not be executed when users aren't authenticated.
223-225: LGTM! Consistent callback handling added.This change ensures that
successCallbackis invoked for non-logged-in users after dispatching the action, matching the behavior for logged-in users (lines 191-193). Together with the similar change inonXstateSettingsChange, this provides consistent callback handling across all authentication states.src/redux/defaultSettings/util.ts (1)
9-9: LGTM on named import.Import matches the new named export; no functional changes.
src/redux/defaultSettings/defaultSettings.ts (1)
3-3: LGTM on named import.Consistent with updated typings; AUDIO_INITIAL_STATE remains valid with optional repeatSettings.
src/redux/slices/AudioPlayer/state.ts (1)
36-39: Good addition: local persistence of repeatSettings.Reducer cleanly updates state; pairs well with modal’s optimistic dispatch.
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (4)
137-153: Nice guard on selectedVerseKey.Validates the selected key belongs to the current chapter before using it.
236-275: Robust JSON-safe serialization."Infinity" handling and selective field emission look correct and side‑effect free.
467-493: Runtime guards + numeric send-path are correct.Early-returns prevent crashes; removed redundant Number(...) casts. Good.
495-513: onSettingsChange signature verified — code is correct.The call at lines 505–513 matches the hook's signature exactly:
- key:
'repeatSettings'✓- value:
prepareRepeatSettingsForApi(serializableSettings)✓- action:
setRepeatSettings(serializableSettings)(Redux action) ✓- undoAction:
setRepeatSettings(previousRepeatSettings)(Redux action) ✓- preferenceGroup:
PreferenceGroup.AUDIO✓- successCallback:
() => play(normalizedSettings)(optional param) ✓The implementation properly serializes settings, prepares the API payload, and dispatches both forward and rollback actions with a success callback.
src/utils/auth/preferencesMapper.ts (1)
4-4: No remaining default imports of AudioState exist.Verification confirms:
- Line 4 in preferencesMapper.ts correctly imports
{ AudioState }as a named export- No other default imports of AudioState found across the codebase
- AudioState.ts exports only as named export
| const serializeRepeatSettings = (settings: RepeatSettingsLike): RepeatSettingsPreference => { | ||
| const repeatRange = serializeRepeatNumericValue(settings.repeatRange); | ||
| const repeatEachVerse = serializeRepeatNumericValue(settings.repeatEachVerse); | ||
| const delayMultiplier = serializeRepeatNumericValue(settings.delayMultiplier); | ||
|
|
||
| return { | ||
| from: settings.from, | ||
| to: settings.to, | ||
| ...(repeatRange !== undefined && { repeatRange }), | ||
| ...(repeatEachVerse !== undefined && { repeatEachVerse }), | ||
| ...(delayMultiplier !== undefined && { delayMultiplier }), | ||
| }; | ||
| }; | ||
|
|
||
| const serializeOptionalRepeatSettings = ( | ||
| settings?: RepeatSettingsLike, | ||
| ): RepeatSettingsPreference | undefined => | ||
| settings ? serializeRepeatSettings(settings) : undefined; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
De-duplicate serialization logic across layers.
The modal and preferencesMapper both need JSON-safe repeat settings. Extract serialize/prepare utilities to a shared module and reuse.
I can extract a shared serializer under src/utils/audioRepeat/persistence.ts and update both call sites. Want a patch?
Also applies to: 495-513
🤖 Prompt for AI Agents
In src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx around lines
257-275 (and the duplicate logic at 495-513), extract the serialization helpers
into a new shared module src/utils/audioRepeat/persistence.ts and remove the
duplicated implementations here and in the preferencesMapper; implement and
export serializeRepeatSettings and serializeOptionalRepeatSettings from that
module (export any needed types), then replace the local implementations with
imports from the shared module in both the modal and preferencesMapper call
sites, preserving existing behavior and types.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (3)
527-535: Guard against undefined verse keys in mode change.Lines 531-532 may assign
undefinedifselectedVerseKeyis not provided or iffirstVerseKeyInThisChapter/lastVerseKeyInThisChapterare undefined. This could cause issues downstream. Add guards or use the default verse calculation logic.Apply this diff:
const onRepetitionModeChange = (mode: RepetitionMode) => { logValueChange('repitition_mode', repetitionMode, mode); + const singleVerseKey = selectedVerseKey ?? firstVerseKeyInThisChapter ?? makeVerseKey(chapterNumber, 1); + const rangeFromKey = firstVerseKeyInThisChapter ?? makeVerseKey(chapterNumber, 1); + const rangeToKey = lastVerseKeyInThisChapter ?? makeVerseKey(chapterNumber, 1); setVerseRepetition((prevVerseRepetition) => ({ ...prevVerseRepetition, - from: mode === RepetitionMode.Single ? selectedVerseKey : firstVerseKeyInThisChapter, - to: mode === RepetitionMode.Single ? selectedVerseKey : lastVerseKeyInThisChapter, + from: mode === RepetitionMode.Single ? singleVerseKey : rangeFromKey, + to: mode === RepetitionMode.Single ? singleVerseKey : rangeToKey, })); setRepetitionMode(mode); };
542-556: Redundant spread ofrangeobject.Lines 551-555 spread both
rangeandnormalizedRange. SincenormalizedRangeis computed from the candidates (which include range values), spreadingrangefirst is redundant—normalizedRangealready contains the finalfromandtovalues. Simplify:const onRangeChange = (range) => { const isFrom = !!range.from; const logKey = isFrom ? 'repeat_verse_range_from' : 'repeat_verse_range_to'; const oldValue = isFrom ? verseRepetition.from : verseRepetition.to; const newValue = isFrom ? range.from : range.to; logValueChange(logKey, oldValue, newValue); const candidateFrom = range.from ?? verseRepetition.from; const candidateTo = range.to ?? verseRepetition.to; const normalizedRange = normalizeVerseRange(candidateFrom, candidateTo); setVerseRepetition({ ...verseRepetition, - ...range, ...normalizedRange, }); };
1-630: Consider extracting helpers to reduce component size.The file is 630 lines with many helper functions (normalization, validation, serialization). Consider extracting these to separate utility modules:
src/utils/audioRepeat/validation.tsfornormalizeRepeatValue,isVerseKeyInChapter,normalizeVerseRangesrc/utils/audioRepeat/persistence.tsfor serialization helpers (if not already extracted)src/utils/audioRepeat/defaults.tsforbuildDefaultVerseRepetition,getRepeatKeys,getRepeatCyclesThis would improve maintainability and testability while keeping the component focused on UI concerns.
As per coding guidelines (write short functions, single responsibility).
♻️ Duplicate comments (1)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (1)
236-307: Consider whether serialization extraction was completed.Previous review suggested extracting serialization logic to a shared module (e.g.,
src/utils/audioRepeat/persistence.ts) to avoid duplication with preferencesMapper. The comment was marked as addressed, but the logic remains inline here. If the duplication was removed elsewhere but kept here intentionally, this is fine. Otherwise, consider completing the extraction for better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.*
📄 CodeRabbit inference engine (.cursor/rules/bug-handling-with-todo-comments.mdc)
If you encounter a bug in existing code, or the instructions lead to suboptimal or buggy code, add comments starting with "TODO:" outlining the problems.
**/*.*: Utilize Early Returns: Use early returns to avoid nested conditions and improve readability.
Conditional Classes: Prefer conditional classes over ternary operators for class attributes.
**/*.*: Use comments sparingly, and when you do, make them meaningful.
Don't comment on obvious things. Excessive or unclear comments can clutter the codebase and become outdated.
Use comments to convey the 'why' behind specific actions or explain unusual behavior and potential pitfalls.
Provide meaningful information about the function's behavior and explain unusual behavior and potential pitfalls.
**/*.*: Write short functions that only do one thing.
Follow the single responsibility principle (SRP), which means that a function should have one purpose and perform it effectively.
If a function becomes too long or complex, consider breaking it into smaller, more manageable functions.Order functions with those that are composing other functions appearing earlier in the file. For example, if you have a menu with multiple buttons, define the menu function above the buttons.
**/*.*: Always add helpful comments to the code explaining what you are doing.
Never delete old comments, unless they are no longer relevant because the code has been rewritten or deleted.
**/*.*: Choose names for variables, functions, and classes that reflect their purpose and behavior.
A name should tell you why it exists, what it does, and how it is used. If a name requires a comment, then the name does not reveal its intent.
Use specific names that provide a clearer understanding of what the variables represent and how they are used.
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/next-js-conventions.mdc)
**/*.{js,jsx,ts,tsx}: Rely on Next.js Pages Router for state changes.
Minimize 'use client' usage: Prefer server components and Next.js SSR features.
Minimize 'use client' usage: Use 'use client' only for Web API access in small components.
Minimize 'use client' usage: Avoid using 'use client' for data fetching or state management.
**/*.{js,jsx,ts,tsx}: Optimize Web Vitals (LCP, CLS, FID)
Use dynamic loading for non-critical components using @src/components/dls/Spinner/Spinner.tsx
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react-functional-components.mdc)
src/components/**/*.tsx: Always use React functional components with hooks.
Use React.FC for functional components with props.
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use functional components over class components
Keep components small and focused
Extract reusable logic into custom hooks
Use composition over inheritance
Split large components into smaller, focused ones
Follow the Rules of Hooks
Use custom hooks for reusable logic
Use appropriate dependency arrays in useEffect
Implement cleanup in useEffect when needed
Avoid nested hooks
Use useState for local component state
Avoid prop drilling through proper state management
Implement proper memoization (useMemo, useCallback)
Use React.memo for expensive components
Avoid unnecessary re-renders
Implement proper lazy loading
Use proper key props in lists
Profile and optimize render performance
Show appropriate loading and error states
Handle async errors properly
Show user-friendly error messages
Implement proper fallback UI
Log errors appropriately
Handle edge cases gracefully
Use semantic HTML elements
Implement proper ARIA attributes
Ensure keyboard navigation
Handle focus management
Provide proper alt text for images
Use proper imports/exports
Document complex component logic
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Implement proper prop types with TypeScript
**/*.tsx: Prefix interfaces for React props with 'Props' (e.g., ButtonProps)
Implement proper error boundariesReact: use functional components only; extract reusable logic into custom hooks; apply proper memoization (React.memo/useMemo/useCallback)
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use xstate for complex state logic
**/*.{ts,tsx}: TypeScript: prefer interfaces over type aliases for object shapes; avoid any; add explicit return types for public functions
Error handling: define and use custom error types; use async/await correctly; show user-friendly error messages
Use Redux Toolkit for state management and XState for complex state machines
Implement i18n with next-translate in components and utilities
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx,d.ts}: Prefer interfaces over types for object definitions
Use type for unions, intersections, and mapped types
Avoid usingany, preferunknownfor unknown types
Leverage TypeScript's built-in utility types
Use generics for reusable type patterns
Use PascalCase for type names and interfaces
Use camelCase for variables and functions
Use UPPER_CASE for constants
Use descriptive names with auxiliary verbs (e.g., isLoading, hasError)
Use explicit return types for public functions
Use arrow functions for callbacks and methods
Implement proper error handling with custom error types
Use function overloads for complex type scenarios
Prefer async/await over Promises
Use readonly for immutable properties
Leverage discriminated unions for type safety
Use type guards for runtime type checking
Implement proper null checking
Avoid type assertions unless necessary
Create custom error types for domain-specific errors
Use Result types for operations that can fail
Use try-catch blocks with typed catch clauses
Handle Promise rejections properly
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
{src,types}/**/*.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript throughout the codebase with strict configuration
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer React functional components for UI implementation
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
{src,types}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use path aliases @/ for src and @/dls/* for design system imports
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Organize React components by feature and place DLS components under src/components/dls
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Order imports: React first, then external, then internal; alphabetize within groups with blank lines between groups
Naming: camelCase for variables/functions; PascalCase for types, interfaces, and React components; UPPER_CASE for constants
Functions should be <= 30 lines, prefer early returns, single responsibility, and descriptive names
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
**/*.{ts,tsx,js,jsx,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier formatting: single quotes, 2-space indentation, 100-character line width, and trailing commas
Files:
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx
🧬 Code graph analysis (1)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (3)
src/utils/verse.ts (3)
getChapterNumberFromKey(44-45)makeVerseKey(217-227)getVerseNumberFromKey(54-55)src/redux/types/AudioState.ts (2)
JsonNumberString(1-1)RepeatSettingsPreference(3-9)src/redux/slices/AudioPlayer/state.ts (1)
selectAudioPlayerState(12-12)
🔇 Additional comments (4)
src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx (4)
4-6: LGTM: Clean selector aliasing.The import aliasing for
useSelectorfrom both libraries is a clean solution to distinguish XState and Redux selectors throughout the component.
309-358: Well-structured default builder.The function cleanly delegates to helper functions and follows single responsibility principle. The large input interface is justified given the need to aggregate defaults from multiple sources (selection, actor, persisted).
420-459: Large dependency array is justified.The
useMemohas 17 dependencies, which is substantial but necessary given the complex fallback logic that considers selected verse, actor state, and persisted preferences. This is acceptable given the component's requirements, though it suggests high complexity.Consider whether the component could be split to reduce complexity, but this is not a blocking issue.
469-495: Robust validation guards implemented.The function correctly:
- Normalizes the verse range before processing
- Guards against undefined
from/to(lines 473-475)- Validates against NaN values (lines 481-483)
- Removes redundant
Number()wrappers since helpers already return numbersAs per previous feedback (now addressed).
| const normalizeRepeatValue = ( | ||
| value: number | string | null | undefined, | ||
| fallback: number, | ||
| ): number => { | ||
| if (value === null || value === undefined) return fallback; | ||
| if (value === Infinity || value === 'Infinity') return Infinity; | ||
| if (typeof value === 'string') { | ||
| const parsedValue = Number(value); | ||
| return Number.isNaN(parsedValue) ? fallback : parsedValue; | ||
| } | ||
| return value; | ||
| }; |
There was a problem hiding this comment.
Validate against negative values for repeat counters.
The function parses numeric strings without validating they're non-negative. Repeat counters should be positive, but a string like "-5" would be parsed and returned. Add a guard to ensure the result is non-negative for repeat settings.
Apply this diff:
const normalizeRepeatValue = (
value: number | string | null | undefined,
fallback: number,
): number => {
if (value === null || value === undefined) return fallback;
if (value === Infinity || value === 'Infinity') return Infinity;
if (typeof value === 'string') {
const parsedValue = Number(value);
- return Number.isNaN(parsedValue) ? fallback : parsedValue;
+ return Number.isNaN(parsedValue) || parsedValue < 0 ? fallback : parsedValue;
}
- return value;
+ return value < 0 ? fallback : value;
};🤖 Prompt for AI Agents
In src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx around lines
52 to 63, the normalizeRepeatValue function parses numeric strings but doesn't
guard against negative repeat counts; update it so parsed string values and
numeric inputs that are negative return the fallback instead of a negative
number. Specifically, after parsing a string, check if parsedValue is NaN or
parsedValue < 0 and return fallback in those cases; likewise for the numeric
branch ensure the returned value is non-negative (return fallback if value < 0).
Keep existing handling for null/undefined and 'Infinity'/Infinity.
| const onPlayClick = () => { | ||
| logButtonClick('start_repeat_play'); | ||
| onSettingsChangeWithoutDispatch('repeatSettings', verseRepetition, PreferenceGroup.AUDIO, play); | ||
| const normalizedRange = normalizeVerseRange(verseRepetition.from, verseRepetition.to); | ||
| const normalizedSettings = { | ||
| ...verseRepetition, | ||
| ...normalizedRange, | ||
| }; | ||
| setVerseRepetition(normalizedSettings); | ||
| const serializableSettings = serializeRepeatSettings(normalizedSettings); | ||
| const previousRepeatSettings = serializeOptionalRepeatSettings(persistedRepeatSettings); | ||
| onSettingsChange( | ||
| 'repeatSettings', | ||
| prepareRepeatSettingsForApi(serializableSettings), | ||
| setRepeatSettings(serializableSettings), | ||
| setRepeatSettings(previousRepeatSettings), | ||
| PreferenceGroup.AUDIO, | ||
| () => play(normalizedSettings), | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redux persistence appears implemented but verify dispatch.
Line 510 passes setRepeatSettings(serializableSettings) to onSettingsChange, which should dispatch the action to Redux. This addresses the previous comment about Redux not being updated. However, verify that onSettingsChange actually dispatches the action (it should based on the signature, but confirmation would be helpful).
Additionally, this function handles multiple responsibilities (normalize → update state → serialize → persist → play). Consider extracting some logic:
const prepareAndSerializeSettings = (settings: VerseRepetitionState) => {
const normalizedRange = normalizeVerseRange(settings.from, settings.to);
const normalizedSettings = { ...settings, ...normalizedRange };
return {
normalizedSettings,
serializable: serializeRepeatSettings(normalizedSettings),
};
};
const onPlayClick = () => {
logButtonClick('start_repeat_play');
const { normalizedSettings, serializable } = prepareAndSerializeSettings(verseRepetition);
setVerseRepetition(normalizedSettings);
// ... rest of persistence logic
};🤖 Prompt for AI Agents
In src/components/AudioPlayer/RepeatAudioModal/RepeatAudioModal.tsx around lines
497 to 515, verify that onSettingsChange actually dispatches the Redux action
(ensure you pass the action creator or call dispatch inside onSettingsChange
rather than passing the result of a setter), and refactor the
normalize/serialize sequence into a helper (e.g., prepareAndSerializeSettings)
that returns normalizedSettings and serializable settings; then simplify
onPlayClick to call that helper, update local state with
setVerseRepetition(normalizedSettings), and call onSettingsChange with the
proper action/dispatch arguments followed by the play callback.
zonetecde
changed the title
Summary
Fixes #QF-2639 & QF-239
Fix issue where values were not persisted between modal openings
Fix RepeatAudioModal to correctly load settings:
Add safeguard to automatically swap start and end verses when they are in the wrong order
Need PR 764 of BE to works entirely.
Type of change
Test plan
I've made several Playwright tests covering all these cases:
They are included in the following commits:
Checklist
Screenshots or videos
Summary by CodeRabbit
Bug Fixes
Improvements
Chores