# Fix Plan: Messages UI Scroll/Back Bugs

**Date:** Jan 30, 2026  
**Author:** Claude Architect  
**Investigation:** `docs/INVESTIGATION.md`  
**Bugs:** Messages scroll triggers click, Back button goes to dashboard

---

## Overview

Two fixes required:
1. **Fix 1:** Prevent scroll from triggering click on conversation cards
2. **Fix 2:** Make browser back button return to message list (not dashboard)

**Estimated time:** 2-3 hours total

---

## Fix 1: Scroll Triggering Click

### Problem
User scrolls through message list, lifts finger → accidentally clicks into conversation.

### Root Cause
The `useLongPress` hook's `touchmove` listener may not fire during native scroll momentum because the scroll container captures touch events.

### Solution: Position-Based Scroll Detection

Instead of relying on `touchmove` events (which may be consumed by scroll container), detect scroll by checking if the element's position changed between touchstart and touchend.

### Changes

#### File: `admin/src/hooks/useLongPress.js`

**Current (problematic):**
```javascript
const handleMove = useCallback((event) => {
  if (event.touches) {
    const moveX = Math.abs(event.touches[0].clientX - startPosition.current.x);
    const moveY = Math.abs(event.touches[0].clientY - startPosition.current.y);
    if (moveX > 10 || moveY > 10) {
      cancelledByScroll.current = true;
      // ...
    }
  }
}, []);
```

**Fix - Add element position tracking:**

Add new ref to track element's initial rect:
```javascript
const startRect = useRef(null);
```

In `start` function, save element position:
```javascript
const start = useCallback((event) => {
  cancelledByScroll.current = false;
  
  // Store initial touch position
  if (event.touches) {
    startPosition.current = {
      x: event.touches[0].clientX,
      y: event.touches[0].clientY,
    };
  } else {
    startPosition.current = {
      x: event.clientX,
      y: event.clientY,
    };
  }

  // NEW: Store element's initial position for scroll detection
  if (event.currentTarget) {
    startRect.current = event.currentTarget.getBoundingClientRect();
  }
  
  // ... rest of existing code
}, [callback, threshold, onStart]);
```

In `clear` function, check if element moved (scroll happened):
```javascript
const clear = useCallback((event, shouldTriggerClick = true) => {
  if (timeout.current) {
    clearTimeout(timeout.current);
    timeout.current = null;
  }

  // NEW: Check if element position changed (scroll detected)
  if (startRect.current && event?.currentTarget) {
    const currentRect = event.currentTarget.getBoundingClientRect();
    const positionDelta = Math.abs(currentRect.top - startRect.current.top);
    if (positionDelta > 10) {
      cancelledByScroll.current = true;
    }
  }

  if (longPressTriggered) {
    onFinish?.(event);
    event?.preventDefault?.();
  } else if (shouldTriggerClick && !cancelledByScroll.current) {
    onCancel?.(event);
  }

  // Reset all refs
  setLongPressTriggered(false);
  cancelledByScroll.current = false;
  startRect.current = null;  // NEW: Reset rect
  
  // ... rest of cleanup
}, [longPressTriggered, onFinish, onCancel, handleMove]);
```

### Why This Works
- On touchstart: Save element's position (getBoundingClientRect().top)
- On touchend: Check if position changed by >10px
- If position changed: User scrolled → don't trigger click
- Works even if touchmove events are captured by scroll container

### Alternative: Also Keep touchmove Detection
Keep both approaches for redundancy - touchmove for fast detection (cancels long-press timer early), position-check for failsafe on touchend.

---

## Fix 2: Back Button Navigation

### Problem
Pressing browser back from conversation view goes to dashboard instead of message list.

### Root Cause
1. `activeLeadId` is only read from URL on component mount
2. No listener for URL changes to sync back to state
3. `setSearchParams` replaces URL without adding history entry

### Solution: Proper History Management

Two-part fix:
1. Use `navigate()` to push history entries when opening conversations
2. Add effect to sync URL → state when browser back is pressed

### Changes

#### File: `admin/src/pages/Messages.jsx`

**Change 1: Push history entry when opening conversation**

Find where `setActiveLeadId` is called from card click:
```javascript
// CURRENT (around line 1573-1584):
<EnhancedConversationCard
  onClick={() => setActiveLeadId(conversation.lead.id)}
  ...
/>
```

Change to use navigate:
```javascript
// NEW:
<EnhancedConversationCard
  onClick={() => {
    // Push history entry so back button works
    navigate(`/inbox?lead=${conversation.lead.id}`);
    setActiveLeadId(conversation.lead.id);
  }}
  ...
/>
```

**Change 2: Add effect to sync URL → state**

Add new effect after the existing searchParams effect (around line 1347):
```javascript
// NEW: Sync URL changes back to state (handles browser back button)
useEffect(() => {
  const leadFromUrl = searchParams.get('lead');
  
  // Only update if URL doesn't match current state
  // This handles browser back/forward navigation
  if (leadFromUrl !== activeLeadId) {
    setActiveLeadId(leadFromUrl);
  }
}, [searchParams]); // Only depend on searchParams, NOT activeLeadId
```

**Change 3: Update the existing effect to not fight with the new one**

Modify the existing effect (lines 1347-1355) to only handle state → URL sync:
```javascript
// EXISTING - Modify to only push URL when state changes FROM user action
useEffect(() => {
  // Only update URL if state changed (not from URL change)
  const leadFromUrl = searchParams.get('lead');
  if (activeLeadId && leadFromUrl !== activeLeadId) {
    setSearchParams({ lead: activeLeadId }, { replace: true });
    loadConversation(activeLeadId);
  } else if (!activeLeadId && leadFromUrl) {
    setSearchParams({}, { replace: true });
    setActiveConversation(null);
    setActiveTriage(null);
  } else if (activeLeadId && leadFromUrl === activeLeadId) {
    // URL already correct, just load the conversation
    loadConversation(activeLeadId);
  } else {
    // Both null, nothing to do
    setActiveConversation(null);
    setActiveTriage(null);
  }
}, [activeLeadId, loadConversation]);
```

**Simpler Alternative - Consolidated Effect:**

Replace both effects with one cleaner implementation:
```javascript
// Sync activeLeadId with URL (bidirectional)
useEffect(() => {
  const leadFromUrl = searchParams.get('lead');
  
  // URL changed externally (browser back/forward) → update state
  if (leadFromUrl !== activeLeadId) {
    setActiveLeadId(leadFromUrl);
    return; // Let the state change trigger re-render, which will run this effect again
  }
  
  // State matches URL → load conversation data
  if (activeLeadId) {
    loadConversation(activeLeadId);
  } else {
    setActiveConversation(null);
    setActiveTriage(null);
  }
}, [searchParams, activeLeadId, loadConversation]);
```

Wait - this could cause issues. Let me think more carefully...

**Final Recommended Approach:**

1. Remove URL sync from state effect
2. Add separate effect for URL → state sync
3. Use navigate() for state → URL sync (in click handler)

```javascript
// Effect 1: Load conversation when activeLeadId changes
useEffect(() => {
  if (activeLeadId) {
    loadConversation(activeLeadId);
  } else {
    setActiveConversation(null);
    setActiveTriage(null);
  }
}, [activeLeadId, loadConversation]);

// Effect 2: Sync URL → state (for browser back/forward)
useEffect(() => {
  const leadFromUrl = searchParams.get('lead') || null;
  if (leadFromUrl !== activeLeadId) {
    setActiveLeadId(leadFromUrl);
  }
}, [searchParams]); // Intentionally omit activeLeadId to avoid loops

// Click handler updates both state and URL:
const handleCardClick = useCallback((leadId) => {
  navigate(`/inbox?lead=${leadId}`);
  // Don't need setActiveLeadId - the URL change will trigger Effect 2
}, [navigate]);

// Back handler:
const handleBack = useCallback(() => {
  navigate('/inbox');
  // Don't need setActiveLeadId - the URL change will trigger Effect 2  
}, [navigate]);
```

### Files Modified Summary

| Line(s) | Change |
|---------|--------|
| ~1347-1355 | Simplify to only load conversation on activeLeadId change |
| NEW ~1356 | Add URL → state sync effect |
| ~1573, ~1584 | Use navigate() in onClick handlers |
| ~1599 | Update onBack to use navigate('/inbox') |

---

## Implementation Order

1. **Fix 2 first** (back button) - Simpler, higher confidence
2. **Fix 1 second** (scroll detection) - Requires careful testing

---

## Testing Checklist

### Fix 1 (Scroll Detection)
- [ ] Fast scroll through message list, lift finger → should NOT open conversation
- [ ] Slow scroll, lift finger → should NOT open conversation  
- [ ] Tap without scrolling → SHOULD open conversation
- [ ] Long-press → SHOULD show context menu
- [ ] Test on iPhone Safari
- [ ] Test on Android Chrome

### Fix 2 (Back Button)
- [ ] Click conversation → press browser back → should show message list
- [ ] Click conversation → press browser back → press back again → should leave inbox
- [ ] Deep link to `/inbox?lead=xxx` → press back → should go to previous page
- [ ] Use in-app back button (arrow) → should show message list
- [ ] Forward/back navigation should work correctly

---

## Risk Assessment

| Change | Risk | Mitigation |
|--------|------|------------|
| useLongPress position tracking | Low - additive change | Keep existing touchmove detection as backup |
| URL/state sync | Medium - timing issues | Test thoroughly, use single source of truth (URL) |
| navigate() calls | Low | Standard React Router pattern |

---

## Rollback Plan

If issues arise:
1. Revert useLongPress changes (git revert or restore from commit)
2. For URL sync issues, remove Effect 2 and restore original Effect

---

**Plan Ready for Builder: Yes**  
**Blockers: None**