QF-2082 - Hide the navigation drawer when not on Quran reading pages. #2546
Conversation
…icking the 'Navigate Quran' button on the main page.
WalkthroughNavbarBody gained route- and hydration-aware sidebar visibility: it uses useRouter and QURAN_READER_ROUTES, reads hydration and sidebar state from Redux, toggles SidebarNavigation visibility on route changes and after hydration, and switches drawer toggles to explicit action creators. Changes
Sequence Diagram(s)sequenceDiagram
participant NB as NavbarBody
participant Router as NextRouter
participant Store as Redux Store
participant UI as SidebarNavigation
NB->>Store: selectIsPersistGateHydrationComplete
NB->>Store: selectIsSidebarNavigationVisible
NB->>Router: get current route (useRouter)
alt Hydration not complete
NB->>UI: do not render SidebarNavigation
else Hydration complete
NB->>Router: is route in QURAN_READER_ROUTES?
alt Quran route
NB->>UI: render SidebarNavigation
else Non-Quran route
NB->>Store: dispatch setIsSidebarNavigationVisible(false)
NB->>UI: do not render SidebarNavigation
end
end
Note over NB,Store: On route change -> re-evaluate and possibly toggle visibility
Note over NB,Store: Drawer toggles use explicit action creators
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Attention points:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (13)**/*.*📄 CodeRabbit inference engine (.cursor/rules/bug-handling-with-todo-comments.mdc)
Files:
**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/next-js-conventions.mdc)
Files:
src/components/**/*.tsx📄 CodeRabbit inference engine (.cursor/rules/react-functional-components.mdc)
Files:
**/*.{tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Files:
**/*.tsx📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Files:
**/*.{ts,tsx,d.ts}📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Files:
{src,types}/**/*.ts?(x)📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.tsx📄 CodeRabbit inference engine (CLAUDE.md)
Files:
{src,types}/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/components/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,scss}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (21)📚 Learning: 2025-08-01T08:50:03.171ZApplied to files:
📚 Learning: 2025-10-25T12:14:44.062ZApplied to files:
📚 Learning: 2025-10-22T20:33:55.176ZApplied to files:
📚 Learning: 2025-10-28T09:59:48.894ZApplied to files:
📚 Learning: 2025-10-22T20:42:09.214ZApplied to files:
📚 Learning: 2025-08-01T08:49:45.154ZApplied to files:
📚 Learning: 2025-08-01T08:51:05.924ZApplied to files:
📚 Learning: 2025-08-01T08:51:05.924ZApplied to files:
📚 Learning: 2025-10-07T08:38:28.343ZApplied to files:
📚 Learning: 2025-10-07T08:37:58.213ZApplied to files:
📚 Learning: 2025-08-01T08:51:05.924ZApplied to files:
📚 Learning: 2025-08-01T08:50:54.518ZApplied to files:
📚 Learning: 2025-08-01T08:50:54.518ZApplied to files:
📚 Learning: 2025-08-01T08:50:54.518ZApplied to files:
📚 Learning: 2025-10-18T11:22:53.139ZApplied to files:
📚 Learning: 2025-08-01T08:50:54.518ZApplied to files:
📚 Learning: 2025-08-01T08:50:54.518ZApplied to files:
📚 Learning: 2025-08-01T08:50:54.518ZApplied to files:
📚 Learning: 2025-08-01T08:50:54.518ZApplied to files:
📚 Learning: 2025-08-01T08:50:22.602ZApplied to files:
📚 Learning: 2025-08-01T08:50:54.518ZApplied to files:
🧬 Code graph analysis (1)src/components/Navbar/NavbarBody/index.tsx (1)
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/Navbar/NavbarBody/index.tsx(4 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/Navbar/NavbarBody/index.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/Navbar/NavbarBody/index.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/Navbar/NavbarBody/index.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/Navbar/NavbarBody/index.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/Navbar/NavbarBody/index.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/Navbar/NavbarBody/index.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/Navbar/NavbarBody/index.tsx
{src,types}/**/*.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript throughout the codebase with strict configuration
Files:
src/components/Navbar/NavbarBody/index.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer React functional components for UI implementation
Files:
src/components/Navbar/NavbarBody/index.tsx
{src,types}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use path aliases @/ for src and @/dls/* for design system imports
Files:
src/components/Navbar/NavbarBody/index.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/Navbar/NavbarBody/index.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/Navbar/NavbarBody/index.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/Navbar/NavbarBody/index.tsx
🔇 Additional comments (2)
src/components/Navbar/NavbarBody/index.tsx (2)
6-6: Prefer typed Redux hooks (useAppDispatch/useAppSelector).Strengthen TS inference and avoid
anydrift by using typed hooks.// src/redux/hooks.ts import { TypedUseSelectorHook, useDispatch, useSelector } from 'react-redux'; import type { RootState, AppDispatch } from './store'; export const useAppDispatch = () => useDispatch<AppDispatch>(); export const useAppSelector: TypedUseSelectorHook<RootState> = useSelector;Then here:
-import { useDispatch, useSelector } from 'react-redux'; +import { useAppDispatch, useAppSelector } from '@/redux/hooks'; @@ - const dispatch = useDispatch(); + const dispatch = useAppDispatch(); @@ - const isSidebarNavigationVisible = useSelector(selectIsSidebarNavigationVisible); - const isPersistHydrationComplete = useSelector(selectIsPersistGateHydrationComplete); + const isSidebarNavigationVisible = useAppSelector(selectIsSidebarNavigationVisible); + const isPersistHydrationComplete = useAppSelector(selectIsPersistGateHydrationComplete);Also applies to: 63-67
52-59: Centralize reader-route detection to avoid drift and improve clarity.Hardcoding the set risks omissions (future routes). Encapsulate in a routing util and reuse app-wide.
// src/utils/routing.ts export const QURAN_READER_ROUTES = new Set([ '/[chapterId]', '/[chapterId]/[verseId]', '/hizb/[hizbId]', '/juz/[juzId]', '/page/[pageId]', '/rub/[rubId]', ]); export const isQuranReaderRoutePathname = (pathname: string) => QURAN_READER_ROUTES.has(pathname);Then:
-const isQuranReaderRoute = QURAN_READER_ROUTES.has(router.pathname); +import { isQuranReaderRoutePathname } from '@/utils/routing'; +const isQuranReaderRoute = isQuranReaderRoutePathname(router.pathname);Based on coding guidelines (SRP, DRY, descriptive names).
Also applies to: 65-65
| const shouldRenderSidebarNavigation = | ||
| isQuranReaderRoute || isSidebarNavigationVisible || hasResetSidebarAfterHydration.current; | ||
|
|
There was a problem hiding this comment.
Bug: SidebarNavigation mounts on non-reader routes after hydration.
hasResetSidebarAfterHydration.current in the OR makes shouldRenderSidebarNavigation true post-hydration on every route, so the sidebar component mounts on home/media pages (even if closed). This contradicts the PR goal and adds unnecessary work.
Fix by removing the ref/second effect and deriving render strictly from reader routes; re-run the single effect after hydration via dependency:
@@
- const isSidebarNavigationVisible = useSelector(selectIsSidebarNavigationVisible);
- const isPersistHydrationComplete = useSelector(selectIsPersistGateHydrationComplete);
- const hasResetSidebarAfterHydration = useRef(false);
+ const isSidebarNavigationVisible = useSelector(selectIsSidebarNavigationVisible);
+ const isPersistHydrationComplete = useSelector(selectIsPersistGateHydrationComplete);
@@
- useEffect(() => {
- if (isQuranReaderRoute) return;
- // Disable the sidebar when not on any Quran reader route
- dispatch({ type: setIsSidebarNavigationVisible.type, payload: false });
- }, [dispatch, isQuranReaderRoute]);
+ useEffect(() => {
+ if (isQuranReaderRoute) return;
+ // Force-hide the sidebar on non-reader routes; runs on mount and after hydration.
+ dispatch({ type: setIsSidebarNavigationVisible.type, payload: false });
+ }, [dispatch, isQuranReaderRoute, isPersistHydrationComplete]);
@@
- const shouldRenderSidebarNavigation =
- isQuranReaderRoute || isSidebarNavigationVisible || hasResetSidebarAfterHydration.current;
+ const shouldRenderSidebarNavigation = isQuranReaderRoute;
@@
- useEffect(() => {
- if (hasResetSidebarAfterHydration.current) return;
- if (!isPersistHydrationComplete) return;
- hasResetSidebarAfterHydration.current = true;
- if (isQuranReaderRoute) return;
- dispatch({ type: setIsSidebarNavigationVisible.type, payload: false });
- }, [dispatch, isPersistHydrationComplete, isQuranReaderRoute]);
+ // Hydration one-off reset removed; unified into the effect above.
@@
- {shouldRenderSidebarNavigation && <SidebarNavigation />}
+ {shouldRenderSidebarNavigation && <SidebarNavigation />}As per coding guidelines (early returns, keep components small).
Also applies to: 79-85, 152-152
🤖 Prompt for AI Agents
In src/components/Navbar/NavbarBody/index.tsx around lines 76-78 (also apply
changes to 79-85 and line 152), remove the hasResetSidebarAfterHydration.current
ref and the secondary effect that sets it; instead compute
shouldRenderSidebarNavigation directly from isQuranReaderRoute ||
isSidebarNavigationVisible so the sidebar only renders on reader routes or when
explicitly visible, and update the existing single effect to depend on the
hydration flag (or relevant hydration state) so it re-runs after hydration
rather than using a ref; also refactor for an early return to keep the component
small.
Summary
Fixes #QF-2082
Changed the logic for displaying the navigation sidebar:
Only downside: if the user who had the navigation sidebar open goes back to a reading page after visiting a non-reading page, the sidebar will remain hidden until they open it again.
Type of change
Test plan
Added a Playwright test to verify the new sidebar behavior on the homepage and re-ran all tests related to the navigation sidebar.
Checklist
Screenshots or videos
Summary by CodeRabbit
Bug Fixes
Improvements