fix(web): alert modals not clickable when shown above expo-router modals#5
Conversation
Summary of ChangesHello @leeroybrun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical UI interaction bug on the web platform where alert modals became unresponsive when displayed concurrently with expo-router modals. The fix ensures that alert modals correctly receive pointer events and user interactions by explicitly managing event propagation and pointer event styling, thereby restoring expected functionality and improving user experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughAdds web-specific event handlers and pointer-events styling to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (3)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue on the web where alert modals were not clickable when appearing over an expo-router modal. The fix is implemented by stopping event propagation from the modal container and explicitly setting pointerEvents: 'auto' to ensure the modal can receive interactions. The approach is sound and effectively resolves the described problem. I have one suggestion to improve the TypeScript typings for better code clarity and long-term maintainability.
|
|
||
| // On web, stop events from propagating to expo-router's modal overlay | ||
| // which intercepts clicks when it applies pointer-events: none to body | ||
| const stopPropagation = (e: { stopPropagation: () => void }) => e.stopPropagation(); |
There was a problem hiding this comment.
For better type safety and code clarity, it's recommended to use a more specific type for the event object. Since React is already imported in this file, you can use React.SyntheticEvent as the base type for the event. This is more idiomatic in a React environment than a structural type like { stopPropagation: () => void }.
| const stopPropagation = (e: { stopPropagation: () => void }) => e.stopPropagation(); | |
| const stopPropagation = (e: React.SyntheticEvent) => e.stopPropagation(); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sources/modal/components/BaseModal.tsx (1)
12-17: Consider addingonTouchStartfor mobile web compatibility.The current handlers cover mouse interactions (
onClick,onPointerDown), but mobile web browsers may also dispatch touch events. AddingonTouchStart: stopPropagationwould ensure the fix works consistently on touch-enabled web browsers.Suggested improvement
const stopPropagation = (e: { stopPropagation: () => void }) => e.stopPropagation(); const webEventHandlers = Platform.OS === 'web' - ? { onClick: stopPropagation, onPointerDown: stopPropagation } + ? { onClick: stopPropagation, onPointerDown: stopPropagation, onTouchStart: stopPropagation } : {};
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sources/modal/components/BaseModal.tsx
🔇 Additional comments (2)
sources/modal/components/BaseModal.tsx (2)
67-71: LGTM!Spreading
webEventHandlersat theKeyboardAvoidingViewlevel is appropriate—it wraps the entire modal content and ensures events are stopped at the outermost container.
111-114: LGTM!The
Platform.selectapproach withas constis idiomatic for applying web-specific styles, and the inline comment clearly explains the rationale. This correctly ensures the modal container can receive pointer events when expo-router setspointer-events: noneon the body.
On web, when an alert modal is shown while an expo-router modal (like "new session") is open, the alert's OK button was unclickable. This happened because expo-router sets pointer-events: none on body and pointer-events: auto on its own overlay, which intercepted clicks meant for our alert modal. Fix: - Add pointerEvents: 'auto' to modal container to enable interactions - Stop event propagation on web to prevent clicks from reaching expo-router's overlay
leeroybrun
force-pushed
the
slopus/pr/fix-modal-button-click
branch
from
ef1856c to
6c428d7
Compare
fix: correctly escape Windows paths for project ids
1 participant
On web, when an alert modal is shown while an expo-router modal (like "new session") is open, the alert's OK button was unclickable. This happened because expo-router sets pointer-events: none on body and pointer-events: auto on its own overlay, which intercepted clicks meant for our alert modal.
Fix:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.