Add Instagram-style pinch-to-zoom for media in conversations

[onefovrth]

Summary

• Implements Instagram-style pinch-to-zoom on photos and videos in message bubbles using a "lift and overlay" pattern — media lifts into a window-level overlay with a proportional dark scrim, scales up to 3.5x with rubber-band resistance below 1x, supports simultaneous pan while zoomed, and spring-animates back to its original position on release
• Video playback continues seamlessly during zoom via AVPlayer reparenting
• Unifies photo/video media bubbles with consistent container-based overlays: sender identity (top-left), info badge (top-right), reactions (bottom-left), and top gradient
• Media overlays follow swipe-to-reply gesture with animated spring return, and corners animate from sharp to 20pt during swipe
• Play button now appears on blurred videos; tapping a blurred video reveals and plays in one action
• Extracts debug section in ConversationInfoView to fix pre-existing type-check timeout

Test plan

• Pinch-zoom a photo in a conversation — verify it lifts with scrim, scales smoothly, and spring-animates back on release
• Pinch-zoom while panning — verify simultaneous zoom + pan works
• Pinch-zoom a playing video — verify playback continues during zoom
• Try to pinch-zoom a blurred photo — verify it does nothing
• Verify long-press, double-tap, swipe-to-reply, and scroll all still work normally
• Verify media reappears in the message list after snap-back animation completes

🤖 Generated with Claude Code

Note

Add Instagram-style pinch-to-zoom for media in conversations

• Introduces MediaZoomState, a shared observable class that tracks zoom scale, translation, source frame, and active media reference, passed via SwiftUI environment through the cell hierarchy.
• Adds pinch gesture recognition in GestureOverlayView that initiates, updates, and ends a zoom session, temporarily disabling the scroll view's pan gesture during zoom.
• Renders a full-screen MediaZoomOverlay in MessagesView that animates media from its source frame and shows a scrim based on zoom level.
• Refactors VideoTapAttachmentView and AttachmentPlaceholder to expose image/player references upward for zooming, add avatar/info/reaction overlays on full-bleed attachment cells, and defer blurred video playback until after reveal.
• Adds adjacentToFullBleedAbove/adjacentToFullBleedBelow flags to MessagesGroup so consecutive full-bleed attachment cells use hairline spacing instead of standard padding.

^{Macroscope summarized 9569dad.}

[claude]

Claude finished @onefovrth's task —— View job

---

PR Review Complete

I've reviewed the pinch-to-zoom implementation across all changed files. Here's my comprehensive feedback:

---

🔴 Critical Issues

1. Race Condition in MediaZoomOverlay (High Priority)

File: MediaZoomOverlay.swift:46-53

The dismiss animation completion handler unconditionally calls state.reset(), which can clobber state if a user starts a new zoom while the previous zoom's dismiss animation is still running.

Problem:

.onChange(of: state.isActive) { _, active in
    if !active {
        withAnimation(.spring(duration: 0.35, bounce: 0.2)) {
            // ... animation
        } completion: {
            state.reset()  // ❌ Unconditional - can clear new zoom's state
        }
    }
}

Fix:

} completion: {
    guard !state.isActive else { return }  // ✅ Guard against new zoom
    state.reset()
}

This matches Macroscope's finding. Without this fix, rapid pinch gestures could corrupt state.

---

2. @unchecked Sendable Without Justification (High Priority)

File: MediaZoomState.swift:6

@Observable
final class MediaZoomState: @unchecked Sendable {

MediaZoomState uses @unchecked Sendable but doesn't appear to need it. The class:

• Is only accessed from the main thread (SwiftUI environment)
• Contains mutable properties (UIImage, AVPlayer, etc.)
• Has no synchronization primitives

Issues:

• UIImage and AVPlayer are not thread-safe
• No actor isolation or locks protect the mutable state
• According to CLAUDE.md, the codebase uses @unchecked Sendable in 68 files, but most are for legitimate reasons (actors, database managers, etc.)

Recommendation:

• Remove @unchecked Sendable unless there's a documented threading requirement
• If cross-thread access is needed, use @MainActor isolation instead:

  @Observable @MainActor
  final class MediaZoomState {

---

3. Asymmetric Gesture Recognizer Simultaneity Check (Medium Priority)

File: MessageContextMenuWrapper.swift:575-580

The simultaneity delegate method only checks otherGestureRecognizer but not gestureRecognizer, making it asymmetric:

func gestureRecognizer(
    _ gestureRecognizer: UIGestureRecognizer,
    shouldRecognizeSimultaneouslyWith otherGestureRecognizer: UIGestureRecognizer
) -> Bool {
    if gestureRecognizer is UIPinchGestureRecognizer || otherGestureRecognizer is UIPinchGestureRecognizer {
        if otherGestureRecognizer is UILongPressGestureRecognizer  // ❌ Only checks other
            && (otherGestureRecognizer as? UILongPressGestureRecognizer)?.minimumPressDuration == 0 {
            return true
        }
        return false
    }
    // ...
}

Problem: When UIKit calls this method with arguments swapped (press tracker as gestureRecognizer, pinch as otherGestureRecognizer), it returns false, potentially blocking pinch gestures.

Fix:

if gestureRecognizer is UIPinchGestureRecognizer || otherGestureRecognizer is UIPinchGestureRecognizer {
    // Check both arguments for press tracker
    if let lp = otherGestureRecognizer as? UILongPressGestureRecognizer, lp.minimumPressDuration == 0 {
        return true
    }
    if let lp = gestureRecognizer as? UILongPressGestureRecognizer, lp.minimumPressDuration == 0 {
        return true
    }
    return false
}

---

⚠️ Code Quality Issues

4. Large File Complexity

File: MessagesGroupItemView.swift (+323/-148 lines)

This file is approaching SwiftLint's 625-line limit for type bodies. Consider extracting:

• VideoTapAttachmentView (lines 294-394) to its own file
• AttachmentPlaceholder (lines 409-800) to its own file
• Media overlay components (MediaContainerID, MediaContainerInfo, etc.) to a shared file

Benefit: Improved readability, easier testing, faster compilation.

---

5. Missing Error Handling in Video Playback

File: MessagesGroupItemView.swift:578-579

try? AVAudioSession.sharedInstance().setCategory(.playback)
try? AVAudioSession.sharedInstance().setActive(true)

Silent error suppression with try? can hide failures. According to CLAUDE.md, use Logger.error instead:

do {
    try AVAudioSession.sharedInstance().setCategory(.playback)
    try AVAudioSession.sharedInstance().setActive(true)
} catch {
    Log.error("Failed to configure audio session: \(error)")
}

---

6. Weak Property Naming

File: MediaZoomState.swift:14

var cornerRadius: CGFloat = 0

This property is named cornerRadius but it actually stores the source corner radius for the zoom overlay, not a current corner radius. Better name: sourceCornerRadius or originalCornerRadius.

---

🧪 Test Coverage Issues

7. Zero Test Coverage for New Code

The PR adds ~815 lines of new code but includes no tests. Critical paths that need testing:

MediaZoomState:

• beginZoom / updateZoom / endZoom lifecycle
• Rubber-band scale clamping (< 1.0 and > 3.5)
• Scrim alpha calculation

Gesture Coordination:

• Pinch → zoom activation
• Pan + pinch simultaneous gestures
• Gesture cancellation when context menu opens
• Scroll view disable/enable during pinch

Video Player Continuity:

• Player reparenting during zoom
• Playback state preservation
• Race condition from rapid zoom/dismiss cycles (issue [Screen] Onboarding #1)

Recommendation: Add unit tests for MediaZoomState and integration tests for gesture handling. According to CLAUDE.md, use the test-writer agent proactively after implementing features.

---

🎨 SwiftUI Best Practices

8. Animation Value Dependencies

File: MessagesGroupItemView.swift:490, 621-622

Multiple .animation() modifiers on the same view can conflict:

.animation(.easeOut(duration: 0.25), value: showBlurOverlay)  // Line 490
// ... later in photoContent ...
.animation(.easeOut(duration: 0.25), value: showBlurOverlay)  // Line 621
.animation(.easeOut(duration: 0.15), value: isPressed)        // Line 622

Issue: The second .animation for showBlurOverlay is redundant. Consolidate:

.animation(.easeOut(duration: 0.25), value: showBlurOverlay)
.animation(.easeOut(duration: 0.15), value: isPressed)

---

9. Opacity Toggle Pattern

File: AttachmentPlaceholder.swift:515

.opacity(mediaZoomState.attachmentKey == attachment.key ? 0 : 1)

This hides the source media during zoom, which is correct. However, consider using .hidden() for performance if the view is complex:

.opacity(mediaZoomState.attachmentKey == attachment.key ? 0 : 1)

Actually, opacity is fine here since it maintains layout. No change needed.

---

🔒 Security Considerations

10. No Security Issues Found

• Media content is not exposed outside the sandbox
• No sensitive data logged
• User-controlled zoom gestures don't expose vulnerabilities

---

⚡ Performance Considerations

11. Potential Main Thread Blocking

File: MessagesGroupItemView.swift:625-706

The loadAttachment() function uses async/await correctly, but there's a potential concern:

.task {
    await loadAttachment()
}

This runs on every view appearance. For media in a list, this could cause:

• Redundant loads if cells are reused
• Memory pressure from many simultaneous video loads

Recommendation:

• Ensure ImageCache and VideoURLCache are effective
• Consider adding load cancellation when views disappear
• Monitor memory usage with many video attachments

---

12. Geometry Reader Overhead

File: MediaZoomOverlay.swift:17-30

The overlay uses GeometryReader at the root, which can trigger layout passes:

GeometryReader { proxy in
    let localFrame = localSourceFrame(in: proxy)
    // ...
}

This is acceptable for an overlay that's only visible during zoom. No changes needed.

---

📋 Additional Observations

13. ConversationInfoView Refactor (Positive)

The extraction of the debug section to fix type-check timeout is good:

• Reduces complexity
• Improves compilation performance
• Follows CLAUDE.md guidance on file organization

---

14. Adjacency Tracking Logic (Well-Designed)

File: MessagesListProcessor.swift:214-228

The logic for hairline spacing between full-bleed attachments is clean and maintainable. Good work!

---

🎯 Summary

Blocking Issues (Must Fix):

1. ❌ Race condition in MediaZoomOverlay.swift:52 - guard against new zoom in completion
2. ❌ Review @unchecked Sendable usage - likely unnecessary
3. ⚠️ Asymmetric gesture recognizer check - add symmetric logic

Recommended (Should Fix):
4. Add test coverage for zoom lifecycle and gesture coordination
5. Extract large components from MessagesGroupItemView.swift
6. Replace try? with proper error logging

Nice to Have:
7. Rename cornerRadius to sourceCornerRadius for clarity
8. Remove duplicate animation modifiers

Overall Assessment:
This is a complex, well-structured feature with solid architecture. The lift-and-overlay pattern is clean, and the gesture coordination is sophisticated. However, the lack of tests and race condition bug are concerning for production readiness. Fix the critical issues and add test coverage before merging.

---

Estimated Effort:

• Critical fixes: ~30 minutes
• Test coverage: 2-3 hours
• Refactoring: 1-2 hours

---

[macroscopeapp]

🟠 High MediaZoom/MediaZoomOverlay.swift:46

If a user begins a new zoom while the dismiss animation is still running, the completion handler unconditionally calls state.reset(), which clears sourceImage, sourcePlayer, and attachmentKey for the newly-started zoom. The guard should check !state.isActive before resetting to avoid clobbering the new zoom's state.

                    } completion: {
-                        state.reset()
+                        guard !state.isActive else { return }
+                        state.reset()
                    }

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file Convos/Conversation Detail/Messages/MessagesListView/MediaZoom/MediaZoomOverlay.swift around lines 46-53:

If a user begins a new zoom while the dismiss animation is still running, the completion handler unconditionally calls `state.reset()`, which clears `sourceImage`, `sourcePlayer`, and `attachmentKey` for the newly-started zoom. The guard should check `!state.isActive` before resetting to avoid clobbering the new zoom's state.

Evidence trail:
MediaZoomOverlay.swift lines 43-52 (REVIEWED_COMMIT): `.onChange(of: state.isActive)` handler shows unconditional `state.reset()` call in animation completion at lines 49-51.

MediaZoomState.swift lines 56-61 (REVIEWED_COMMIT): `reset()` function clears `isActive`, `sourceImage`, `sourcePlayer`, `attachmentKey`.

MediaZoomState.swift lines 22-40 (REVIEWED_COMMIT): `beginZoom()` sets all state properties including `isActive = true` at line 39.

[macroscopeapp]

🟢 Low MessagesListView/MessageContextMenuWrapper.swift:570

In gestureRecognizer(_:shouldRecognizeSimultaneouslyWith:) at lines 570-576, the pinch-to-press-tracker simultaneity check is asymmetric: it only returns true when otherGestureRecognizer is the press tracker (UILongPress with minimumPressDuration == 0). When UIKit calls the method with the arguments swapped—press tracker as gestureRecognizer and pinch as otherGestureRecognizer—the check fails and the method returns false, blocking pinch gestures during press tracking. The logic should be symmetric to handle both argument orderings.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file Convos/Conversation Detail/Messages/MessagesListView/MessageContextMenuWrapper.swift around line 570:

In `gestureRecognizer(_:shouldRecognizeSimultaneouslyWith:)` at lines 570-576, the pinch-to-press-tracker simultaneity check is asymmetric: it only returns `true` when `otherGestureRecognizer` is the press tracker (UILongPress with `minimumPressDuration == 0`). When UIKit calls the method with the arguments swapped—press tracker as `gestureRecognizer` and pinch as `otherGestureRecognizer`—the check fails and the method returns `false`, blocking pinch gestures during press tracking. The logic should be symmetric to handle both argument orderings.

Evidence trail:
Convos/Conversation Detail/Messages/MessagesListView/MessageContextMenuWrapper.swift lines 565-586 at REVIEWED_COMMIT. The `gestureRecognizer(_:shouldRecognizeSimultaneouslyWith:)` method checks `otherGestureRecognizer is UILongPressGestureRecognizer` (line 571) but never checks `gestureRecognizer is UILongPressGestureRecognizer`, making the simultaneity check asymmetric.

[macroscopeapp]

Approvability

Verdict: Needs human review

1 blocking correctness issue found. This PR introduces a new user-facing feature (pinch-to-zoom for media) with new state management, gesture recognition, and overlay components. Additionally, an unresolved high-severity comment identifies a potential race condition in the zoom state reset logic that could cause visual glitches.

^{You can customize Macroscope's approvability policy. Learn more.}