fix: Resolve sidebar overlay and interaction issues (#103) #108
Conversation
- Added semi-transparent backdrop overlay when drawer is open - Implemented tap-to-close functionality on backdrop - Added close button () in drawer header - Blocked underlying screen interaction using pointerEvents - Made drawer content scrollable with ScrollView - Enhanced animations with parallel backdrop fade - Improved styling with shadows and proper z-index layering - Fixed TypeScript configuration error Fixes AOSSIE-Org#103
WalkthroughImplements an animated custom drawer with backdrop overlay, pointer-events blocking of underlying content, StatusBar, close controls, and scrollable drawer items; also replaces tsconfig extends with an explicit compilerOptions block and an exclude list. (46 words) Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant AppUI as App UI
participant Drawer as CustomDrawer
participant Anim as Animated API
participant Backdrop as Backdrop Overlay
participant Main as Main Content
rect rgb(220,240,255)
Note over User,AppUI: Open drawer
User->>AppUI: Tap hamburger
AppUI->>Drawer: request open
Drawer->>Anim: Animated.parallel(start translateX in, opacity in)
Anim->>Drawer: slide-in complete
Anim->>Backdrop: fade-in complete
Drawer->>Main: set pointerEvents='none'
Drawer-->>User: Drawer visible
end
rect rgb(255,235,235)
Note over User,Drawer: Close drawer
User->>Backdrop: Tap backdrop
User->>Drawer: Tap close button
Backdrop/Drawer->>Anim: Animated.parallel(start translateX out, opacity out)
Anim->>Drawer: slide-out complete
Anim->>Backdrop: fade-out complete
Drawer->>Main: restore pointerEvents
Drawer-->>User: Drawer closed
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Frontend/src/Screens/CustomDrawer.jsx (1)
53-75: Logout: don’t assume JSON + check HTTP status before branching ondata.error.
await res.json()can throw (empty/non-JSON body), and non-2xx responses should be treated as failures even if JSON parses.const handleLogout = async () => { try { const res = await fetch(`${BASE_URL}/delete_profile`, { method: 'DELETE', }); - - const data = await res.json(); - - if (!data.error) { + let data; + try { + data = await res.json(); + } catch { + data = null; + } + + if (res.ok && data && !data.error) { closeDrawer(); navigation.dispatch( CommonActions.reset({ index: 0, routes: [{ name: 'Onboarding' }], }) ); } else { - Alert.alert("Logout Failed", data.error || "Something went wrong."); + Alert.alert('Logout Failed', (data && data.error) || `Request failed (${res.status}).`); } } catch (error) { Alert.alert("Logout Error", "Unable to logout. Please try again."); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Frontend/src/Screens/CustomDrawer.jsx(3 hunks)Frontend/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Frontend/src/Screens/CustomDrawer.jsx (1)
Frontend/src/Components/HeaderWithBack.jsx (2)
navigation(7-7)styles(20-39)
🔇 Additional comments (1)
Frontend/src/Screens/CustomDrawer.jsx (1)
15-29: Overlay + parallel animations + scrollable drawer: looks solid for the reported interaction bleed-through.
Backdrop layering (zIndex: 999/1000) +pointerEventsgating on main content is a good approach for blocking underlying interaction while keeping the drawer interactive.Also applies to: 77-101, 103-148, 156-167, 168-182
| <TouchableOpacity | ||
| style={styles.backdropTouchable} | ||
| activeOpacity={1} | ||
| onPress={closeDrawer} | ||
| /> | ||
| </Animated.View> | ||
| )} | ||
|
|
||
| {/* Animated Drawer Panel */} | ||
| <Animated.View style={[styles.drawer, { transform: [{ translateX }] }]}> | ||
| <Text style={styles.drawerHeader}>BabyNest</Text> | ||
| <TouchableOpacity onPress={() => navigateTo('Home')} style={styles.link}> | ||
| <Text>Home</Text> | ||
| </TouchableOpacity> | ||
| <TouchableOpacity onPress={() => navigateTo('Tasks')} style={styles.link}> | ||
| <Text>Tasks & AI Recommendations</Text> | ||
| </TouchableOpacity> | ||
| <TouchableOpacity onPress={() => navigateTo('Weight')} style={styles.link}> | ||
| <Text>Weight Tracking</Text> | ||
| </TouchableOpacity> | ||
| <TouchableOpacity onPress={() => navigateTo('Medicine')} style={styles.link}> | ||
| <Text>Medicine Tracking</Text> | ||
| </TouchableOpacity> | ||
| <TouchableOpacity onPress={() => navigateTo('Symptoms')} style={styles.link}> | ||
| <Text>Symptoms Tracking</Text> | ||
| </TouchableOpacity> | ||
| <TouchableOpacity onPress={() => navigateTo('BloodPressure')} style={styles.link}> | ||
| <Text>Blood Pressure Tracking</Text> | ||
| </TouchableOpacity> | ||
| <TouchableOpacity onPress={() => navigateTo('Discharge')} style={styles.link}> | ||
| <Text>Discharge Tracking</Text> | ||
| </TouchableOpacity> | ||
|
|
||
|
|
||
| <View style={styles.logoutContainer}> | ||
| <TouchableOpacity onPress={handleLogout}> | ||
| <Text style={styles.logoutText}>Logout</Text> | ||
| <StatusBar backgroundColor="#fff" barStyle="dark-content" /> | ||
|
|
||
| {/* Drawer Header with Close Button */} | ||
| <View style={styles.drawerHeaderContainer}> | ||
| <Text style={styles.drawerHeader}>BabyNest</Text> | ||
| <TouchableOpacity onPress={closeDrawer} style={styles.closeButton}> | ||
| <Text style={styles.closeButtonText}>✕</Text> | ||
| </TouchableOpacity> |
There was a problem hiding this comment.
A11y: add labels/roles for close/backdrop; keep backdrop out of screen reader focus.
<TouchableOpacity
style={styles.backdropTouchable}
activeOpacity={1}
onPress={closeDrawer}
+ accessible={false}
/>
@@
- <TouchableOpacity onPress={closeDrawer} style={styles.closeButton}>
+ <TouchableOpacity
+ onPress={closeDrawer}
+ style={styles.closeButton}
+ accessibilityRole="button"
+ accessibilityLabel="Close drawer"
+ >
<Text style={styles.closeButtonText}>✕</Text>
</TouchableOpacity>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <TouchableOpacity | |
| style={styles.backdropTouchable} | |
| activeOpacity={1} | |
| onPress={closeDrawer} | |
| /> | |
| </Animated.View> | |
| )} | |
| {/* Animated Drawer Panel */} | |
| <Animated.View style={[styles.drawer, { transform: [{ translateX }] }]}> | |
| <Text style={styles.drawerHeader}>BabyNest</Text> | |
| <TouchableOpacity onPress={() => navigateTo('Home')} style={styles.link}> | |
| <Text>Home</Text> | |
| </TouchableOpacity> | |
| <TouchableOpacity onPress={() => navigateTo('Tasks')} style={styles.link}> | |
| <Text>Tasks & AI Recommendations</Text> | |
| </TouchableOpacity> | |
| <TouchableOpacity onPress={() => navigateTo('Weight')} style={styles.link}> | |
| <Text>Weight Tracking</Text> | |
| </TouchableOpacity> | |
| <TouchableOpacity onPress={() => navigateTo('Medicine')} style={styles.link}> | |
| <Text>Medicine Tracking</Text> | |
| </TouchableOpacity> | |
| <TouchableOpacity onPress={() => navigateTo('Symptoms')} style={styles.link}> | |
| <Text>Symptoms Tracking</Text> | |
| </TouchableOpacity> | |
| <TouchableOpacity onPress={() => navigateTo('BloodPressure')} style={styles.link}> | |
| <Text>Blood Pressure Tracking</Text> | |
| </TouchableOpacity> | |
| <TouchableOpacity onPress={() => navigateTo('Discharge')} style={styles.link}> | |
| <Text>Discharge Tracking</Text> | |
| </TouchableOpacity> | |
| <View style={styles.logoutContainer}> | |
| <TouchableOpacity onPress={handleLogout}> | |
| <Text style={styles.logoutText}>Logout</Text> | |
| <StatusBar backgroundColor="#fff" barStyle="dark-content" /> | |
| {/* Drawer Header with Close Button */} | |
| <View style={styles.drawerHeaderContainer}> | |
| <Text style={styles.drawerHeader}>BabyNest</Text> | |
| <TouchableOpacity onPress={closeDrawer} style={styles.closeButton}> | |
| <Text style={styles.closeButtonText}>✕</Text> | |
| </TouchableOpacity> | |
| <TouchableOpacity | |
| style={styles.backdropTouchable} | |
| activeOpacity={1} | |
| onPress={closeDrawer} | |
| accessible={false} | |
| /> | |
| </Animated.View> | |
| )} | |
| {/* Animated Drawer Panel */} | |
| <Animated.View style={[styles.drawer, { transform: [{ translateX }] }]}> | |
| <StatusBar backgroundColor="#fff" barStyle="dark-content" /> | |
| {/* Drawer Header with Close Button */} | |
| <View style={styles.drawerHeaderContainer}> | |
| <Text style={styles.drawerHeader}>BabyNest</Text> | |
| <TouchableOpacity | |
| onPress={closeDrawer} | |
| style={styles.closeButton} | |
| accessibilityRole="button" | |
| accessibilityLabel="Close drawer" | |
| > | |
| <Text style={styles.closeButtonText}>✕</Text> | |
| </TouchableOpacity> |
🤖 Prompt for AI Agents
In Frontend/src/Screens/CustomDrawer.jsx around lines 95 to 112, the backdrop
and close button lack proper accessibility attributes; hide the backdrop from
screen readers and add explicit labels/roles for the close control: mark the
backdrop TouchableOpacity so it is not accessible (e.g., set accessible={false}
or importantForAccessibility="no-hide-descendants") so it doesn’t receive
screen-reader focus, and on the visible close button add
accessibilityRole="button", a clear accessibilityLabel like "Close drawer", and
an accessibilityHint (e.g., "Closes the navigation drawer") to improve
keyboard/screen-reader interaction.
|
missing proofs |
- Guard setState calls with isMounted ref to prevent memory leaks - Add useEffect cleanup to handle component unmount - Add accessibility labels and roles for screen readers - Fix tsconfig moduleResolution for Metro bundler compatibility - Update lib settings to include es2021 for better type support Co-authored-by: CodeRabbit <coderabbitai@users.noreply.github.com>
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 (1)
Frontend/src/Screens/CustomDrawer.jsx (1)
64-86: Harden logout fetch: checkres.okbeforeres.json().As-is,
await res.json()can throw on non-JSON/error responses and you lose the server’s status context.const res = await fetch(`${BASE_URL}/delete_profile`, { method: 'DELETE', }); - const data = await res.json(); + if (!res.ok) { + throw new Error(`Logout failed: ${res.status}`); + } + const data = await res.json(); if (!data.error) { closeDrawer();
♻️ Duplicate comments (3)
Frontend/tsconfig.json (1)
2-19: Consider re-introducingextendsto avoid drifting from React Native/Metro defaults.You fixed the big one (
"moduleResolution": "bundler"), but maintaining a fully-manualtsconfigis easy to get subtly wrong (notably"module": "commonjs"+libset vstarget: esnext). I’d prefer extending the official RN config and overriding only what you need.{ + "extends": "@react-native/typescript-config/tsconfig.json", "compilerOptions": { "target": "esnext", - "module": "commonjs", - "lib": [ - "es2017", - "es2021" - ], + "lib": ["esnext"], "allowJs": true, "jsx": "react-native", "noEmit": true, "isolatedModules": true, "strict": false, "moduleResolution": "bundler", "allowSyntheticDefaultImports": true, "esModuleInterop": true, "skipLibCheck": true, "resolveJsonModule": true }, "exclude": [ "node_modules", "babel.config.js", "metro.config.js", "jest.config.js" ] }Please verify via a clean typecheck + Metro build after this change (especially if any dependencies rely on ESM
exports).Frontend/src/Screens/CustomDrawer.jsx (2)
16-20: Stop running animations on unmount (don’t just guardsetState).Right now the cleanup only flips
isMounted, but any in-flightAnimated.timingcontinues after unmount.import React, { useRef, useState, useEffect } from 'react'; @@ const translateX = useRef(new Animated.Value(-DRAWER_WIDTH)).current; const backdropOpacity = useRef(new Animated.Value(0)).current; const [isDrawerOpen, setIsDrawerOpen] = useState(false); const isMounted = useRef(true); useEffect(() => { return () => { isMounted.current = false; + translateX.stopAnimation(); + backdropOpacity.stopAnimation(); }; - }, []); + }, [translateX, backdropOpacity]);Also applies to: 40-56
96-115: Backdrop shouldn’t usually be a screen-reader “button”.Since you already have a visible close button in the header, making the full-screen backdrop accessible is typically noisy/confusing for a11y focus.
<TouchableOpacity style={styles.backdropTouchable} activeOpacity={1} onPress={closeDrawer} - accessible={true} - accessibilityRole="button" - accessibilityLabel="Close drawer" + accessible={false} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Frontend/src/Screens/CustomDrawer.jsx(3 hunks)Frontend/tsconfig.json(1 hunks)
🔇 Additional comments (2)
Frontend/tsconfig.json (1)
20-25: Exclude list looks reasonable for config JS files.Frontend/src/Screens/CustomDrawer.jsx (1)
22-38: Overall drawer UX changes (backdrop + scrollable content + close button) look solid.Also applies to: 40-57, 136-167
- Add pointerEvents to drawer view to prevent touch when closed - Fix tsconfig.json to extend official @react-native/typescript-config - Use node moduleResolution instead of bundler for compatibility - Add es2021 lib for modern JavaScript features - Follow React Native best practices for animated overlays This ensures drawer is completely non-interactive when closed and TypeScript configuration follows official React Native standards.
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 (1)
Frontend/src/Screens/CustomDrawer.jsx (1)
64-86: Guard async operations in handleLogout against unmount.The
handleLogoutfunction doesn't checkisMounted.currentbefore callingcloseDrawer()andnavigation.dispatch()after the async fetch completes. If the component unmounts during the API call, these operations could still execute.Apply this diff to add the guard:
const data = await res.json(); if (!data.error) { + if (!isMounted.current) return; closeDrawer(); navigation.dispatch( CommonActions.reset({ index: 0, routes: [{ name: 'Onboarding' }], }) ); } else { + if (!isMounted.current) return; Alert.alert("Logout Failed", data.error || "Something went wrong."); } } catch (error) { + if (!isMounted.current) return; Alert.alert("Logout Error", "Unable to logout. Please try again."); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Frontend/src/Screens/CustomDrawer.jsx(3 hunks)Frontend/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Frontend/tsconfig.json
🔇 Additional comments (4)
Frontend/src/Screens/CustomDrawer.jsx (4)
22-57: LGTM: Animation logic is well-structured.The parallel animations for the drawer and backdrop are properly synchronized, and the state updates are correctly guarded with
isMounted.currentchecks to prevent updates after unmount.
106-113: Backdrop accessibility conflicts with previous review guidance.The previous review recommended setting
accessible={false}on the backdrop TouchableOpacity to keep it out of screen reader focus, since it's purely a visual overlay and users should discover the explicit close button instead. The current implementation setsaccessible={true}with accessibility labels, making the backdrop a focusable element for screen readers.While the current approach isn't incorrect, it creates two separate "Close drawer" actions (backdrop + close button) that screen readers will announce, which may be redundant.
Consider whether you want to follow the previous accessibility recommendation:
<TouchableOpacity style={styles.backdropTouchable} activeOpacity={1} onPress={closeDrawer} - accessible={true} - accessibilityRole="button" - accessibilityLabel="Close drawer" + accessible={false} />This would hide the backdrop from screen readers and keep only the explicit close button as the discoverable close control.
122-170: Well-structured drawer UI with proper accessibility and scrollability.The drawer implementation successfully addresses the PR objectives:
- StatusBar integration for proper status bar appearance
- Header with close button (including accessibility attributes)
- ScrollView for scrollable navigation content
- Clear visual separation between navigation items and logout action
178-260: LGTM: Styling correctly implements layering and visual design.The styles properly implement the z-index layering (backdrop at 999, drawer at 1000) and include appropriate shadows, spacing, and visual hierarchy for the drawer interface.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
Frontend/src/Screens/CustomDrawer.jsx (1)
95-97: A11y: don’t expose the invisible full-screen backdrop touch target to screen readers.
The full-screen backdropTouchableOpacitybeingaccessible={true}can steal focus and present a “Close drawer” element that isn’t visually discoverable. Hide the backdrop from accessibility, and keep the visible close button as the accessible control.- <TouchableOpacity + <TouchableOpacity style={styles.backdropTouchable} activeOpacity={1} onPress={closeDrawer} - accessible={true} - accessibilityRole="button" - accessibilityLabel="Close drawer" + accessible={false} + importantForAccessibility="no" />Also applies to: 100-118, 121-124
Frontend/tsconfig.json (1)
2-20: TypeScript config should extend@react-native/typescript-configand setmoduleResolution: "bundler"for Metro compatibility.The current
"moduleResolution": "node"with a narrow"lib"set can cause path resolution mismatches with Metro. Extending the official RN config and using"bundler"(TypeScript 5+) aligns with Metro's module resolution behavior. Also ensure"module": "esnext"and sync any path aliases acrosstsconfig.json,babel.config.js, andmetro.config.js.{ + "extends": "@react-native/typescript-config", "compilerOptions": { "target": "esnext", - "module": "commonjs", - "lib": [ - "es2017", - "es2021" - ], + "module": "esnext", + "moduleResolution": "bundler", + "lib": ["ESNext"], "allowJs": true, "jsx": "react-native", "noEmit": true, "isolatedModules": true, "strict": false, - "moduleResolution": "node", "allowSyntheticDefaultImports": true, "esModuleInterop": true, "skipLibCheck": true, "resolveJsonModule": true, "forceConsistentCasingInFileNames": true },
🧹 Nitpick comments (3)
Frontend/src/Screens/CustomDrawer.jsx (2)
25-41: Optional: stop in-flight animations before starting a new open/close.
If users rapidly tap open/close, multipleAnimated.parallel(...).start()calls can overlap; considertranslateX.stopAnimation(); backdropOpacity.stopAnimation();at the start ofopenDrawer/closeDrawer.Also applies to: 43-60
67-89: Logout fetch: consider checkingres.okbeforeres.json().
Non-JSON error bodies (or 204/empty) will throw inres.json()and collapse into the generic catch alert.Frontend/tsconfig.json (1)
21-26:excludelist is fine; consider whether you also wantincludefor tighter scope.
Not required, but adding an explicitinclude(e.g.,["src"]) can prevent accidental type-checking of stray files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Frontend/src/Screens/CustomDrawer.jsx(3 hunks)Frontend/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Frontend/src/Screens/CustomDrawer.jsx (1)
Frontend/src/Components/HeaderWithBack.jsx (2)
navigation(7-7)styles(20-39)
🔇 Additional comments (1)
Frontend/src/Screens/CustomDrawer.jsx (1)
16-23: Good animation cleanup on unmount (prevents orphaned animations / setState-after-unmount).
StoppingtranslateX/backdropOpacityand flippingisMounted.currentin the effect cleanup is the right safeguard.
2 participants
Fixes #103
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.