Add native dev-client scripts and fix mobile shell layout#13
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe PR refactors the app's development workflow by introducing platform-specific dev scripts with daemon URL configuration, integrating safe-area layout handling across React Native UI components, enabling ProjectSidebar to hide its bottom navigation in drawer contexts, and adding draft restoration logic to ChatComposer's send failure paths. ChangesDevelopment Runtime Scripts & Configuration
Safe Area Layout System
Chat Composer Draft Restoration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/app/_layout.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. app/components/AppShell.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. app/components/ChatComposer.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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
🧹 Nitpick comments (1)
app/components/MobileTabBar.tsx (1)
23-29: 💤 Low valueConsider whether the iOS minimum inset of 24px is necessary.
The
Math.max(insets.bottom, 24)ensures at least 24px of bottom padding on iOS. While this provides consistent spacing on older devices without a home indicator, it may be unnecessary on modern iOS whereinsets.bottomalready accounts for the home indicator (~34px).If the 24px minimum is intentional for visual consistency, consider adding a comment. Otherwise, the system-provided
insets.bottomshould be sufficient:Suggested simplification
const insets = useSafeAreaInsets(); const bottomInset = - Platform.OS === "web" - ? 0 - : Platform.OS === "ios" - ? Math.max(insets.bottom, 24) - : insets.bottom; + Platform.OS === "web" ? 0 : insets.bottom;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/MobileTabBar.tsx` around lines 23 - 29, The bottomInset calculation in MobileTabBar.tsx currently forces a 24px minimum on iOS via Math.max(insets.bottom, 24); decide whether to keep that legacy minimum or rely on the system inset: either (A) simplify by replacing Math.max(insets.bottom, 24) with insets.bottom so bottomInset uses the system safe area directly, or (B) if the 24px floor is intentional for visual consistency, add an explanatory comment above the bottomInset const referring to the reason and when it can be removed; update the const named bottomInset and references to insets so the change is localized to the bottomInset computation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/TopBar.tsx`:
- Around line 12-14: TopBar currently clamps the iOS safe-area top inset with a
magic number (const in TopBar: topInset calculation using Math.max(insets.top,
54)), which is unexplained and mirrors a similar clamp in MobileTabBar
(Math.max(insets.bottom, 24)); extract the 54 (and the 24) into clearly named
constants (e.g., IOS_MIN_TOP_INSET and IOS_MIN_BOTTOM_INSET) with a short
comment documenting why the value was chosen (notch vs Dynamic Island vs
non-notch testing) and update TopBar.topInset and MobileTabBar to use these
constants so the intent and source of the numbers are explicit and easy to
adjust after device testing.
---
Nitpick comments:
In `@app/components/MobileTabBar.tsx`:
- Around line 23-29: The bottomInset calculation in MobileTabBar.tsx currently
forces a 24px minimum on iOS via Math.max(insets.bottom, 24); decide whether to
keep that legacy minimum or rely on the system inset: either (A) simplify by
replacing Math.max(insets.bottom, 24) with insets.bottom so bottomInset uses the
system safe area directly, or (B) if the 24px floor is intentional for visual
consistency, add an explanatory comment above the bottomInset const referring to
the reason and when it can be removed; update the const named bottomInset and
references to insets so the change is localized to the bottomInset computation.
🪄 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 Plus
Run ID: e1a40bef-533a-43ca-916d-fd64ebe465c9
⛔ Files ignored due to path filters (3)
app/assets/images/adaptive-icon.pngis excluded by!**/*.pngapp/assets/images/favicon.pngis excluded by!**/*.pngapp/assets/images/icon.pngis excluded by!**/*.png
📒 Files selected for processing (12)
AGENTS.mdREADME.mdapp/.env.exampleapp/app/_layout.tsxapp/components/AppShell.tsxapp/components/ChatComposer.tsxapp/components/MobileTabBar.tsxapp/components/ProjectSidebar.tsxapp/components/TopBar.tsxapp/package.jsondocs/dev-runtime.mdpackage.json
TraderSamwise
changed the title
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
1 participant
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
Verification
Summary by CodeRabbit
New Features
Documentation
Chores