383 lines
12 KiB
Markdown
383 lines
12 KiB
Markdown
# Card/Modal Unification Plan
|
|
|
|
**Status:** Implemented
|
|
**Date:** 2026-02-26
|
|
**Author:** Claude + Taylor
|
|
|
|
---
|
|
|
|
## Executive Summary
|
|
|
|
Unify SessionCard and Modal into a single component with an `enlarged` prop, eliminating 165 lines of duplicated code and ensuring feature parity across both views.
|
|
|
|
---
|
|
|
|
## 1. Problem Statement
|
|
|
|
### 1.1 What's Broken
|
|
|
|
The AMC dashboard displays agent sessions as cards in a grid. Clicking a card opens a "modal" for a larger, focused view. These two views evolved independently, creating:
|
|
|
|
| Issue | Impact |
|
|
|-------|--------|
|
|
| **Duplicated rendering logic** | Modal.js reimplemented header, chat, input from scratch (227 lines) |
|
|
| **Feature drift** | Card had context usage display; modal didn't. Modal had timestamps; card didn't. |
|
|
| **Maintenance burden** | Every card change required parallel modal changes (often forgotten) |
|
|
| **Inconsistent UX** | Users see different information depending on view |
|
|
|
|
### 1.2 Why This Matters
|
|
|
|
The modal's purpose is simple: **show an enlarged view with more screen space for content**. It should not be a separate implementation with different features. Users clicking a card expect to see *the same thing, bigger* — not a different interface.
|
|
|
|
### 1.3 Root Cause
|
|
|
|
The modal was originally built as a separate component because it needed:
|
|
- Backdrop blur with click-outside-to-close
|
|
- Escape key handling
|
|
- Body scroll lock
|
|
- Entrance/exit animations
|
|
|
|
These concerns led developers to copy-paste card internals into the modal rather than compose them.
|
|
|
|
---
|
|
|
|
## 2. Goals and Non-Goals
|
|
|
|
### 2.1 Goals
|
|
|
|
1. **Zero duplicated rendering code** — Single source of truth for how sessions display
|
|
2. **Automatic feature parity** — Any card change propagates to modal without extra work
|
|
3. **Preserve modal behaviors** — Backdrop, escape key, animations, scroll lock
|
|
4. **Add missing features to both views** — Smart scroll, sending state feedback
|
|
|
|
### 2.2 Non-Goals
|
|
|
|
- Changing the visual design of either view
|
|
- Adding new features beyond parity + smart scroll + sending state
|
|
- Refactoring other components
|
|
|
|
---
|
|
|
|
## 3. User Workflows
|
|
|
|
### 3.1 Current User Journey
|
|
|
|
```
|
|
User sees session cards in grid
|
|
│
|
|
├─► Card shows: status, agent, cwd, context usage, last 20 messages, input
|
|
│
|
|
└─► User clicks card
|
|
│
|
|
└─► Modal opens with DIFFERENT layout:
|
|
- Combined status badge (dot inside)
|
|
- No context usage
|
|
- All messages with timestamps
|
|
- Different input implementation
|
|
- Keyboard hints shown
|
|
```
|
|
|
|
### 3.2 Target User Journey
|
|
|
|
```
|
|
User sees session cards in grid
|
|
│
|
|
├─► Card shows: status, agent, cwd, context usage, last 20 messages, input
|
|
│
|
|
└─► User clicks card
|
|
│
|
|
└─► Modal opens with SAME card, just bigger:
|
|
- Identical header layout
|
|
- Context usage visible
|
|
- All messages (not limited to 20)
|
|
- Same input components
|
|
- Same everything, more space
|
|
```
|
|
|
|
### 3.3 User Benefits
|
|
|
|
| Benefit | Rationale |
|
|
|---------|-----------|
|
|
| **Cognitive consistency** | Same information architecture in both views reduces learning curve |
|
|
| **Trust** | No features "hiding" in one view or the other |
|
|
| **Predictability** | Click = zoom, not "different interface" |
|
|
|
|
---
|
|
|
|
## 4. Design Decisions
|
|
|
|
### 4.1 Architecture: Shared Component with Prop
|
|
|
|
**Decision:** Add `enlarged` prop to SessionCard. Modal renders `<SessionCard enlarged={true} />`.
|
|
|
|
**Alternatives Considered:**
|
|
|
|
| Alternative | Rejected Because |
|
|
|-------------|------------------|
|
|
| Modal wraps Card with CSS transform | Breaks layout, accessibility issues, can't change message limit |
|
|
| Higher-order component | Unnecessary complexity for single boolean difference |
|
|
| Render props pattern | Overkill, harder to read |
|
|
| Separate "CardContent" extracted | Still requires prop to control limit, might as well be on SessionCard |
|
|
|
|
**Rationale:** A single boolean prop is the simplest solution that achieves all goals. The `enlarged` prop controls exactly two things: container sizing and message limit. Everything else is identical.
|
|
|
|
---
|
|
|
|
### 4.2 Message Limit: Card 20, Enlarged All
|
|
|
|
**Decision:** Card shows last 20 messages. Enlarged view shows all.
|
|
|
|
**Rationale:**
|
|
- Cards in a grid need bounded height for visual consistency
|
|
- 20 messages is enough context without overwhelming the card
|
|
- Enlarged view exists specifically to see more — no artificial limit makes sense
|
|
- Implementation: `limit` prop on ChatMessages (20 default, null for unlimited)
|
|
|
|
---
|
|
|
|
### 4.3 Header Layout: Keep Card's Multi-Row Style
|
|
|
|
**Decision:** Use the card's multi-row header layout for both views.
|
|
|
|
**Modal had:** Single row with combined status badge (dot inside badge)
|
|
**Card had:** Multi-row with separate dot, status badge, agent badge, cwd badge, context usage
|
|
|
|
**Rationale:**
|
|
- Card layout shows more information (context usage was missing from modal)
|
|
- Multi-row handles overflow gracefully with `flex-wrap`
|
|
- Consistent with the "modal = bigger card" philosophy
|
|
|
|
---
|
|
|
|
### 4.4 Spacing: Keep Tighter (Card Style)
|
|
|
|
**Decision:** Use card's tighter spacing (`px-4 py-3`, `space-y-2.5`) for both views.
|
|
|
|
**Modal had:** Roomier spacing (`px-5 py-4`, `space-y-4`)
|
|
|
|
**Rationale:**
|
|
- Tighter spacing is more information-dense
|
|
- Enlarged view gains space from larger container, not wider margins
|
|
- Consistent visual rhythm between views
|
|
|
|
---
|
|
|
|
### 4.5 Empty State Text: "No messages to show"
|
|
|
|
**Decision:** Standardize on "No messages to show" (neither original).
|
|
|
|
**Card had:** "No messages yet"
|
|
**Modal had:** "No conversation messages"
|
|
|
|
**Rationale:** "No messages to show" is neutral and accurate — doesn't imply timing ("yet") or specific terminology ("conversation").
|
|
|
|
---
|
|
|
|
## 5. Implementation Details
|
|
|
|
### 5.1 SessionCard.js Changes
|
|
|
|
```
|
|
BEFORE: SessionCard({ session, onClick, conversation, onFetchConversation, onRespond, onDismiss })
|
|
AFTER: SessionCard({ session, onClick, conversation, onFetchConversation, onRespond, onDismiss, enlarged = false })
|
|
```
|
|
|
|
**New behaviors controlled by `enlarged`:**
|
|
|
|
| Aspect | `enlarged=false` (card) | `enlarged=true` (modal) |
|
|
|--------|-------------------------|-------------------------|
|
|
| Container classes | `h-[850px] w-[600px] cursor-pointer hover:...` | `max-w-5xl max-h-[90vh]` |
|
|
| Click handler | `onClick(session)` | `undefined` (no-op) |
|
|
| Message limit | 20 | null (all) |
|
|
|
|
**New feature: Smart scroll tracking**
|
|
|
|
```js
|
|
// Track if user is at bottom
|
|
const wasAtBottomRef = useRef(true);
|
|
|
|
// On scroll, update tracking
|
|
const handleScroll = () => {
|
|
wasAtBottomRef.current = el.scrollHeight - el.scrollTop - el.clientHeight < 50;
|
|
};
|
|
|
|
// On new messages, only scroll if user was at bottom
|
|
if (hasNewMessages && wasAtBottomRef.current) {
|
|
el.scrollTop = el.scrollHeight;
|
|
}
|
|
```
|
|
|
|
**Rationale:** Users reading history shouldn't be yanked to bottom when new messages arrive. Only auto-scroll if they were already at the bottom (watching live updates).
|
|
|
|
---
|
|
|
|
### 5.2 Modal.js Changes
|
|
|
|
**Before:** 227 lines reimplementing header, chat, input, scroll, state management
|
|
|
|
**After:** 62 lines — backdrop wrapper only
|
|
|
|
```js
|
|
export function Modal({ session, conversations, onClose, onRespond, onFetchConversation, onDismiss }) {
|
|
// Closing animation state
|
|
// Body scroll lock
|
|
// Escape key handler
|
|
|
|
return html`
|
|
<div class="backdrop...">
|
|
<${SessionCard}
|
|
session=${session}
|
|
conversation=${conversations[session.session_id] || []}
|
|
onFetchConversation=${onFetchConversation}
|
|
onRespond=${onRespond}
|
|
onDismiss=${onDismiss}
|
|
onClick=${() => {}}
|
|
enlarged=${true}
|
|
/>
|
|
</div>
|
|
`;
|
|
}
|
|
```
|
|
|
|
**Preserved behaviors:**
|
|
- Backdrop blur (`bg-[#02050d]/84 backdrop-blur-sm`)
|
|
- Click outside to close
|
|
- Escape key handler
|
|
- Body scroll lock (`document.body.style.overflow = 'hidden'`)
|
|
- Entrance/exit animations (CSS classes)
|
|
|
|
---
|
|
|
|
### 5.3 ChatMessages.js Changes
|
|
|
|
```
|
|
BEFORE: ChatMessages({ messages, status })
|
|
AFTER: ChatMessages({ messages, status, limit = 20 })
|
|
```
|
|
|
|
**Logic change:**
|
|
```js
|
|
// Before: always slice to 20
|
|
const displayMessages = allDisplayMessages.slice(-20);
|
|
|
|
// After: respect limit prop
|
|
const displayMessages = limit ? allDisplayMessages.slice(-limit) : allDisplayMessages;
|
|
```
|
|
|
|
---
|
|
|
|
### 5.4 SimpleInput.js / QuestionBlock.js Changes
|
|
|
|
**New feature: Sending state feedback**
|
|
|
|
```js
|
|
const [sending, setSending] = useState(false);
|
|
|
|
const handleSubmit = async (e) => {
|
|
if (sending) return;
|
|
setSending(true);
|
|
try {
|
|
await onRespond(...);
|
|
} finally {
|
|
setSending(false);
|
|
}
|
|
};
|
|
|
|
// In render:
|
|
<button disabled=${sending}>
|
|
${sending ? 'Sending...' : 'Send'}
|
|
</button>
|
|
```
|
|
|
|
**Rationale:** Users need feedback that their message is being sent. Without this, they might click multiple times or think the UI is broken.
|
|
|
|
---
|
|
|
|
### 5.5 App.js Changes
|
|
|
|
**Removed (unused after refactor):**
|
|
- `conversationLoading` state — was only passed to Modal
|
|
- `refreshConversation` callback — was only used by Modal's custom send handler
|
|
|
|
**Modified:**
|
|
- `respondToSession` now refreshes conversation immediately after successful send
|
|
- Modal receives same props as SessionCard (onRespond, onFetchConversation, onDismiss)
|
|
|
|
---
|
|
|
|
## 6. Dependency Graph
|
|
|
|
```
|
|
App.js
|
|
│
|
|
├─► SessionCard (in grid)
|
|
│ ├─► ChatMessages (limit=20)
|
|
│ │ └─► MessageBubble
|
|
│ ├─► QuestionBlock (with sending state)
|
|
│ │ └─► OptionButton
|
|
│ └─► SimpleInput (with sending state)
|
|
│
|
|
└─► Modal (backdrop wrapper)
|
|
└─► SessionCard (enlarged=true)
|
|
├─► ChatMessages (limit=null)
|
|
│ └─► MessageBubble
|
|
├─► QuestionBlock (with sending state)
|
|
│ └─► OptionButton
|
|
└─► SimpleInput (with sending state)
|
|
```
|
|
|
|
**Key insight:** Modal no longer has its own rendering tree. It delegates entirely to SessionCard.
|
|
|
|
---
|
|
|
|
## 7. Metrics
|
|
|
|
| Metric | Before | After | Change |
|
|
|--------|--------|-------|--------|
|
|
| Modal.js lines | 227 | 62 | -73% |
|
|
| Total duplicated code | ~180 lines | 0 | -100% |
|
|
| Features requiring dual maintenance | All | None | -100% |
|
|
| Prop surface area (Modal) | 6 custom | 6 same as card | Aligned |
|
|
|
|
---
|
|
|
|
## 8. Verification Checklist
|
|
|
|
- [x] Card displays: status dot, status badge, agent badge, cwd, context usage, messages, input
|
|
- [x] Modal displays: identical to card, just larger
|
|
- [x] Card limits to 20 messages
|
|
- [x] Modal shows all messages
|
|
- [x] Smart scroll works in both views
|
|
- [x] "Sending..." feedback works in both views
|
|
- [x] Escape closes modal
|
|
- [x] Click outside closes modal
|
|
- [x] Entrance/exit animations work
|
|
- [x] Body scroll locked when modal open
|
|
|
|
---
|
|
|
|
## 9. Future Considerations
|
|
|
|
### 9.1 Potential Enhancements
|
|
|
|
| Enhancement | Rationale | Blocked By |
|
|
|-------------|-----------|------------|
|
|
| Keyboard navigation in card grid | Accessibility | None |
|
|
| Resize modal dynamically | User preference | None |
|
|
| Pin modal to side (split view) | Power user workflow | Design decision needed |
|
|
|
|
### 9.2 Maintenance Notes
|
|
|
|
- **Any SessionCard change** automatically applies to modal view
|
|
- **To add modal-only behavior**: Check `enlarged` prop (but avoid this — keep views identical)
|
|
- **To change message limit**: Modify the `limit` prop value in SessionCard's ChatMessages call
|
|
|
|
---
|
|
|
|
## 10. Lessons Learned
|
|
|
|
1. **Composition > Duplication** — When two UIs show the same data, compose them from shared components
|
|
2. **Props for variations** — A single boolean prop is often sufficient for "same thing, different context"
|
|
3. **Identify the actual differences** — Modal needed only: backdrop, escape key, scroll lock, animations. Everything else was false complexity.
|
|
4. **Feature drift is inevitable** — Duplicated code guarantees divergence over time. Only shared code stays in sync.
|