fix: add overlay backdrop to sidebar drawer (fixes #103) #113
Conversation
WalkthroughAdds an animated fullscreen overlay/backdrop to CustomDrawer, running drawer translation and overlay opacity animations in parallel, with a fullscreen TouchableOpacity to dismiss the drawer; layout and styles adjusted to render overlay and drawer together and add shadow styling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Frontend/src/Screens/CustomDrawer.jsx(4 hunks)
🔇 Additional comments (4)
Frontend/src/Screens/CustomDrawer.jsx (4)
11-12: LGTM!Both animated values are correctly initialized using
useRefto persist across renders, with appropriate initial values for the hidden state.
14-42: LGTM!Using
Animated.parallelensures synchronized transitions between drawer slide and overlay fade. Native driver usage is correct for transform and opacity properties.
49-71: Verify the endpoint behavior matches the "logout" intent.The function calls
DELETE /delete_profile, which suggests permanent profile deletion rather than a typical logout (session invalidation). If this truly deletes user data, consider:
- Adding a confirmation dialog before the destructive action
- Renaming the function to reflect the actual behavior (e.g.,
handleDeleteAccount)- Or, if this should be logout, using a different endpoint
139-167: LGTM!Style definitions are well-structured. Shadow implementation correctly uses
elevationfor Android andshadow*properties for iOS. ThezIndexordering ensures proper layering (drawer above overlay above content).
| <Animated.View | ||
| style={[ | ||
| styles.overlay, | ||
| { | ||
| opacity: overlayOpacity, | ||
| pointerEvents: overlayOpacity.interpolate({ | ||
| inputRange: [0, 1], | ||
| outputRange: ['none', 'auto'], | ||
| }), | ||
| }, | ||
| ]} | ||
| pointerEvents={overlayOpacity._value > 0 ? 'auto' : 'none'} | ||
| > | ||
| <TouchableOpacity | ||
| style={styles.overlayTouchable} | ||
| activeOpacity={1} | ||
| onPress={closeDrawer} | ||
| /> | ||
| </Animated.View> |
There was a problem hiding this comment.
Fix the pointerEvents handling - current implementation is broken.
Two issues here:
-
Line 87-90:
pointerEventsis not animatable in React Native. The interpolation returns anAnimatedvalue, butpointerEventsexpects a static string ('none'|'auto'). This won't work. -
Line 93: Accessing
overlayOpacity._valueis using a private API that only captures the value at render time, not during animation. This won't update as the animation progresses.
Use a state variable to track drawer open state instead:
+import React, { useRef, useState } from 'react';
-import React, { useRef } from 'react';
...
+ const [isDrawerOpen, setIsDrawerOpen] = useState(false);
const overlayOpacity = useRef(new Animated.Value(0)).current;
const openDrawer = () => {
+ setIsDrawerOpen(true);
Animated.parallel([
...
const closeDrawer = () => {
Animated.parallel([
...
- ]).start();
+ ]).start(() => setIsDrawerOpen(false));
};Then update the overlay:
<Animated.View
style={[
styles.overlay,
- {
- opacity: overlayOpacity,
- pointerEvents: overlayOpacity.interpolate({
- inputRange: [0, 1],
- outputRange: ['none', 'auto'],
- }),
- },
+ { opacity: overlayOpacity },
]}
- pointerEvents={overlayOpacity._value > 0 ? 'auto' : 'none'}
+ pointerEvents={isDrawerOpen ? 'auto' : 'none'}
>📝 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.
| <Animated.View | |
| style={[ | |
| styles.overlay, | |
| { | |
| opacity: overlayOpacity, | |
| pointerEvents: overlayOpacity.interpolate({ | |
| inputRange: [0, 1], | |
| outputRange: ['none', 'auto'], | |
| }), | |
| }, | |
| ]} | |
| pointerEvents={overlayOpacity._value > 0 ? 'auto' : 'none'} | |
| > | |
| <TouchableOpacity | |
| style={styles.overlayTouchable} | |
| activeOpacity={1} | |
| onPress={closeDrawer} | |
| /> | |
| </Animated.View> | |
| <Animated.View | |
| style={[ | |
| styles.overlay, | |
| { opacity: overlayOpacity }, | |
| ]} | |
| pointerEvents={isDrawerOpen ? 'auto' : 'none'} | |
| > | |
| <TouchableOpacity | |
| style={styles.overlayTouchable} | |
| activeOpacity={1} | |
| onPress={closeDrawer} | |
| /> | |
| </Animated.View> |
🤖 Prompt for AI Agents
In Frontend/src/Screens/CustomDrawer.jsx around lines 82 to 100, the overlay's
pointerEvents is incorrectly being set with an Animated interpolation and by
reading overlayOpacity._value; pointerEvents must be a static string and _value
is a private snapshot that won't update during animation. Replace the animated
pointerEvents logic by adding a local state boolean (e.g., isOverlayInteractive)
that you set true when opening animation starts or when opacity animation
reaches visible, and false when closing finishes; keep the Animated.View opacity
animation as-is but remove the pointerEvents interpolation and the _value
access, and bind pointerEvents on the Animated.View (or wrapper) to
isOverlayInteractive ? 'auto' : 'none'; ensure you toggle that state in the
drawer open/close handlers or animation callbacks so pointerEvents reflects the
current interactive state.
…eedback) - Add useState to track drawer open state - Remove animated interpolation for pointerEvents (not supported in RN) - Remove private API access (_value) - Use isDrawerOpen state for static pointerEvents value - Close animation callback now properly sets state
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Frontend/src/Screens/CustomDrawer.jsx (1)
15-29: Parallel animations implemented correctly.The use of
Animated.parallelto synchronize drawer translation and overlay fade-in is good. Both animations useuseNativeDriver: truefor optimal performance.Note:
isDrawerOpenis set totruebefore the animation starts, making the overlay interactive (pointerEvents='auto') even while its opacity is still 0. In practice, this is an acceptable trade-off since the 300ms animation is fast and blocking interactions during the opening animation would prevent users from quickly dismissing the drawer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Frontend/src/Screens/CustomDrawer.jsx(5 hunks)
🔇 Additional comments (5)
Frontend/src/Screens/CustomDrawer.jsx (5)
1-1: Previous review feedback implemented correctly.The addition of
useStateand theisDrawerOpenstate properly addresses the previous review comment aboutpointerEventshandling. This replaces the broken animated interpolation and private_valueaccess with a clean boolean state approach.Also applies to: 12-13
31-44: Close animation properly sequenced.Correctly sets
isDrawerOpentofalsein the animation completion callback. This keeps the overlay interactive (pointerEvents='auto') while it fades out, preventing users from accidentally tapping through to the content behind during the closing animation.
84-96: Overlay implementation correctly fixes previous review issues.The overlay now properly uses the
isDrawerOpenstate to controlpointerEventswith a static string value ('auto'or'none'), completely resolving the previous review comment about:
- Animated interpolation for
pointerEvents(which isn't supported in React Native)- Private
_valueAPI access (which doesn't update during animation)The full-screen
TouchableOpacitycorrectly provides tap-to-dismiss functionality.
136-147: Overlay styles are well-defined.The overlay uses standard absolute positioning to cover the entire screen with appropriate z-index (999, below the drawer's 1000). The semi-transparent black backdrop (
rgba(0, 0, 0, 0.5)) provides good visual separation.
159-162: Cross-platform shadow styling applied correctly.The iOS-specific shadow properties complement the existing Android
elevation, providing appropriate depth cues for the drawer on both platforms.
1 participant
Closes #
📝 Description
🔧 Changes Made
📷 Screenshots or Visual Changes (if applicable)
🤝 Collaboration
Collaborated with:
@username(optional)✅ Checklist
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.