fix(bottom-sheet): stop swallowing the keyboard inset - #17
Conversation
`fillsHeight: true` applied `edgesIgnoringSafeArea(.bottom)`, which — like `ignoresSafeArea(.all)` — covers every safe-area region, `.keyboard` included. That switches off SwiftUI's keyboard avoidance for the whole sheet, so a host with a text field gets its content pinned under the keyboard with no way to scroll to it. The intent was only to let the sheet's background run under the home indicator, which is `.container`. Narrow it to that. dashwallet-ios hit this on the contested-name sheet, where the three action buttons sit outside the ScrollView and became unreachable while typing; it was worked around there with a Done toolbar and interactive scroll-dismiss. Every future sheet with a text field would have needed the same workaround.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe fills-height bottom sheet replaces the deprecated safe-area modifier with ChangesBottom sheet safe-area handling
Estimated code review effort: 2 (Simple) | ~5 minutes Merge Risk: ⚪ Minimal · up to The change restores keyboard avoidance for filled-height bottom sheets while retaining background extension under the home indicator. No actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ 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 review |
✅ Action performedReview finished.
|
2 participants
Issue being fixed or feature implemented
BottomSheet(fillsHeight: true)appliededgesIgnoringSafeArea(.bottom). That API — likeignoresSafeArea(.all, edges:)— covers every safe-area region,.keyboardamong them, so it switches off SwiftUI's keyboard avoidance for the whole sheet.The intent, per the code around it, was only to let the sheet's background run under the home indicator. That is
.container.Any host with a text field inside a filled-height sheet loses keyboard avoidance: content that sits below the scrollable region ends up pinned under the keyboard with no way to reach it. dashwallet-ios hit exactly this on the contested-name sheet, where three action buttons sit outside the
ScrollViewand became unreachable while typing. It was worked around there with a Done keyboard toolbar and interactive scroll-to-dismiss — a workaround every future sheet with a text field would have needed too.What was done?
Narrowed the region to
.container, keeping the home-indicator behaviour and restoring keyboard avoidance.ignoresSafeArea(_:edges:)is iOS 14+, matching both the component's@available(iOS 14, macOS 11, *)and the package floor, so no availability gate is needed.The
fillsHeight: falsebranch is untouched — it deliberately does not ignore the bottom inset, because.presentationDetents([.height])adds that inset itself.How Has This Been Tested?
swift build -c release— clean.swift test— 16 tests, 0 failures.Behavioural verification belongs on the consumer side: a filled-height sheet with a text field should now move with the keyboard, and the sheet background should still run under the home indicator.
Breaking Changes
None in API. Sheets that happened to rely on the keyboard being ignored would now move with it — that is the fix, not a regression.
Checklist:
Summary by CodeRabbit