fix(ui): polish toast

[pwltr]

Description

• update toast styles to design prototype v59
• fix touches not registered on toast and close button
• add natural drag and drag upwards to dismiss

Screenshot / Video

Simulator.Screen.Recording.-.iPhone.17.-.2025-11-28.at.18.41.32.mov

QA Notes

For debugging I changed in TabBar.swift to

private func onSendPress() {
        app.toast(type: .success, title: "You’re Back Online!", description: "Successfully reconnected to the Internet.")
    }

    private func onReceivePress() {
        app.toast(type: .lightning, title: "Spending Balance Ready", description: "You can now pay anyone, anywhere, instantly.", autoHide: false)
    }

[claude]

PR Review: Toast UI Polish

Overview

This PR enhances the toast notification system with updated styling, improved touch handling, and natural drag-to-dismiss functionality. The changes are well-implemented overall, but I've identified some areas for improvement.

---

✅ Strengths

1. Good SwiftUI Patterns: The implementation follows modern SwiftUI practices with proper state management using @State and gesture handling
2. Improved UX: The drag-to-dismiss with resistance and auto-hide pause/resume creates a polished user experience
3. Accessibility: Added accessibilityLabel for the dismiss button (ToastView.swift:38)
4. Visual Polish: Updated colors and shadows align with design system (opacity 0.32, corner radius 16, proper shadow)

---

🔴 Critical Issues

1. State Reset Logic Bug (ToastView.swift:51-55)

.onChange(of: toast) { _ in
    // Reset drag offset and pause state when toast changes
    dragOffset = 0
    hasPausedAutoHide = false
}

Problem: When toast is a struct with Equatable, this onChange may not trigger reliably if toast properties change but remain equal, or could trigger unexpectedly. The pause state reset here conflicts with the drag gesture's pause management.

Recommendation: Remove this .onChange entirely. The drag gesture already manages hasPausedAutoHide properly in .onEnded. If you need to reset drag offset when a new toast appears, track it via the toast's identity (add an ID field) rather than the entire struct.

---

2. Race Condition in Auto-Hide (ToastWindowManager.swift:38-43)

func hideToast() {
    cancelAutoHide()
    toastWindow?.toastFrame = .zero
    withAnimation(.easeInOut(duration: 0.4)) {
        currentToast = nil
    }
}

Problem: Setting toastFrame = .zero immediately breaks touch interception while the toast is still animating out (0.4s). If a user taps during this animation, unexpected behavior could occur.

Recommendation: Set toastFrame = .zero after the animation completes:

func hideToast() {
    cancelAutoHide()
    withAnimation(.easeInOut(duration: 0.4)) {
        currentToast = nil
    }
    DispatchQueue.main.asyncAfter(deadline: .now() + 0.4) {
        self.toastWindow?.toastFrame = .zero
    }
}

Apply the same fix to scheduleAutoHide (ToastWindowManager.swift:71).

---

3. Memory Leak Risk (ToastWindowManager.swift:70-76)

let workItem = DispatchWorkItem { [weak self] in
    guard let self else { return }
    toastWindow?.toastFrame = .zero
    withAnimation(.easeInOut(duration: 0.4)) {
        self.currentToast = nil
    }
    autoHideStartTime = nil
    autoHideDuration = 0
}

Problem: The DispatchWorkItem is stored in autoHideWorkItem but uses [weak self]. While this prevents a retain cycle, if multiple toasts are shown rapidly, old work items could still fire after being "cancelled" if cancellation happens after dispatch but before execution.

Recommendation: This is actually handled correctly with cancelAutoHide() being called first. However, for extra safety, add a check:

let workItem = DispatchWorkItem { [weak self] in
    guard let self, autoHideWorkItem != nil else { return }
    // ... rest of logic
}

---

⚠️ Moderate Issues

4. Missing MainActor Annotations (ToastWindowManager.swift)

Problem: The class mutates UI state (currentToast, toastWindow) but isn't marked with @MainActor. While many calls happen on main thread via DispatchQueue.main, this isn't enforced at compile time.

Recommendation: Add @MainActor to the class:

@MainActor
class ToastWindowManager: ObservableObject {
    // ...
}

This removes the need for explicit DispatchQueue.main.asyncAfter calls and makes thread safety explicit.

---

5. Accessibility: Missing Gesture Hint (ToastView.swift:58-93)

Problem: VoiceOver users won't know they can swipe up to dismiss the toast.

Recommendation: Add accessibility traits and hint:

.accessibilityAddTraits(.isButton)
.accessibilityHint("Swipe up to dismiss")

---

6. Hardcoded Magic Numbers

Problem: Values like dismissThreshold: 50, dragOffset = translation * 0.08, dragOffset = -200 (ToastView.swift:13,66,84) make the code harder to maintain.

Recommendation: Extract to named constants:

private enum Constants {
    static let dismissThreshold: CGFloat = 50
    static let downwardDragResistance: CGFloat = 0.08
    static let dismissAnimationOffset: CGFloat = -200
}

---

💡 Minor Improvements

7. Inconsistent Access Modifier (ToastWindowManager.swift:8)

Changed toastWindow from private to internal. If this is intentional for testing/debugging, consider adding a comment explaining why. Otherwise, keep it private and expose via a computed property if needed.

---

8. Optional Callback Design

The onDragStart and onDragEnd callbacks are optional but always provided by ToastWindowView. Consider making them non-optional to simplify the API or document why they're optional.

---

9. Performance: Preference Key Updates (ToastWindowView.swift:168-180)

The ToastFramePreferenceKey triggers on every geometry change. Consider throttling updates or only updating when the frame meaningfully changes (e.g., abs(oldFrame.origin.y - newFrame.origin.y) > 1).

---

🧪 Testing Recommendations

1. Test rapid toast succession: Show multiple toasts quickly to ensure auto-hide timers cancel properly
2. Test drag edge cases:
   • Drag down, then up, then release (should dismiss if past threshold)
   • Very fast swipe gestures
   • Multi-finger gestures
3. Test touch pass-through: Ensure touches correctly pass through to underlying window when toast isn't present or touch is outside toast bounds
4. Test accessibility: VoiceOver swipe gestures, dynamic type, increased contrast
5. Test memory: Profile with Instruments to ensure no window/view controller leaks

---

📝 Code Style Notes

✅ Follows SwiftFormat conventions (150 char line width, Swift 5.10)
✅ Proper use of @State and SwiftUI lifecycle
✅ Good separation of concerns (ToastView for UI, ToastWindowManager for lifecycle)

---

🔒 Security Assessment

No security concerns identified. The changes are UI-only and don't interact with sensitive data or Bitcoin/Lightning operations.

---

Summary

Recommendation: Request Changes

The PR implements a nice UX improvement, but the critical issues (state reset logic, race condition in touch handling, missing MainActor) should be addressed before merging. The moderate issues are important for production quality.

Priority fixes:

1. Fix the toastFrame timing race condition
2. Remove/fix the .onChange(of: toast) logic
3. Add @MainActor to ToastWindowManager
4. Add accessibility hints for swipe gestures

Nice work on the drag interaction implementation! 🎨

[claude]

Code Review - PR #254: Polish Toast UI

Great work on improving the toast interaction! The drag-to-dismiss and pause-on-interaction features are solid UX improvements.

Priority Issues to Address

1. Memory Leak Risk (ToastWindowManager.swift:94-108): The check autoHideWorkItem != nil in scheduleAutoHide creates a strong reference cycle. Remove this self-reference check.

2. Race Condition (ToastWindowManager.swift:197-200): onPreferenceChange can fire during animations while hitTest runs on another thread. Wrap updateToastFrame in DispatchQueue.main.async for synchronization.

3. Magic Number (ToastView.swift:70): dragOffset = -200 is hardcoded. Calculate based on screen size instead.

4. Accessibility: VoiceOver users cannot dismiss toasts. Add .accessibilityAction(.dismiss).

Code Quality Improvements

• Missing error handling in setupToastWindow() - fails silently
• Inconsistent state management with hasToast and toastFrame - should be atomic
• SwiftUI anti-pattern using DispatchQueue.main.asyncAfter - use Task sleep instead
• GeometryReader may cause performance issues during animations - consider debouncing
• Large shadow radius may impact drag animation performance

Test Coverage

No unit tests found. Recommend adding tests for:

• Auto-hide timing accuracy
• Pause/resume during drag
• Concurrent toast display
• Memory leak verification
• VoiceOver interactions

Summary

Approve with recommendations. Solid UX improvements overall! The drag interaction feels natural and pause-on-interaction is thoughtful. Please address the memory leak and accessibility issues before merging.

---

Review generated with Claude Code

[claude]

PR Review: Toast UI Polish

Great work on improving the toast UI! The implementation shows attention to detail and adds nice interactive features. Here's my comprehensive review:

---

✅ Strengths

1. Modern SwiftUI Patterns: The code follows modern SwiftUI best practices with proper use of @State, @MainActor, and gesture handling.

2. Smooth UX Improvements:

   • Natural drag-to-dismiss gesture with resistance on downward drag is a nice touch
   • Pause/resume auto-hide during interaction is user-friendly
   • Proper spring animations for snap-back behavior

3. Thread Safety: Good use of @MainActor on ToastWindowManager and proper main thread dispatch in updateToastFrame().

4. Hit Testing Fix: The PassThroughWindow implementation properly handles touch events by tracking toast frame and state.

5. Accessibility: Added accessibilityLabel for the dismiss button (line 35).

---

🔍 Issues & Concerns

1. Breaking Change - Missing id Parameter (CRITICAL)

The Toast initializer now requires an id parameter, but existing call sites use the default. This works fine, but the Equatable conformance might cause issues:

Toast.swift:3

struct Toast: Equatable {
    let id: UUID
    // ...

Since id is a UUID, two Toast instances with identical content will be unequal if they have different IDs. This could break existing equality checks. Consider:

• Making id non-participating in equality: Implement custom Equatable
• Or using Identifiable instead if ID is only for SwiftUI tracking

Recommendation:

struct Toast: Identifiable, Equatable {
    let id: UUID
    // other properties...
    
    static func == (lhs: Toast, rhs: Toast) -> Bool {
        // Compare everything except id
        lhs.type == rhs.type &&
        lhs.title == rhs.title &&
        lhs.description == rhs.description &&
        lhs.autoHide == rhs.autoHide &&
        lhs.visibilityTime == rhs.visibilityTime
    }
}

---

2. Race Condition in updateToastFrame() (MEDIUM)

ToastWindowManager.swift:14

DispatchQueue.main.async { [weak self] in
    guard let self, let window = toastWindow else { return }
    // ...
    window.toastFrame = convertedFrame
}

Since ToastWindowManager is already @MainActor, the DispatchQueue.main.async is redundant and introduces a small delay. The comment mentions thread safety for hitTest, but:

• hitTest runs on the main thread
• The async dispatch doesn't guarantee atomicity—hitTest could still read between writes

Recommendation: Remove the async dispatch since the class is already @MainActor:

func updateToastFrame(globalFrame: CGRect) {
    guard let window = toastWindow else { return }
    let windowOrigin = window.convert(CGPoint.zero, to: nil)
    let convertedFrame = CGRect(
        origin: CGPoint(
            x: globalFrame.origin.x - windowOrigin.x,
            y: globalFrame.origin.y - windowOrigin.y
        ),
        size: globalFrame.size
    )
    window.toastFrame = convertedFrame
}

---

3. Magic Number for Drag Offset (MINOR)

ToastView.swift:70

dragOffset = -200

The hard-coded -200 should be a named constant or calculated based on screen height:

private let dismissAnimationOffset: CGFloat = -200

---

4. Potential Memory Leak in Animation Task (LOW)

ToastView.swift:74-77

Task { @MainActor in
    try? await Task.sleep(nanoseconds: UInt64(0.3 * 1_000_000_000))
    onDismiss()
}

This task is not stored, so it can't be cancelled if the view disappears during the sleep. Consider:

@State private var dismissTask: Task<Void, Never>?

// In gesture handler:
dismissTask?.cancel()
dismissTask = Task { @MainActor in
    try? await Task.sleep(nanoseconds: UInt64(0.3 * 1_000_000_000))
    guard !Task.isCancelled else { return }
    onDismiss()
}

---

5. Code Duplication (MINOR)

ToastWindowManager.swift:69-70, 115-117

The pattern of sleeping for 0.4s then clearing toastFrame is duplicated:

try? await Task.sleep(nanoseconds: UInt64(0.4 * 1_000_000_000))
self?.toastWindow?.toastFrame = .zero

Consider extracting to a helper method:

private func clearToastFrameAfterAnimation() async {
    try? await Task.sleep(nanoseconds: UInt64(0.4 * 1_000_000_000))
    guard !Task.isCancelled else { return }
    toastWindow?.toastFrame = .zero
}

---

🎯 Recommendations

1. Fix Equatable conformance to exclude id from equality checks (critical)
2. Remove unnecessary DispatchQueue.main.async in updateToastFrame() (medium)
3. Extract magic numbers to named constants (minor)
4. Add cancellation to dismiss animation task (low priority)
5. Add unit tests for auto-hide pause/resume logic if not already present

---

📝 Code Quality Score: 8/10

The implementation is solid with good UX improvements. The main concerns are the Equatable conformance change and the redundant async dispatch. Once those are addressed, this will be excellent!

---

🧪 Testing Suggestions

• Test rapid toast dismissal/creation to ensure no lingering frames
• Test toast interaction during app backgrounding/foregrounding
• Verify drag gestures work correctly with VoiceOver enabled
• Test with toasts of varying text lengths to ensure layout handles overflow

---

Great work overall! Let me know if you have questions about any of these suggestions.

[claude]

Code Review: Toast UI Polish

Summary

This PR implements visual and interaction improvements to the toast notification system, including updated styling, drag-to-dismiss functionality, and improved touch handling. Overall, the implementation is well-structured and thoughtful.

---

✅ Strengths

1. Excellent Touch Handling Architecture

   • The PassThroughWindow + frame tracking approach is elegant and solves the touch-through problem correctly
   • Using GeometryReader with PreferenceKey to track toast frame position is the right SwiftUI pattern

2. Smart Auto-Hide Pause/Resume

   • Pause/resume logic during drag is well-implemented
   • Properly calculates remaining time when pausing (ToastWindowManager.swift:70-88)
   • Good use of Date() for time tracking

3. Modern SwiftUI Patterns

   • Proper use of Task instead of DispatchQueue for async operations
   • @MainActor annotations on the manager class
   • Weak self captures to prevent retain cycles (ToastWindowManager.swift:64, 96)

4. Good Accessibility

   • Accessibility label added to dismiss button (ToastView.swift:35)

---

🐛 Potential Issues

1. Race Condition in showToast (Medium Priority)

// ToastWindowManager.swift:37-54
func showToast(_ toast: Toast) {
    cancelAutoHide()
    toastWindow?.hasToast = false
    toastWindow?.toastFrame = .zero
    
    toastWindow?.hasToast = true  // ⚠️ Set to true, then...
    
    withAnimation(.easeInOut(duration: 0.4)) {
        currentToast = toast  // ...this triggers preference change that updates toastFrame
    }
}

Issue: hasToast is set to true before toastFrame is populated by the preference key callback. This creates a brief window where touches might incorrectly pass through.

Suggested fix: Set hasToast = true inside the updateToastFrame callback or use a single atomic state update.

---

2. Hard-coded Magic Number (Low Priority)

// ToastView.swift:70
dragOffset = -200

This should be calculated based on screen height or toast height to work correctly on all devices (especially smaller devices like iPhone SE).

Suggested fix:

dragOffset = -UIScreen.main.bounds.height

---

3. Equatable Conformance Broken (High Priority)

// Toast.swift:3
struct Toast: Equatable {
    let id: UUID  // ⚠️ Each toast now has unique ID

Issue: Adding id: UUID with a default parameter breaks Equatable. Two toasts with identical content but different IDs will no longer be equal. This could cause unexpected behavior if equality checks are used elsewhere.

Impact: Since .id(toast.id) is used in ToastWindowView.swift:202, SwiftUI will treat each toast as unique, which is likely the intended behavior. However, if code elsewhere relies on toast equality, it will break.

Recommended:

• Either implement custom Equatable conformance that excludes id
• Or verify that no code relies on toast equality for business logic

// Option 1: Custom Equatable
extension Toast {
    static func == (lhs: Toast, rhs: Toast) -> Bool {
        lhs.type == rhs.type &&
        lhs.title == rhs.title &&
        lhs.description == rhs.description &&
        lhs.autoHide == rhs.autoHide &&
        lhs.visibilityTime == rhs.visibilityTime
    }
}

---

4. Potential Memory Leak (Low Priority)

// ToastView.swift:74-77
Task { @MainActor in
    try? await Task.sleep(nanoseconds: UInt64(0.3 * 1_000_000_000))
    onDismiss()  // ⚠️ No weak capture
}

If the toast view is deallocated before this task completes, the closure captures could keep it alive. Consider using [weak self] pattern or ensure the task is cancelled when the view disappears.

---

🎨 Style & Design Feedback

1. Visual Changes Look Good

   • Updated shadow (radius: 10, y: 25) provides better depth
   • Corner radius increased to 16 (from 8) - more modern
   • Background opacity reduced to 0.32 - less intrusive ✅

2. Drag Resistance

   • Downward drag resistance of 0.08 feels about right for preventing accidental dismissal
   • Upward drag being free is intuitive ✅

---

⚡ Performance Considerations

1. GeometryReader Performance: Using GeometryReader in an overlay is fine, but be aware it can cause extra layout passes. The current usage is minimal and shouldn't cause issues.

2. Task Cancellation: Good job checking Task.isCancelled in scheduleAutoHide (ToastWindowManager.swift:101, 112)

---

🧪 Testing Recommendations

Since this touches critical UI infrastructure, I recommend testing:

1. Rapid toast succession: Show multiple toasts quickly to verify no crashes or visual glitches
2. Drag during auto-hide: Verify pause/resume works correctly at the exact moment of auto-hide
3. Screen rotation: Ensure toastFrame updates correctly
4. Accessibility: Test with VoiceOver to ensure dismiss button is discoverable
5. Edge cases:
   • Toast with very long text
   • Rapid drag up/down movements
   • Backgrounding app during toast display

---

📋 Minor Suggestions

1. Constants Organization
   Consider extracting magic numbers to constants:

   private enum Constants {
       static let dismissThreshold: CGFloat = 50
       static let downwardDragResistance: CGFloat = 0.08
       static let dismissAnimationDuration: Double = 0.3
       static let dismissDistance: CGFloat = -200 // or computed
   }

2. Accessibility: Add .accessibilityHint() to the toast itself to indicate it can be dragged to dismiss

3. Animation Consistency: Animation duration is 0.4 in most places but 0.3 for dismiss. Consider using a consistent duration.

---

✅ Overall Assessment

Strong approval with minor recommendations. The code quality is high and follows SwiftUI best practices. The main concerns are:

1. The race condition in showToast (fixable with minor refactor)
2. The Equatable conformance change (needs verification)
3. Hard-coded dismiss offset (should be dynamic)

Great work on the touch handling architecture! 🎉

---

Recommendation: ✅ Approve with suggested fixes for the race condition and hard-coded offset.

[pwltr]

@piotr-iohk Could you please find out if the updated toast is causing the e2e tests to fail? Might have to wait until they hide or dismiss them in the tests.

[piotr-iohk]

@piotr-iohk Could you please find out if the updated toast is causing the e2e tests to fail? Might have to wait until they hide or dismiss them in the tests.

@pwltr the tests are updated. There is however a legit failure on @security - Security And Privacy @security_1 - Can setup PIN
The app shows "over 50% fee" dialog when not expected... the tests sends 1000 and the fee in the app reads 562.

security_-_Security_And_Privacy_security_1_-_Can_setup_PIN-2025-12-01T17-41-38-937Z.mp4

(There are also some failures like Tapping "WalletOnboardingClose" timeout, but these are unfortunately due to unstable backend recently, and apparently it happend during the run)

[pwltr]

The app shows "over 50% fee" dialog when not expected... the tests sends 1000 and the fee in the app reads 562.

@piotr-iohk Seems like the correct behaviour to me, the fee is more than half the send amount, what am I missing? Also don't see how this PR would affect this functionality.

[pwltr]

E2E tests passing now, failing integration test due to unstable backend.

[piotr-iohk]

The app shows "over 50% fee" dialog when not expected... the tests sends 1000 and the fee in the app reads 562.

@piotr-iohk Seems like the correct behaviour to me, the fee is more than half the send amount, what am I missing? Also don't see how this PR would affect this functionality.

I guess I need to work on my math 🤦 😆
Anyway I don't think I ever seen this in this particular test, so that was unexpected to me (and the test).

[jvsena42]

Tested

• Trigger same toast multiple times
• Tigger different toast without skipping the previous
• swipe down
• swipe up
• close button

[piotr-iohk]

Wondering if any of Claude suggestions need attention (especially the race condition)?

The race condition in showToast (fixable with minor refactor)
The Equatable conformance change (needs verification)
Hard-coded dismiss offset (should be dynamic)

[pwltr]

Wondering if any of Claude suggestions need attention (especially the race condition)?

The race condition in showToast (fixable with minor refactor)
The Equatable conformance change (needs verification)
Hard-coded dismiss offset (should be dynamic)

I think the race condition one is negligible, the rest is fine.