Conversation
📋 PR Review Helper📱 Mobile App Build✅ Ready to test! (commit 🕶️ ASG Client Build⏳ Waiting for build... 🔀 Test Locallygh pr checkout 2486 |
📝 WalkthroughWalkthroughThis pull request refactors Android navigation bar inset handling across multiple screens by replacing Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
mobile/src/app/miniapps/settings/feedback.tsx (2)
321-321:⚠️ Potential issue | 🔴 CriticalCritical:
goBackWithScreenshotis undefined.
goBackWithScreenshotis called here but was never imported or defined. OnlygoBackis destructured fromuseNavigationHistory()on line 41. This will cause aReferenceErrorat runtime when a feature request submission fails.Suggested fix
onPress: () => { - void goBackWithScreenshot() + goBack() },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/app/miniapps/settings/feedback.tsx` at line 321, The call to goBackWithScreenshot is undefined and will throw at runtime; update the feedback submission error path to either use the existing goBack from useNavigationHistory() or introduce/ import a proper goBackWithScreenshot implementation: locate the useNavigationHistory() destructure where goBack is obtained and either replace the call to goBackWithScreenshot with goBack, or add a goBackWithScreenshot function that captures the screenshot (using your app's screenshot utility) and then calls goBack; ensure the new function is declared or imported in the same module and referenced where goBackWithScreenshot is currently used.
344-344:⚠️ Potential issue | 🔴 CriticalCritical:
goBackWithScreenshotis undefined.Same issue as line 321—
goBackWithScreenshotis not defined. This will crash when the user dismisses the "thank you" alert after submitting feedback.Suggested fix
onPress: () => { - void goBackWithScreenshot() + goBack()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/app/miniapps/settings/feedback.tsx` at line 344, The call to goBackWithScreenshot is undefined and will crash; either implement/export this helper or replace the call with the existing navigation/back helper. Concretely, add a goBackWithScreenshot function (or import it) that captures a screenshot (using the existing screenshot utility) and then calls navigation.goBack()/goBack(), or change the alert handler to call the existing goBack function instead of goBackWithScreenshot so the dismiss path uses a defined symbol.mobile/src/contexts/SaferAreaContext.tsx (1)
23-48:⚠️ Potential issue | 🟡 MinorMissing
themein useMemo dependency array.The memoized
adjustedInsetsusestheme.spacing.s4andtheme.spacing.s6, butthemeis not included in the dependency array. If the theme ever changes at runtime, the insets won't recalculate.Suggested fix
- }, [insets]) + }, [insets, theme])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/contexts/SaferAreaContext.tsx` around lines 23 - 48, The memo for adjustedInsets (created with useMemo) reads theme.spacing.s4 and theme.spacing.s6 but only depends on [insets]; update the dependency array to include theme so the value recalculates when the theme changes (e.g., change useMemo(..., [insets]) to useMemo(..., [insets, theme]) or [insets, theme.spacing] as appropriate), ensuring adjustedInsets picks up runtime theme updates.
🧹 Nitpick comments (8)
mobile/src/components/home/AppSwitcher.tsx (2)
172-172: Remove commented code with typo.This commented line contains a typo (
useSafierAreaInsetsinstead ofuseSaferAreaInsets) and appears to be dead code. Remove it to avoid confusion.🧹 Proposed cleanup
- // const insets = useSafierAreaInsets()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/components/home/AppSwitcher.tsx` at line 172, Remove the dead commented line containing the typo in AppSwitcher.tsx: delete the commented invocation "// const insets = useSafierAreaInsets()" (the incorrect symbol useSafierAreaInsets) so only valid code remains; if you intended to use the safe-area hook, add the correct call to useSaferAreaInsets() in the existing logic instead of leaving the commented typo.
180-180: Investigate the unexplained -4 offset.The comment "idk why we need this -4" indicates incomplete understanding. Magic numbers with uncertain rationale can cause issues when the underlying cause changes. Consider investigating the root cause (possibly border width, padding, or rounding) and documenting it properly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/components/home/AppSwitcher.tsx` at line 180, Investigate and remove the unexplained "-4" magic offset used when computing the card width in AppSwitcher (look for CARD_WIDTH usage in AppSwitcher.tsx), determine whether it’s compensating for borderWidth, padding, margin, or pixel rounding on the container or child elements (including any StyleSheet styles like borderWidth/padding/margin or device pixel ratio math), and replace the hardcoded -4 with a deterministic calculation or constant (e.g., subtract explicit border/padding values or use Math.floor/Math.ceil based on PixelRatio) and add a brief comment explaining the exact reason for the adjustment so future readers understand why that compensation is required.mobile/src/components/miniapps/CapsuleMenu.tsx (1)
106-121: Remove large commented-out code block.This 16-line commented block appears to be superseded by the active implementation below. Keeping dead code reduces readability and can cause confusion during maintenance.
🧹 Proposed cleanup
- // focusEffectPreventBack( - // onBackPress - // ? () => { - // onBackPress() - // } - // : () => { - // // Defer screenshot capture so it doesn't block the navigation animation - // // InteractionManager.runAfterInteractions(() => { - // // let shouldGoBack = Platform.OS === "android" - // // handleExit(shouldGoBack) - // // }) - // let shouldGoBack = Platform.OS === "android" - // handleExit(shouldGoBack) - // }, - // onBackPress ? false : true, - // ) -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/components/miniapps/CapsuleMenu.tsx` around lines 106 - 121, Remove the large 16-line commented-out block related to focusEffectPreventBack in CapsuleMenu.tsx: delete the commented conditional block referencing focusEffectPreventBack, onBackPress, InteractionManager.runAfterInteractions, Platform.OS and handleExit since it is superseded by the active implementation below; ensure no other references to that dead code remain and run a quick lint/format to confirm no leftover comment fragments.mobile/src/components/home/AppSwitcherButtton.tsx (1)
134-136: Remove dead commented code.Lines 134-136 contain commented-out alternatives that appear to be leftover debugging/experimentation code. Consider removing these to improve readability.
🧹 Proposed cleanup
if (Platform.OS === "android") { bottomPadding += theme.spacing.s6 } - // const bottomPadding = theme.spacing.s6 - // const bottomPadding = 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/components/home/AppSwitcherButtton.tsx` around lines 134 - 136, Remove the dead commented debug code in AppSwitcherButtton.tsx by deleting the two commented bottomPadding lines (the "// const bottomPadding = theme.spacing.s6" and "// const bottomPadding = 0") so the component code (e.g., within the AppSwitcherButton component or any surrounding render/stylesheet block) is cleaned up and only the active bottomPadding definition remains.mobile/src/app/applet/webview.tsx (2)
268-274: Remove commented-outscreenshotComponentfunction.This function is commented out and unused. If it's no longer needed as part of the refactor to
MiniAppCapsuleMenu, it should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/app/applet/webview.tsx` around lines 268 - 274, Remove the dead/commented code block defining screenshotComponent: delete the commented-out function and its internals (the use of useAppletStatusStore, packageName lookup, Image return and fallback null). Ensure no other references to screenshotComponent remain (e.g., within MiniAppCapsuleMenu or surrounding components) and run the TypeScript/JSX build to confirm there are no unused symbol warnings.
403-431: Remove commented-out debug code.The "rainbow bars" debugging overlay spans ~30 lines and is entirely commented out. Debug artifacts should be removed rather than committed as comments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/app/applet/webview.tsx` around lines 403 - 431, Remove the large commented-out "rainbow bars" debug JSX block in mobile/src/app/applet/webview.tsx (the commented View/... View elements used for debugging insets/screenshots inside the WebView component) so the file does not contain the ~30-line debug artifact; delete the entire commented section including its opening comment "rainbow bars for debugging insets / screenshots" and the enclosed <View className=...> elements, then run the project's linter/formatter to ensure no leftover whitespace or formatting issues.mobile/src/components/ignite/Screen.tsx (1)
47-51: JSDoc comment is inconsistent with the renamed prop.The documentation describes "Skip the automatic Android 3-button nav bar bottom inset" but the prop is now named
extraAndroidInsets, which implies adding extra insets rather than skipping them.Suggested fix
/** - * Skip the automatic Android 3-button nav bar bottom inset. - * Use when a parent (e.g. tab bar) already handles bottom spacing. + * Add extra bottom padding on Android for screens where buttons + * need additional spacing above the navigation bar. */ extraAndroidInsets?: boolean🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/components/ignite/Screen.tsx` around lines 47 - 51, The JSDoc for the prop is misleading after the rename: update the comment around the extraAndroidInsets prop in Screen (mobile/src/components/ignite/Screen.tsx) to match the prop semantics (i.e., explain that setting extraAndroidInsets true adds extra bottom inset on Android to account for the 3-button nav bar when a parent does NOT already handle bottom spacing), or alternatively rename the prop to something like skipAndroidInsets if you intend the original "skip" behavior—change the JSDoc to reference the chosen semantics and the extraAndroidInsets symbol so the documentation aligns with the prop's behavior.mobile/src/contexts/SaferAreaContext.tsx (1)
30-46: Consider removing commented-out code.The commented-out blocks (lines 30-34 and 43-46) document the old additive padding behavior. If this is intentional for historical reference during the transition, that's fine—but once the refactor is stable, consider removing these to keep the codebase clean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/src/contexts/SaferAreaContext.tsx` around lines 30 - 46, Remove the commented-out legacy additive-padding blocks in SaferAreaContext.tsx around the insets handling (the commented if (insets.top) ... and the commented else-if for insets.bottom <= theme.spacing.s6) to clean up the file; keep the current explicit checks using insets.top === 0 and insets.bottom === 0 and ensure overrides/top and overrides/bottom logic using theme.spacing.s4 and theme.spacing.s6 remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mobile/src/app/miniapps/settings/feedback.tsx`:
- Around line 382-387: resolveScreenshotPackageName() may return null and the
current fallback to "" causes silent failures in saveScreenshot(), broken
rendering in MiniAppMoreActionsSheet and invalid navigation in handlers
(handleAddRemoveFromHome, handleShare, handleSettings); fix by calling
resolveScreenshotPackageName() once into a local variable and if it is
null/undefined return early or render an explicit error/placeholder UI instead
of passing an empty string to MiniAppCapsuleMenu and MiniAppMoreActionsSheet,
and add guards in saveScreenshot(), handleAddRemoveFromHome(), handleShare(),
and handleSettings() to no-op or surface an error when packageName is missing so
no invalid URLs or navigation occur.
In `@mobile/src/components/ignite/Screen.tsx`:
- Line 263: Remove the redundant call "const {theme} = useAppTheme()" and
instead extend the earlier destructuring that already calls useAppTheme() (the
one that currently pulls out colors and themeContext) to also include theme and
spacing so the component reuses that single hook result; update any references
to theme/spacing to use the variables from that initial destructuring and delete
the duplicate hook invocation.
In `@mobile/src/components/miniapps/CapsuleMenu.tsx`:
- Around line 86-87: The code assigns amountToChop = insets.top *
PixelRatio.get() and then immediately overwrites it with 0; remove the dead
assignment by keeping the intended value — either delete the initial calculation
or (preferably) remove the subsequent `amountToChop = 0` so `amountToChop` uses
the computed `insets.top * PixelRatio.get()` in CapsuleMenu.tsx where the
variable is declared.
In `@mobile/src/contexts/NavigationHistoryContext.tsx`:
- Around line 548-554: The beforeRemove listener currently always invokes backFn
from useFocusEffect; change it so the listener is only registered on iOS when
iosDontPreventBack is false. Inside NavigationHistoryContext.tsx wrap the
useFocusEffect/navigation.addListener block with a conditional check
(Platform.OS === "ios" && !iosDontPreventBack), remove iosDontPreventBack from
the effect dependency array so the callback only depends on backFn, and ensure
the cleanup still calls the returned unsubscribe from navigation.addListener.
---
Outside diff comments:
In `@mobile/src/app/miniapps/settings/feedback.tsx`:
- Line 321: The call to goBackWithScreenshot is undefined and will throw at
runtime; update the feedback submission error path to either use the existing
goBack from useNavigationHistory() or introduce/ import a proper
goBackWithScreenshot implementation: locate the useNavigationHistory()
destructure where goBack is obtained and either replace the call to
goBackWithScreenshot with goBack, or add a goBackWithScreenshot function that
captures the screenshot (using your app's screenshot utility) and then calls
goBack; ensure the new function is declared or imported in the same module and
referenced where goBackWithScreenshot is currently used.
- Line 344: The call to goBackWithScreenshot is undefined and will crash; either
implement/export this helper or replace the call with the existing
navigation/back helper. Concretely, add a goBackWithScreenshot function (or
import it) that captures a screenshot (using the existing screenshot utility)
and then calls navigation.goBack()/goBack(), or change the alert handler to call
the existing goBack function instead of goBackWithScreenshot so the dismiss path
uses a defined symbol.
In `@mobile/src/contexts/SaferAreaContext.tsx`:
- Around line 23-48: The memo for adjustedInsets (created with useMemo) reads
theme.spacing.s4 and theme.spacing.s6 but only depends on [insets]; update the
dependency array to include theme so the value recalculates when the theme
changes (e.g., change useMemo(..., [insets]) to useMemo(..., [insets, theme]) or
[insets, theme.spacing] as appropriate), ensuring adjustedInsets picks up
runtime theme updates.
---
Nitpick comments:
In `@mobile/src/app/applet/webview.tsx`:
- Around line 268-274: Remove the dead/commented code block defining
screenshotComponent: delete the commented-out function and its internals (the
use of useAppletStatusStore, packageName lookup, Image return and fallback
null). Ensure no other references to screenshotComponent remain (e.g., within
MiniAppCapsuleMenu or surrounding components) and run the TypeScript/JSX build
to confirm there are no unused symbol warnings.
- Around line 403-431: Remove the large commented-out "rainbow bars" debug JSX
block in mobile/src/app/applet/webview.tsx (the commented View/... View elements
used for debugging insets/screenshots inside the WebView component) so the file
does not contain the ~30-line debug artifact; delete the entire commented
section including its opening comment "rainbow bars for debugging insets /
screenshots" and the enclosed <View className=...> elements, then run the
project's linter/formatter to ensure no leftover whitespace or formatting
issues.
In `@mobile/src/components/home/AppSwitcher.tsx`:
- Line 172: Remove the dead commented line containing the typo in
AppSwitcher.tsx: delete the commented invocation "// const insets =
useSafierAreaInsets()" (the incorrect symbol useSafierAreaInsets) so only valid
code remains; if you intended to use the safe-area hook, add the correct call to
useSaferAreaInsets() in the existing logic instead of leaving the commented
typo.
- Line 180: Investigate and remove the unexplained "-4" magic offset used when
computing the card width in AppSwitcher (look for CARD_WIDTH usage in
AppSwitcher.tsx), determine whether it’s compensating for borderWidth, padding,
margin, or pixel rounding on the container or child elements (including any
StyleSheet styles like borderWidth/padding/margin or device pixel ratio math),
and replace the hardcoded -4 with a deterministic calculation or constant (e.g.,
subtract explicit border/padding values or use Math.floor/Math.ceil based on
PixelRatio) and add a brief comment explaining the exact reason for the
adjustment so future readers understand why that compensation is required.
In `@mobile/src/components/home/AppSwitcherButtton.tsx`:
- Around line 134-136: Remove the dead commented debug code in
AppSwitcherButtton.tsx by deleting the two commented bottomPadding lines (the
"// const bottomPadding = theme.spacing.s6" and "// const bottomPadding = 0") so
the component code (e.g., within the AppSwitcherButton component or any
surrounding render/stylesheet block) is cleaned up and only the active
bottomPadding definition remains.
In `@mobile/src/components/ignite/Screen.tsx`:
- Around line 47-51: The JSDoc for the prop is misleading after the rename:
update the comment around the extraAndroidInsets prop in Screen
(mobile/src/components/ignite/Screen.tsx) to match the prop semantics (i.e.,
explain that setting extraAndroidInsets true adds extra bottom inset on Android
to account for the 3-button nav bar when a parent does NOT already handle bottom
spacing), or alternatively rename the prop to something like skipAndroidInsets
if you intend the original "skip" behavior—change the JSDoc to reference the
chosen semantics and the extraAndroidInsets symbol so the documentation aligns
with the prop's behavior.
In `@mobile/src/components/miniapps/CapsuleMenu.tsx`:
- Around line 106-121: Remove the large 16-line commented-out block related to
focusEffectPreventBack in CapsuleMenu.tsx: delete the commented conditional
block referencing focusEffectPreventBack, onBackPress,
InteractionManager.runAfterInteractions, Platform.OS and handleExit since it is
superseded by the active implementation below; ensure no other references to
that dead code remain and run a quick lint/format to confirm no leftover comment
fragments.
In `@mobile/src/contexts/SaferAreaContext.tsx`:
- Around line 30-46: Remove the commented-out legacy additive-padding blocks in
SaferAreaContext.tsx around the insets handling (the commented if (insets.top)
... and the commented else-if for insets.bottom <= theme.spacing.s6) to clean up
the file; keep the current explicit checks using insets.top === 0 and
insets.bottom === 0 and ensure overrides/top and overrides/bottom logic using
theme.spacing.s4 and theme.spacing.s6 remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 438a7be6-539c-4913-a91f-b128c3c1de4b
📒 Files selected for processing (21)
mobile/src/app/(tabs)/account.tsxmobile/src/app/(tabs)/home.tsxmobile/src/app/(tabs)/store.tsxmobile/src/app/applet/webview.tsxmobile/src/app/asg/gallery.tsxmobile/src/app/index.tsxmobile/src/app/miniapps/settings/feedback.tsxmobile/src/app/onboarding/live.tsxmobile/src/app/onboarding/os.tsxmobile/src/app/pairing/btclassic.tsxmobile/src/app/pairing/prep.tsxmobile/src/app/pairing/select-glasses-model.tsxmobile/src/app/pairing/success.tsxmobile/src/components/glasses/Gallery/GalleryScreen.tsxmobile/src/components/home/AppSwitcher.tsxmobile/src/components/home/AppSwitcherButtton.tsxmobile/src/components/ignite/Screen.tsxmobile/src/components/miniapps/CapsuleMenu.tsxmobile/src/contexts/NavigationHistoryContext.tsxmobile/src/contexts/SaferAreaContext.tsxmobile/src/services/SocketComms.ts
Comment on lines
+382
to
+387
| <> | ||
| <MiniAppCapsuleMenu packageName={resolveScreenshotPackageName() || ""} viewShotRef={viewShotRef} /> | ||
| <Screen preset="fixed" ref={viewShotRef} safeAreaEdges={["top", "bottom"]}> | ||
| <View className="h-12 justify-center"> | ||
| <Text tx="feedback:giveFeedback" className="text-xl text-foreground" /> | ||
| </View> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how MiniAppCapsuleMenu handles empty/null packageName
ast-grep --pattern $'function MiniAppCapsuleMenu($$$) {
$$$
}'
# Also check the component's props interface
rg -n -A 10 'interface.*CapsuleMenu.*Props' --type tsRepository: Mentra-Community/MentraOS
Length of output: 8711
🏁 Script executed:
# Find the saveScreenshot method implementation
rg -n "saveScreenshot" --type ts -B 2 -A 5Repository: Mentra-Community/MentraOS
Length of output: 6308
🏁 Script executed:
# Check MiniAppMoreActionsSheet component for packageName handling
fd "MiniAppMoreActionsSheet" --type fRepository: Mentra-Community/MentraOS
Length of output: 51
🏁 Script executed:
# Look for any validation or conditional logic around packageName in CapsuleMenu
sed -n '42,148p' mobile/src/components/miniapps/CapsuleMenu.tsxRepository: Mentra-Community/MentraOS
Length of output: 3312
🏁 Script executed:
fd "MiniAppMoreActionsSheet" --type tsxRepository: Mentra-Community/MentraOS
Length of output: 239
🏁 Script executed:
rg -n "MiniAppMoreActionsSheet" --type ts -A 20 | head -60Repository: Mentra-Community/MentraOS
Length of output: 2616
🏁 Script executed:
sed -n '153,250p' mobile/src/components/miniapps/CapsuleMenu.tsxRepository: Mentra-Community/MentraOS
Length of output: 4005
🏁 Script executed:
sed -n '215,250p' mobile/src/components/miniapps/CapsuleMenu.tsxRepository: Mentra-Community/MentraOS
Length of output: 1641
Empty string packageName causes silent failures and broken behavior.
When resolveScreenshotPackageName() returns null, the empty string fallback causes:
- Silent screenshot failure —
saveScreenshot()never finds a matching app and silently skips saving - Broken UI —
MiniAppMoreActionsSheetfails to find the app, rendering with missing icon and empty name text - Unhandled edge cases — Other actions like
handleAddRemoveFromHome(),handleShare(), andhandleSettings()proceed with empty packageName, creating invalid URLs and navigating with incomplete data
Consider requiring a valid packageName or returning early if resolveScreenshotPackageName() returns null instead of using an empty string fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/src/app/miniapps/settings/feedback.tsx` around lines 382 - 387,
resolveScreenshotPackageName() may return null and the current fallback to ""
causes silent failures in saveScreenshot(), broken rendering in
MiniAppMoreActionsSheet and invalid navigation in handlers
(handleAddRemoveFromHome, handleShare, handleSettings); fix by calling
resolveScreenshotPackageName() once into a local variable and if it is
null/undefined return early or render an explicit error/placeholder UI instead
of passing an empty string to MiniAppCapsuleMenu and MiniAppMoreActionsSheet,
and add guards in saveScreenshot(), handleAddRemoveFromHome(), handleShare(),
and handleSettings() to no-op or surface an error when packageName is missing so
no invalid URLs or navigation occur.
| ref, | ||
| className, | ||
| } = props | ||
| const {theme} = useAppTheme() |
There was a problem hiding this comment.
Duplicate useAppTheme() call.
useAppTheme() is already destructured on lines 249-251 to get colors and themeContext. This second call on line 263 is redundant.
Suggested fix
Remove the duplicate call and reuse the existing destructuring:
} = props
- const {theme} = useAppTheme()
let $containerInsets = useSafeAreaInsetsStyle(safeAreaEdges, "padding")
// on some screens, we need some extra bottom padding on android so buttons look nice:
const insets = useSaferAreaInsets()
if (Platform.OS === "android" && extraAndroidInsets) {
- $containerInsets = {...$containerInsets, paddingBottom: insets.bottom + theme.spacing.s6}
+ $containerInsets = {...$containerInsets, paddingBottom: insets.bottom + spacing.s6}
}And update the initial destructuring to include spacing:
const {
- theme: {colors},
+ theme: {colors, spacing},
themeContext,
} = useAppTheme()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/src/components/ignite/Screen.tsx` at line 263, Remove the redundant
call "const {theme} = useAppTheme()" and instead extend the earlier
destructuring that already calls useAppTheme() (the one that currently pulls out
colors and themeContext) to also include theme and spacing so the component
reuses that single hook result; update any references to theme/spacing to use
the variables from that initial destructuring and delete the duplicate hook
invocation.
Comment on lines
86
to
+87
| let amountToChop = insets.top * PixelRatio.get() | ||
|
|
||
| amountToChop = 0 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove dead assignment.
amountToChop is calculated from insets.top * PixelRatio.get() on line 86, then immediately overwritten to 0 on line 87. Remove the dead first assignment.
🧹 Proposed fix
- let amountToChop = insets.top * PixelRatio.get()
- amountToChop = 0
+ const amountToChop = 0📝 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.
Suggested change
| let amountToChop = insets.top * PixelRatio.get() | |
| amountToChop = 0 | |
| const amountToChop = 0 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/src/components/miniapps/CapsuleMenu.tsx` around lines 86 - 87, The
code assigns amountToChop = insets.top * PixelRatio.get() and then immediately
overwrites it with 0; remove the dead assignment by keeping the intended value —
either delete the initial calculation or (preferably) remove the subsequent
`amountToChop = 0` so `amountToChop` uses the computed `insets.top *
PixelRatio.get()` in CapsuleMenu.tsx where the variable is declared.
Comment on lines
548
to
+554
| const unsubscribe = navigation.addListener("beforeRemove", (e) => { | ||
| // If forceGestureEnabled is active, the native gesture is handling the removal. | ||
| // Don't call backFn to avoid a double goBack(). | ||
| if (!forceGestureEnabled) { | ||
| backFn?.() | ||
| } | ||
| backFn?.() | ||
| }) | ||
| return () => { | ||
| unsubscribe() | ||
| } | ||
| }, [backFn, iosDontPreventBack, forceGestureEnabled]), | ||
| }, [backFn, iosDontPreventBack]), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all usages of focusEffectPreventBack to understand the impact
echo "=== All focusEffectPreventBack call sites ==="
rg -n "focusEffectPreventBack\(" --type ts --type tsx -A 3
echo ""
echo "=== All setForceGestureEnabled usages ==="
rg -n "setForceGestureEnabled" --type ts --type tsx -B 2 -A 2Repository: Mentra-Community/MentraOS
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Fix file type issue and search for focusEffectPreventBack
echo "=== Looking for focusEffectPreventBack definition and usages ==="
rg -n "focusEffectPreventBack" -A 3 -B 1
echo ""
echo "=== Looking for forceGestureEnabled references ==="
rg -n "forceGestureEnabled" -A 1 -B 1
echo ""
echo "=== Checking NavigationHistoryContext.tsx around line 548 ==="
rg -n "beforeRemove" -A 5 -B 2Repository: Mentra-Community/MentraOS
Length of output: 14836
🏁 Script executed:
#!/bin/bash
# Read the full focusEffectPreventBack function implementation
echo "=== Full focusEffectPreventBack function ==="
sed -n '540,580p' mobile/src/contexts/NavigationHistoryContext.tsx
echo ""
echo "=== Checking the early return logic for iosDontPreventBack ==="
sed -n '540,570p' mobile/src/contexts/NavigationHistoryContext.tsx
echo ""
echo "=== Look for any removal of forceGestureEnabled check in git history ==="
git log -p --follow -S "forceGestureEnabled" -- mobile/src/contexts/NavigationHistoryContext.tsx | head -100Repository: Mentra-Community/MentraOS
Length of output: 2290
🏁 Script executed:
#!/bin/bash
# Check if there's been a recent change to the beforeRemove listener
echo "=== Git history for NavigationHistoryContext.tsx ==="
git log --oneline -20 mobile/src/contexts/NavigationHistoryContext.tsx
echo ""
echo "=== Check for the old forceGestureEnabled check in beforeRemove ==="
git log -p -- mobile/src/contexts/NavigationHistoryContext.tsx | grep -B 5 -A 5 "forceGestureEnabled" | head -50
echo ""
echo "=== Verify AllProviders.tsx gestureEnabled logic ==="
sed -n '159,180p' mobile/src/contexts/AllProviders.tsx
echo ""
echo "=== Verify webview.tsx handleWebViewBack function ==="
rg -n "const handleWebViewBack" -A 10 mobile/src/app/applet/webview.tsxRepository: Mentra-Community/MentraOS
Length of output: 1304
🏁 Script executed:
#!/bin/bash
# Let's trace the complete execution flow when webViewCanGoBack is false on iOS
echo "=== Complete webview.tsx context ==="
sed -n '45,85p' mobile/src/app/applet/webview.tsx
echo ""
echo "=== Check settings.tsx call to focusEffectPreventBack ==="
sed -n '90,105p' mobile/src/app/applet/settings.tsx
echo ""
echo "=== Check NavigationHistoryContext preventBack state usage ==="
rg -n "preventBack" mobile/src/contexts/NavigationHistoryContext.tsx -A 2 -B 1 | head -40Repository: Mentra-Community/MentraOS
Length of output: 3777
🏁 Script executed:
#!/bin/bash
# Check the listener setup - is it conditional or not?
echo "=== Check if beforeRemove listener respects iosDontPreventBack ==="
sed -n '540,565p' mobile/src/contexts/NavigationHistoryContext.tsx | cat -n
echo ""
echo "=== Search for any error reports or issues related to this change ==="
git log --all --oneline --grep="webview\|gesture\|beforeRemove" | head -20
echo ""
echo "=== Check if there are any other screens calling focusEffectPreventBack with backFn that might be affected ==="
rg "focusEffectPreventBack\(" -B 1 -A 2 | grep -v "focusEffectPreventBack()" | grep -v "// focusEffectPreventBack"Repository: Mentra-Community/MentraOS
Length of output: 5771
🏁 Script executed:
#!/bin/bash
# Check if the beforeRemove listener SHOULD be conditional on iosDontPreventBack
echo "=== Analyze the intent: Check the comment vs code ==="
sed -n '540,565p' mobile/src/contexts/NavigationHistoryContext.tsx
echo ""
echo "=== Check recent commits affecting this section ==="
git log -p --follow -n 5 -- mobile/src/contexts/NavigationHistoryContext.tsx | grep -A 20 -B 5 "beforeRemove\|iosDontPreventBack" | head -80
echo ""
echo "=== Verify settings.tsx screenshot behavior with and without native gesture ==="
sed -n '85,110p' mobile/src/app/applet/settings.tsxRepository: Mentra-Community/MentraOS
Length of output: 1858
🏁 Script executed:
#!/bin/bash
# Confirm: does the beforeRemove listener get registered even when iosDontPreventBack is true?
echo "=== The listener registration does NOT check iosDontPreventBack ==="
sed -n '545,565p' mobile/src/contexts/NavigationHistoryContext.tsx | cat -n
echo ""
echo "=== Confirm settings.tsx expectations: backFn should NOT be called when iosDontPreventBack=true ==="
sed -n '94,103p' mobile/src/app/applet/settings.tsx
echo ""
echo "=== Check webview scenario: what happens when forceGestureEnabled=true on iOS ==="
sed -n '73,82p' mobile/src/app/applet/webview.tsxRepository: Mentra-Community/MentraOS
Length of output: 1701
Fix the beforeRemove listener to respect the iosDontPreventBack parameter.
The comment at line 545 states the listener should "skip if iosDontPreventBack — let native gesture handle it," but the code unconditionally registers and executes backFn?.() on all iOS back events, regardless of the iosDontPreventBack flag.
In settings.tsx, iosDontPreventBack=true means the native gesture alone should handle navigation while only capturing the screenshot. However, the beforeRemove listener will still fire saveScreenshot(), contradicting this intent.
The listener registration (lines 548–553) should be conditional:
if (Platform.OS === "ios" && !iosDontPreventBack) {
useFocusEffect(
useCallback(() => {
const unsubscribe = navigation.addListener("beforeRemove", (e) => {
backFn?.()
})
return () => {
unsubscribe()
}
}, [backFn]),
)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/src/contexts/NavigationHistoryContext.tsx` around lines 548 - 554, The
beforeRemove listener currently always invokes backFn from useFocusEffect;
change it so the listener is only registered on iOS when iosDontPreventBack is
false. Inside NavigationHistoryContext.tsx wrap the
useFocusEffect/navigation.addListener block with a conditional check
(Platform.OS === "ios" && !iosDontPreventBack), remove iosDontPreventBack from
the effect dependency array so the callback only depends on backFn, and ensure
the cleanup still calls the returned unsubscribe from navigation.addListener.
This was referenced Apr 16, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
UI/UX Improvements
Chores