Fix iOS thought capture popup overlap and tap targets#208
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 59 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughFixes UI overlap and interaction issues in the thought capture modal by increasing tap target sizes to 44x44 points, updating color styling to amber tones, and refactoring bottom layout padding to be dynamic based on session state and control visibility. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/StillPointApp/Components/ThoughtCaptureView.swift (1)
23-31:⚠️ Potential issue | 🟠 MajorAdd an explicit accessibility label to the dismiss button.
This control is still icon-only, so assistive tech does not get a meaningful action name. Please add an accessibility label/hint on the button while keeping the larger hit target.
Suggested fix
Button { onDismiss() } label: { Image(systemName: "xmark") .font(.system(size: 12, weight: .medium)) .foregroundStyle(Color(SPColor.fg4)) .frame(width: Self.minTapTarget, height: Self.minTapTarget) .contentShape(Rectangle()) } + .accessibilityLabel("Dismiss thought capture") + .accessibilityHint("Closes the thought capture card without saving")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointApp/Components/ThoughtCaptureView.swift` around lines 23 - 31, The dismiss Button in ThoughtCaptureView is icon-only and needs an explicit accessibility label (and optionally a hint) so assistive tech can announce its purpose; update the Button (the closure that calls onDismiss()) to call accessibilityLabel("Dismiss") (or a localized string) and optionally accessibilityHint("Closes the thought capture view") on the Button or the Image, ensuring you keep the existing frame/contentShape (Self.minTapTarget) and styling so the larger hit target remains intact.
🧹 Nitpick comments (1)
ios/StillPointApp/Views/SessionView.swift (1)
5-8: Prefer measuring the bottom chrome instead of hard-coding reserve heights.These constants now have to stay in sync with
persistentDistractionBar,controlPanel, and safe-area/layout tweaks. Deriving the reserve from the rendered overlay height will make the overlap fix much less fragile.Also applies to: 161-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointApp/Views/SessionView.swift` around lines 5 - 8, Replace the hard-coded bottomOverlayReserveWithControls and bottomOverlayReserveDistractionOnly with a computed reserve derived from the actual rendered overlay height: measure the combined height of persistentDistractionBar and controlPanel (plus safe-area bottom inset) at runtime (e.g., via SwiftUI GeometryReader/PreferenceKey or view.background(GeometryReader:)) and expose that measured value as a computed property or `@State/`@Binding used where the constants are referenced (including the other occurrences around the 161-166 region); ensure the reserve used in layout equals measuredOverlayHeight + safeAreaInsets.bottom so the overlap adjusts automatically when those views or safe-area change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/StillPointApp/Views/SessionView.swift`:
- Around line 336-347: The Capture Button in SessionView calls
vm.openThoughtCapture() but remains tappable when the session is inactive;
update the Button (the view containing vm.openThoughtCapture()) to either hide
it or disable it based on the session state exposed by the view model (e.g.,
vm.isSessionActive or similar property on SessionViewModel). Use a conditional
(if vm.isSessionActive) to remove the Button from the view or apply
.disabled(!vm.isSessionActive) and adjust its visual styling (muted
colors/opacity) so it no longer appears tappable when inactive, ensuring the UI
has no dead action while keeping vm.openThoughtCapture() logic unchanged.
In `@src/components/SessionView.tsx`:
- Around line 193-196: The handler handleOpenThoughtCapture currently finalizes
and opens post-distraction capture even when the session is paused; guard it by
checking the session active state (isActive) at the start of
handleOpenThoughtCapture and bail out if false so
finalizeActiveHold(elapsedRef.current) and setShowPostDistractionCapture(true)
are not called for inactive sits; also ensure any UI control that calls
handleOpenThoughtCapture is disabled/hidden when isActive is false to prevent
invoking the handler from the paused state.
---
Outside diff comments:
In `@ios/StillPointApp/Components/ThoughtCaptureView.swift`:
- Around line 23-31: The dismiss Button in ThoughtCaptureView is icon-only and
needs an explicit accessibility label (and optionally a hint) so assistive tech
can announce its purpose; update the Button (the closure that calls onDismiss())
to call accessibilityLabel("Dismiss") (or a localized string) and optionally
accessibilityHint("Closes the thought capture view") on the Button or the Image,
ensuring you keep the existing frame/contentShape (Self.minTapTarget) and
styling so the larger hit target remains intact.
---
Nitpick comments:
In `@ios/StillPointApp/Views/SessionView.swift`:
- Around line 5-8: Replace the hard-coded bottomOverlayReserveWithControls and
bottomOverlayReserveDistractionOnly with a computed reserve derived from the
actual rendered overlay height: measure the combined height of
persistentDistractionBar and controlPanel (plus safe-area bottom inset) at
runtime (e.g., via SwiftUI GeometryReader/PreferenceKey or
view.background(GeometryReader:)) and expose that measured value as a computed
property or `@State/`@Binding used where the constants are referenced (including
the other occurrences around the 161-166 region); ensure the reserve used in
layout equals measuredOverlayHeight + safeAreaInsets.bottom so the overlap
adjusts automatically when those views or safe-area change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bc0a47af-1695-48e5-82cc-276c327b5e47
📒 Files selected for processing (5)
ios/StillPointApp/Components/ThoughtCaptureView.swiftios/StillPointApp/ViewModels/SessionViewModel.swiftios/StillPointApp/Views/SessionView.swiftsrc/components/BuddySessionRoom.tsxsrc/components/SessionView.tsx
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit da42e2d. Configure here.
Raise thought capture contrast and enforce 44pt tap targets so iOS users can reliably dismiss or save without obscuring active timer content on smaller screens. Made-with: Cursor
auerbachb
force-pushed
the
issue-205-ios-thinking-popup
branch
from
da42e2d to
0297aa2
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Keep thought-capture bottom spacing stable during control auto-hide and disable the paused-state Capture action so the UI no longer exposes a dead tap target. Made-with: Cursor
|
@coderabbitai Follow-up applied on latest head ():\n- disabled + visually dimmed inactive iOS Capture button\n- stabilized thought-capture bottom padding while popup is visible\n- rebased branch so prior web-file finding is now out of PR scope\nPlease re-check this head commit. |
|
@coderabbitai Follow-up applied on latest head commit de02566: disabled+dimmed inactive iOS Capture button, stabilized thought-capture bottom padding while popup is visible, and rebased branch so prior web-file finding is out of scope. Please re-check this head. |
|
✅ Actions performedFull review triggered. |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ios/StillPointApp/Views/SessionView.swift (1)
24-24: Defensively clampcontentHeightto avoid invalid frame sizes.On very constrained heights, Line 24 can go negative and then feed
.frame(height:)at Line 65. Clamping prevents brittle layout edge cases.Suggested patch
- let contentHeight = geo.size.height - bottomOverlayReserve + let contentHeight = max(0, geo.size.height - bottomOverlayReserve)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointApp/Views/SessionView.swift` at line 24, Clamp the computed contentHeight to a safe non-negative range before it is used for layout to prevent negative frame sizes; specifically, when computing contentHeight (the expression using geo.size.height and bottomOverlayReserve in SessionView.swift), replace the raw subtraction with a clamped value (e.g., max(0, ...)) and use that clamped variable wherever .frame(height:) (the frame applied later in SessionView) consumes contentHeight so the view never receives a negative height.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ios/StillPointApp/Views/SessionView.swift`:
- Line 24: Clamp the computed contentHeight to a safe non-negative range before
it is used for layout to prevent negative frame sizes; specifically, when
computing contentHeight (the expression using geo.size.height and
bottomOverlayReserve in SessionView.swift), replace the raw subtraction with a
clamped value (e.g., max(0, ...)) and use that clamped variable wherever
.frame(height:) (the frame applied later in SessionView) consumes contentHeight
so the view never receives a negative height.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b68ecb88-88f9-4374-af3d-91f65bc38771
📒 Files selected for processing (2)
ios/StillPointApp/Components/ThoughtCaptureView.swiftios/StillPointApp/Views/SessionView.swift
1 participant
Closes #205
Summary
Test plan
swift testinios/StillPointSharedMade with Cursor
Summary by CodeRabbit
Bug Fixes
Style
Improvements