Skip to content

Conversation

@al-imam
Copy link
Collaborator

@al-imam al-imam commented Nov 12, 2025

Summary

Fixes: QF-3390

This PR enhances the Quran Reader by ensuring that when the user clicks the Next Ayah or Previous Ayah buttons in the audio player, the reader automatically scrolls to the corresponding ayah in both Reading View and Translation View.

Additionally, this PR fixes potential race conditions in the Reading View that occurred when multiple asynchronous verse fetches overlapped. Now, stale fetch requests are safely aborted to prevent inconsistent scrolling behavior.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Test plan

  1. Go to any Chapter page (Reading or Translation view).
  2. Play audio for the current surah.
  3. Scroll manually somewhere else on the page.
  4. Click Next Ayah or Previous Ayah in the audio player.
  5. Verify that the reader automatically scrolls to the correct ayah currently being played.
  6. (Reading View only) Quickly click next/prev multiple times and verify that scrolling remains correct - no flickering or wrong verse jumps occur.

I manually verified the behavior works correctly in both Reading and Translation View In Chapter Page.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Summary by CodeRabbit

  • New Features

    • Improved automatic scrolling to verses from URL or audio controls, now more reliable across reading and translation views.
    • Audio navigation (next/prev) now consistently syncs playback with visible verse.
  • Bug Fixes

    • Prevented stale or overlapping scroll requests to avoid inconsistent scroll positions and race conditions.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Adds abortable, centralized scrolling logic to ReadingView and subscribes both ReadingView and TranslationView hooks to the audio player context to scroll to NEXT_AYAH/PREV_AYAH; includes race-condition guards and starting-verse handling for navigation.

Changes

Cohort / File(s) Summary
ReadingView — Abortable Centralized Scrolling
src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
Adds scrollToVerse helper (useCallback), introduces abortControllerRef and AbortController-based cancellation for page-number fetches, centralizes scrolling logic used for startingVerse and audio-driven navigation, integrates AudioPlayerMachineContext subscription for NEXT_AYAH/PREV_AYAH, and refactors effect dependencies.
TranslationView — Audio Event Subscription
src/components/QuranReader/TranslationView/hooks/useScrollToVirtualizedVerse.ts
Imports AudioPlayerMachineContext and adds a useEffect that subscribes to the audio service (when data type is Chapter) to handle NEXT_AYAH/PREV_AYAH events by calling scrollToBeginningOfVerseCell; preserves existing startingVerse behavior.

Sequence Diagram(s)

sequenceDiagram
    participant AudioPlayer
    participant AudioService as Audio Service (Context)
    participant ReadingHook as ReadingView Hook
    participant TranslationHook as TranslationView Hook
    participant Virtuoso as Virtuoso Ref

    AudioPlayer->>AudioService: emit NEXT_AYAH / PREV_AYAH

    alt ReadingView subscribed
        AudioService->>ReadingHook: NEXT_AYAH / PREV_AYAH event
        ReadingHook->>ReadingHook: abort previous request (AbortController)
        ReadingHook->>ReadingHook: call scrollToVerse(ayah)
        alt verse data available
            ReadingHook->>Virtuoso: sync scroll to verse
        else need page lookup
            ReadingHook->>ReadingHook: fetch page number (with AbortSignal)
            ReadingHook->>Virtuoso: async scroll to verse
        end
    end

    alt TranslationView subscribed
        AudioService->>TranslationHook: NEXT_AYAH / PREV_AYAH event
        TranslationHook->>TranslationHook: extract ayah from audio state
        TranslationHook->>Virtuoso: scroll to beginning of verse cell
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Review AbortController setup, cancellation, and cleanup in ReadingView.
  • Validate race-condition handling when rapidly switching ayahs.
  • Check AudioPlayerMachineContext subscription lifecycle and cleanup in both hooks.
  • Inspect effect dependency arrays for stale closures.

Suggested reviewers

  • osamasayed

Poem

🐰 Hopping through lines with a tidy request,

Abort signals whisper, put races to rest.
Audio calls out, the verses align,
Scrolls find their home, synchronous and fine.
A rabbit applauds this smooth little quest.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding audio player scroll functionality to navigate to Next/Prev ayah in the Quran Reader.
Description check ✅ Passed The PR description is well-structured with Summary, Type of Change, Test Plan, and Checklist sections; it includes the ticket reference, clear explanation of changes in both views, and practical testing steps.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch QF-3390-audio-player-scroll-to-next-prev-ayah

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@osamasayed osamasayed requested a review from Copilot November 30, 2025 17:13
Copilot finished reviewing on behalf of osamasayed November 30, 2025 17:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds automatic scrolling to the corresponding ayah when users click Next/Previous Ayah buttons in the audio player for both Reading View and Translation View. It also fixes race conditions in Reading View that occurred when multiple verse fetch requests overlapped.

Key changes:

  • Added audio player event subscriptions in both view hooks to trigger scrolling on NEXT_AYAH/PREV_AYAH events
  • Refactored Reading View scroll logic into a reusable scrollToVerse callback
  • Implemented AbortController pattern to prevent race conditions from overlapping fetch requests

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/components/QuranReader/TranslationView/hooks/useScrollToVirtualizedVerse.ts Added audio service subscription to scroll to verses on next/prev ayah button clicks
src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts Refactored scroll logic into reusable callback, added abort controller for race condition prevention, and audio service subscription

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts (3)

168-177: Remove virtuosoRef from the dependency array.

Refs are mutable objects whose .current property can change without triggering re-renders. Including refs in dependency arrays is unnecessary and can cause confusion. The callback will still access the current value via virtuosoRef.current when invoked.

  [
-     virtuosoRef,
      pagesVersesRange,
      isUsingDefaultFont,
      initialData,
      verses,
      chapterId,
      quranReaderStyles,
    ],

191-198: Remove virtuosoRef from effect dependencies.

Same as above - refs should not be in dependency arrays as they don't trigger re-renders.

  }, [
    isPagesLookupLoading,
    quranReaderDataType,
    startingVerse,
-   virtuosoRef,
    scrollToVerse,
    pagesVersesRange,
  ]);

163-166: Distinguish AbortError from other errors.

The catch block silently ignores all errors. While ignoring AbortError is correct, other errors (network failures, server errors) are also silently swallowed, making debugging difficult.

       .catch(() => {
-        // Ignore errors
+        .catch((error) => {
+          // Ignore AbortError (expected when canceling stale requests)
+          if (error?.name === 'AbortError') return;
+          // eslint-disable-next-line no-console
+          console.error('Failed to fetch verse page number:', error);
       });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36e33c5 and 40f50d4.

📒 Files selected for processing (1)
  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use TypeScript with strict configuration
Use path alias @/ for src/ and @/dls/* for design system components

**/*.{ts,tsx}: Flag usage of any type in TypeScript - prefer unknown or specific types
Require explicit return types for public/exported functions in TypeScript
Prefer interface over type for object definitions in TypeScript
Use type for unions, intersections, and mapped types in TypeScript
Require enums for repeated raw values of the same category in TypeScript

**/*.{ts,tsx}: Flag any usage of any type - suggest unknown or specific types instead
Require explicit return types for all exported/public functions
Flag implicit any in function parameters
Ensure strict null checks are handled properly
Prefer interface for object shape definitions
Use type only for unions, intersections, and mapped types
Flag type usage where interface is more appropriate
Require enums when the same raw values appear multiple times
Flag repeated string/number literals that should be enums
Use PascalCase for types, interfaces, enums, and components
Use camelCase for variables, functions, and methods
Use UPPER_SNAKE_CASE for constants
Prefix Props interfaces with component name: ButtonProps, ModalProps
Flag unhandled Promise rejections
Require try-catch blocks for async operations
Ensure error types are properly typed, not any
Flag unused imports
Ensure imports are alphabetized within groups
Group import order: React → External packages → Internal modules → Types

**/*.{ts,tsx}: Flag any any types - use unknown or specific types instead
Require explicit return types on exported functions
Use interfaces for object shapes, types for unions in TypeScript
Use enums for repeated categorical values in TypeScript
Functions should be under 30 lines
Flag duplicated code - extract to reusable functions (DRY principle)
Require proper error handling with meaningful fallbacks
Flag unused imports, variables, or dead code
Comments shou...

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
src/components/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/components/**/*.{ts,tsx}: Use React 18 functional components
Organize React components by feature in src/components/
Use TypeScript interfaces for props in functional components

src/components/**/*.{ts,tsx}: Use next-translate with t() for all user-facing text
Use Radix UI primitives for UI components
Use semantic HTML and ARIA attributes for accessibility
Use functional components only (no class components)

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use API utilities from src/utils/auth/api.ts for authenticated requests

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
**/*.*

📄 CodeRabbit inference engine (.cursor/rules/bug-handling-with-todo-comments.mdc)

Add comments starting with 'TODO:' to outline problems or bugs encountered in existing code

**/*.*: Utilize Early Returns: Use early returns to avoid nested conditions and improve readability.
Prefer conditional classes over ternary operators for class attributes.

**/*.*: Use comments sparingly and make them meaningful when you do use them
Don't comment on obvious things; avoid excessive or unclear comments that clutter the codebase and become outdated
Use comments to convey the 'why' behind specific actions and explain unusual behavior and potential pitfalls
Provide meaningful information about function behavior and explain unusual behavior and potential pitfalls in function comments

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

**/*.*: Only modify sections of the code related to the task at hand
Avoid modifying unrelated pieces of code
Accomplish goals with minimal code changes

**/*.*: 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/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
**/*.{js,ts,jsx,tsx,java,py,cs,cpp,c,go,rb,php}

📄 CodeRabbit inference engine (.cursor/rules/clean-code.mdc)

**/*.{js,ts,jsx,tsx,java,py,cs,cpp,c,go,rb,php}: Replace hard-coded values with named constants and use descriptive constant names that explain the value's purpose
Keep constants at the top of the file or in a dedicated constants file
Variables, functions, and classes should reveal their purpose with meaningful names that explain why something exists and how it's used
Avoid abbreviations in names unless they're universally understood
Use comments to explain why something is done a certain way, not what the code does - make the code self-documenting
Document APIs, complex algorithms, and non-obvious side effects in comments
Each function should do exactly one thing and be small and focused
If a function needs a comment to explain what it does, it should be split into smaller, more focused functions
Extract repeated code into reusable functions and share common logic through proper abstraction
Keep related code together and organize code in a logical hierarchy
Hide implementation details and expose clear interfaces through encapsulation
Move nested conditionals into well-named functions for better readability

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
**/*.{ts,js,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/next-js-conventions.mdc)

**/*.{ts,js,jsx,tsx}: Rely on Next.js Pages Router for state changes
Prioritize Web Vitals (LCP, CLS, FID) in performance optimization
Minimize 'use client' usage - prefer server components and Next.js SSR features
Use 'use client' only for Web API access in small components
Avoid using 'use client' for data fetching or state management

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/performance-optimization-rules.mdc)

Optimize Web Vitals (LCP, CLS, FID) in JavaScript and TypeScript files

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)

**/*.{ts,tsx,d.ts}: Prefer interfaces over types for object definitions in TypeScript
Use type for unions, intersections, and mapped types in TypeScript
Avoid using any, prefer unknown for unknown types in TypeScript
Leverage TypeScript's built-in utility types
Use generics for reusable type patterns in TypeScript
Use PascalCase for type names and interfaces in TypeScript
Use camelCase for variables and functions in TypeScript
Use UPPER_CASE for constants in TypeScript
Use descriptive names with auxiliary verbs for boolean variables (e.g., isLoading, hasError) in TypeScript
Keep type definitions close to where they're used in TypeScript
Use explicit return types for public functions in TypeScript
Use arrow functions for callbacks and methods in TypeScript
Use function overloads for complex type scenarios in TypeScript
Prefer async/await over Promises in TypeScript
Use readonly for immutable properties in TypeScript
Leverage discriminated unions for type safety in TypeScript
Use type guards for runtime type checking in TypeScript
Implement proper null checking in TypeScript
Avoid type assertions unless necessary in TypeScript
Use try-catch blocks with typed catch clauses in TypeScript
Handle Promise rejections properly in TypeScript

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx}: Flag functions exceeding 30 lines - should be split
Identify duplicated code that should be extracted to reusable functions (DRY principle)
Flag unused imports, variables, or dead code
Require proper error handling with meaningful fallbacks
Flag skipped linting rules without clear justification
Ensure complex logic has inline comments explaining 'why'
Ensure environment variables are used for configuration instead of hardcoded values
Flag missing input validation or sanitization
Flag undocumented complex business logic

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
**/*.{tsx,jsx,ts,js}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{tsx,jsx,ts,js}: Flag hardcoded strings that should be localized
Ensure proper authentication checks for protected routes

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
**/*.{ts,tsx,js,jsx,json,env,yml,yaml}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Flag any hardcoded credentials or secrets

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.github/instructions/security.instructions.md)

**/*.{ts,tsx,js}: Flag ANY hardcoded credentials, API keys, or secrets in TypeScript/JavaScript code
Ensure all sensitive values use environment variables instead of hardcoding
Use NEXT_PUBLIC_ prefix only for client-side variables in environment variable declarations
Flag sensitive data exposed to client-side code
Require validation for all user inputs in TypeScript/JavaScript code
Flag direct usage of user input without sanitization, particularly with dangerouslySetInnerHTML
Ensure proper encoding for dynamic content to prevent injection attacks
Flag protected routes without auth checks in Next.js applications
Ensure proper session validation for authenticated requests
Flag sensitive operations without authentication checks

Files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
🧠 Learnings (25)
📓 Common learnings
Learnt from: zonetecde
Repo: quran/quran.com-frontend-next PR: 2523
File: src/components/QuranicCalendar/MyProgress/MonthCard.tsx:48-49
Timestamp: 2025-10-09T11:21:33.919Z
Learning: In the Quran in a Year calendar feature (src/components/QuranicCalendar/MyProgress/MonthCard.tsx), instant scrolling (window.scrollTo({ top: 0 }) without smooth behavior) is preferred over smooth scrolling when selecting a week. This is because instant scrolling provides immediate visual feedback of the verses and date changing, whereas smooth scrolling makes it look like nothing is changing during the animation.
📚 Learning: 2025-10-09T11:21:33.919Z
Learnt from: zonetecde
Repo: quran/quran.com-frontend-next PR: 2523
File: src/components/QuranicCalendar/MyProgress/MonthCard.tsx:48-49
Timestamp: 2025-10-09T11:21:33.919Z
Learning: In the Quran in a Year calendar feature (src/components/QuranicCalendar/MyProgress/MonthCard.tsx), instant scrolling (window.scrollTo({ top: 0 }) without smooth behavior) is preferred over smooth scrolling when selecting a week. This is because instant scrolling provides immediate visual feedback of the verses and date changing, whereas smooth scrolling makes it look like nothing is changing during the animation.

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-10-19T11:34:07.609Z
Learnt from: afifvdin
Repo: quran/quran.com-frontend-next PR: 2540
File: src/components/Verse/PlainVerseText/index.tsx:4-4
Timestamp: 2025-10-19T11:34:07.609Z
Learning: In src/components/Verse/PlainVerseText/index.tsx, the translation for verse titles should be wired directly to the component using next-translate rather than passed as props from parent components, as this ensures the wording from locales is centralized and only needs to be changed in one place.

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-10-26T10:22:52.381Z
Learnt from: afifvdin
Repo: quran/quran.com-frontend-next PR: 2540
File: src/components/QuranReader/TranslationView/TranslationText/TranslationAndReference.tsx:60-60
Timestamp: 2025-10-26T10:22:52.381Z
Learning: In src/components/QuranReader/TranslationView/TranslationText/TranslationAndReference.tsx, verse references displayed alongside chapter names should use English/Western numerals only, not localized numerals, as per design specifications.

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-25T13:21:18.501Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-11-25T13:21:18.501Z
Learning: Applies to **/*.{ts,tsx,d.ts} : Handle Promise rejections properly in TypeScript

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:29:33.962Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T12:29:33.962Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Require proper error handling with meaningful fallbacks

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:30:38.634Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-28T12:30:38.634Z
Learning: Applies to **/*.{ts,tsx} : Flag unhandled Promise rejections

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:31:06.687Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T12:31:06.687Z
Learning: Applies to **/*.{ts,tsx} : Handle errors with meaningful fallbacks for async operations

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:30:50.691Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/pre-pr-review.mdc:0-0
Timestamp: 2025-11-28T12:30:50.691Z
Learning: Applies to **/*.{ts,tsx} : Require proper error handling with meaningful fallbacks

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-25T13:20:48.459Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-11-25T13:20:48.459Z
Learning: Applies to **/*.{tsx,jsx} : Handle async errors properly in React components

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:30:38.634Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-28T12:30:38.634Z
Learning: Applies to **/*.{ts,tsx} : Require try-catch blocks for async operations

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:29:33.962Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T12:29:33.962Z
Learning: Applies to **/*.{tsx,jsx} : Ensure proper error states and fallbacks for failed API requests in React

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-25T13:20:48.459Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-11-25T13:20:48.459Z
Learning: Applies to **/*.{tsx,jsx} : Use appropriate dependency arrays in useEffect hooks

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-10-25T12:14:44.062Z
Learnt from: yousefdergham
Repo: quran/quran.com-frontend-next PR: 2547
File: src/pages/learning-plans/[slug]/lessons/[lessonSlugOrId]/index.tsx:58-64
Timestamp: 2025-10-25T12:14:44.062Z
Learning: In quran.com-frontend-next, router.query values (like slug) are not considered reactive dependencies by the project's react-hooks/exhaustive-deps rule and don't need to be included in useEffect dependency arrays. Toast functions from useToast() are also stable and don't need to be in deps.

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:29:33.962Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T12:29:33.962Z
Learning: Applies to **/*.{tsx,jsx} : Flag unnecessary `useEffect` usage in React

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-25T13:20:48.459Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-11-25T13:20:48.459Z
Learning: Applies to **/*.{tsx,jsx} : Avoid unnecessary re-renders in React components

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:30:50.691Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/pre-pr-review.mdc:0-0
Timestamp: 2025-11-28T12:30:50.691Z
Learning: Applies to **/*.tsx : Flag unnecessary useEffect - prefer event handlers or derived state in React components

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:29:33.962Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T12:29:33.962Z
Learning: Applies to **/*.{tsx,jsx} : Flag unnecessary API calls in React - suggest caching with `useSWR`

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:29:33.962Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T12:29:33.962Z
Learning: Applies to **/*.{tsx,jsx} : Ensure proper memoization with `useMemo` and `useCallback` where appropriate in React

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-25T13:20:48.459Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-11-25T13:20:48.459Z
Learning: Applies to **/*.{tsx,jsx} : Use custom hooks for reusable logic in React

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-25T13:20:48.459Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-11-25T13:20:48.459Z
Learning: Applies to **/*.{tsx,jsx} : Implement proper memoization (useMemo, useCallback) in React components

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-25T13:20:48.459Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-11-25T13:20:48.459Z
Learning: Applies to **/*.{tsx,jsx} : Keep React hooks focused and simple

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-25T13:20:48.459Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-11-25T13:20:48.459Z
Learning: Applies to **/*.{tsx,jsx} : Implement cleanup in useEffect when needed in React components

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-28T12:30:50.691Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/pre-pr-review.mdc:0-0
Timestamp: 2025-11-28T12:30:50.691Z
Learning: Applies to **/*.tsx : Use proper memoization (useCallback, useMemo) for callbacks and expensive computations in React components

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
📚 Learning: 2025-11-25T13:20:48.459Z
Learnt from: CR
Repo: quran/quran.com-frontend-next PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-11-25T13:20:48.459Z
Learning: Applies to **/*.{tsx,jsx} : Extract reusable logic into custom hooks in React

Applied to files:

  • src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts
🧬 Code graph analysis (1)
src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts (5)
src/api.ts (1)
  • fetcher (55-89)
src/utils/apiPaths.ts (1)
  • makeVersesFilterUrl (65-66)
src/utils/api.ts (1)
  • getMushafId (65-77)
types/ApiResponses.ts (1)
  • VersesResponse (32-37)
src/xstate/AudioPlayerMachineContext.tsx (1)
  • AudioPlayerMachineContext (17-19)
🔇 Additional comments (1)
src/components/QuranReader/ReadingView/hooks/useScrollToVirtualizedVerse.ts (1)

200-217: LGTM!

The audio player subscription is well-implemented with:

  • Proper cleanup via subscription.unsubscribe()
  • Early return guard when audioService is unavailable or not in Chapter view
  • Validation of ayahNumber before scrolling
  • Correct dependency array (no ref included)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants