diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md deleted file mode 100644 index 3ca605f4..00000000 --- a/IMPLEMENTATION_PLAN.md +++ /dev/null @@ -1,203 +0,0 @@ -# Implementation Plan: Fix Mobile Model Selection - -## Problem Statement -Users cannot change the model when creating a new task on mobile devices. The model selector uses fixed-width Radix UI Popovers with nested secondary popovers that extend to the right, causing the interface to go off-screen on mobile devices (typically 375-428px width). - -## Root Cause Analysis - -### Current Implementation Issues: -1. **Fixed Widths**: Main popover is 320px, secondary popovers are 220px -2. **Horizontal Nesting**: Secondary popovers (thinking levels, reasoning effort, cursor variants) position to the right of the main popover -3. **No Collision Handling**: Radix Popover doesn't have sufficient collision padding configured -4. **No Mobile-Specific UI**: Same component used for all screen sizes - -### Affected Files: -- `/apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx` - Core implementation -- `/apps/ui/src/components/views/agent-view/shared/agent-model-selector.tsx` - Wrapper for agent view -- `/apps/ui/src/components/views/agent-view/input-area/input-controls.tsx` - Usage location - -## Proposed Solution: Responsive Popover with Mobile Optimization - -### Approach: Add Responsive Width & Collision Handling - -**Rationale**: Minimal changes, maximum compatibility, leverages existing Radix UI features - -### Implementation Steps: - -#### 1. Create a Custom Hook for Mobile Detection -**File**: `/apps/ui/src/hooks/use-mobile.ts` (new file) - -```typescript -import { useEffect, useState } from 'react'; - -export function useMobile(breakpoint: number = 768) { - const [isMobile, setIsMobile] = useState(false); - - useEffect(() => { - const mediaQuery = window.matchMedia(`(max-width: ${breakpoint}px)`); - - const handleChange = () => { - setIsMobile(mediaQuery.matches); - }; - - // Set initial value - handleChange(); - - // Listen for changes - mediaQuery.addEventListener('change', handleChange); - return () => mediaQuery.removeEventListener('change', handleChange); - }, [breakpoint]); - - return isMobile; -} -``` - -**Why**: Follows existing pattern from `use-sidebar-auto-collapse.ts`, reusable across components - -#### 2. Update Phase Model Selector with Responsive Behavior -**File**: `/apps/ui/src/components/views/settings-view/model-defaults/phase-model-selector.tsx` - -**Changes**: -- Import and use `useMobile()` hook -- Apply responsive widths: - - Mobile: `w-[calc(100vw-32px)] max-w-[340px]` (full width with padding) - - Desktop: `w-80` (320px - current) -- Add collision handling to Radix Popover: - - `collisionPadding={16}` - Prevent edge overflow - - `avoidCollisions={true}` - Enable collision detection - - `sideOffset={4}` - Add spacing from trigger -- Secondary popovers: - - Mobile: Position `side="bottom"` instead of `side="right"` - - Desktop: Keep `side="right"` (current behavior) - - Mobile width: `w-[calc(100vw-32px)] max-w-[340px]` - - Desktop width: `w-[220px]` (current) - -**Specific Code Changes**: -```typescript -// Add at top of component -const isMobile = useMobile(768); - -// Main popover content - - -// Secondary popovers (thinking level, reasoning effort, etc.) - -``` - -#### 3. Test Responsive Behavior - -**Test Cases**: -- [ ] Mobile (< 768px): Popovers fit within screen, secondary popovers open below -- [ ] Tablet (768-1024px): Popovers use optimal width -- [ ] Desktop (> 1024px): Current behavior preserved -- [ ] Edge cases: Very narrow screens (320px), screen rotation -- [ ] Functionality: All model selections work correctly on all screen sizes - -## Alternative Approaches Considered - -### Alternative 1: Use Sheet Component for Mobile -**Pros**: Better mobile UX, full-screen takeover common pattern -**Cons**: Requires duplicating component logic, more complex state management, different UX between mobile/desktop - -**Verdict**: Rejected - Too much complexity for the benefit - -### Alternative 2: Simplify Mobile UI (Remove Nested Popovers) -**Pros**: Simpler mobile interface -**Cons**: Removes functionality (thinking levels, reasoning effort) on mobile, poor UX - -**Verdict**: Rejected - Removes essential features - -### Alternative 3: Horizontal Scrolling Container -**Pros**: Preserves exact desktop layout -**Cons**: Poor mobile UX, non-standard pattern, accessibility issues - -**Verdict**: Rejected - Bad mobile UX - -## Technical Considerations - -### Breakpoint Selection -- **768px chosen**: Standard tablet breakpoint -- Matches pattern in existing codebase (`use-sidebar-auto-collapse.ts` uses 1024px) -- Covers iPhone SE (375px) through iPhone 14 Pro Max (428px) - -### Collision Handling -- `collisionPadding={16}`: 16px buffer from edges (standard spacing) -- `avoidCollisions={true}`: Radix will automatically reposition if needed -- `sideOffset={4}`: Small gap between trigger and popover - -### Performance -- `useMobile` hook uses `window.matchMedia` (performant, native API) -- Re-renders only on breakpoint changes (not every resize) -- No additional dependencies - -### Compatibility -- Works with existing compact/full modes -- Preserves all functionality -- No breaking changes to props/API -- Compatible with existing styles - -## Implementation Checklist - -- [ ] Create `/apps/ui/src/hooks/use-mobile.ts` -- [ ] Update `phase-model-selector.tsx` with responsive behavior -- [ ] Test on mobile devices/emulators (Chrome DevTools) -- [ ] Test on tablet breakpoint -- [ ] Test on desktop (ensure no regression) -- [ ] Verify all model variants are selectable -- [ ] Check nested popovers (thinking level, reasoning effort, cursor) -- [ ] Verify compact mode still works in agent view -- [ ] Test keyboard navigation -- [ ] Test with touch interactions - -## Rollback Plan -If issues arise: -1. Revert `phase-model-selector.tsx` changes -2. Remove `use-mobile.ts` hook -3. Original functionality immediately restored - -## Success Criteria -✅ Users can select any model on mobile devices (< 768px width) -✅ All nested popover options are accessible on mobile -✅ Desktop behavior unchanged (no regressions) -✅ UI fits within viewport on all screen sizes (320px+) -✅ No horizontal scrolling required -✅ Touch interactions work correctly - -## Estimated Effort -- Implementation: 30-45 minutes -- Testing: 15-20 minutes -- **Total**: ~1 hour - -## Dependencies -None - uses existing Radix UI Popover features - -## Risks & Mitigation -| Risk | Likelihood | Impact | Mitigation | -|------|-----------|--------|------------| -| Breaks desktop layout | Low | Medium | Thorough testing, conditional logic | -| Poor mobile UX | Low | Medium | Follow mobile-first best practices | -| Touch interaction issues | Low | Low | Use Radix UI touch handlers | -| Breakpoint conflicts | Low | Low | Use standard 768px breakpoint | - -## Notes for Developer -- The `compact` prop in `agent-model-selector.tsx` is preserved and still works -- All existing functionality (thinking levels, reasoning effort, cursor variants) remains -- Only visual layout changes on mobile - no logic changes -- Consider adding this pattern to other popovers if successful diff --git a/apps/server/src/services/claude-usage-service.ts b/apps/server/src/services/claude-usage-service.ts index 9dd1943f..c9000582 100644 --- a/apps/server/src/services/claude-usage-service.ts +++ b/apps/server/src/services/claude-usage-service.ts @@ -185,7 +185,6 @@ export class ClaudeUsageService { } as Record, }); } catch (spawnError) { - // ... (error handling omitted for brevity in replace block, keep existing) const errorMessage = spawnError instanceof Error ? spawnError.message : String(spawnError); logger.error('[executeClaudeUsageCommandPty] Failed to spawn PTY:', errorMessage); diff --git a/apps/ui/src/components/usage-popover.tsx b/apps/ui/src/components/usage-popover.tsx index b34765bd..1194cb9c 100644 --- a/apps/ui/src/components/usage-popover.tsx +++ b/apps/ui/src/components/usage-popover.tsx @@ -72,17 +72,17 @@ export function UsagePopover() { const [codexError, setCodexError] = useState(null); // Check authentication status - const isClaudeCliVerified = !!claudeAuthStatus?.authenticated; + const isClaudeAuthenticated = !!claudeAuthStatus?.authenticated; const isCodexAuthenticated = codexAuthStatus?.authenticated; // Determine which tab to show by default useEffect(() => { - if (isClaudeCliVerified) { + if (isClaudeAuthenticated) { setActiveTab('claude'); } else if (isCodexAuthenticated) { setActiveTab('codex'); } - }, [isClaudeCliVerified, isCodexAuthenticated]); + }, [isClaudeAuthenticated, isCodexAuthenticated]); // Check if data is stale (older than 2 minutes) const isClaudeStale = useMemo(() => { @@ -173,10 +173,10 @@ export function UsagePopover() { // Auto-fetch on mount if data is stale useEffect(() => { - if (isClaudeStale && isClaudeCliVerified) { + if (isClaudeStale && isClaudeAuthenticated) { fetchClaudeUsage(true); } - }, [isClaudeStale, isClaudeCliVerified, fetchClaudeUsage]); + }, [isClaudeStale, isClaudeAuthenticated, fetchClaudeUsage]); useEffect(() => { if (isCodexStale && isCodexAuthenticated) { @@ -189,7 +189,7 @@ export function UsagePopover() { if (!open) return; // Fetch based on active tab - if (activeTab === 'claude' && isClaudeCliVerified) { + if (activeTab === 'claude' && isClaudeAuthenticated) { if (!claudeUsage || isClaudeStale) { fetchClaudeUsage(); } @@ -213,7 +213,7 @@ export function UsagePopover() { activeTab, claudeUsage, isClaudeStale, - isClaudeCliVerified, + isClaudeAuthenticated, codexUsage, isCodexStale, isCodexAuthenticated, @@ -348,7 +348,7 @@ export function UsagePopover() { ); // Determine which tabs to show - const showClaudeTab = isClaudeCliVerified; + const showClaudeTab = isClaudeAuthenticated; const showCodexTab = isCodexAuthenticated; return ( diff --git a/apps/ui/src/components/views/board-view/board-header.tsx b/apps/ui/src/components/views/board-view/board-header.tsx index 4fbbb678..1cf48f67 100644 --- a/apps/ui/src/components/views/board-view/board-header.tsx +++ b/apps/ui/src/components/views/board-view/board-header.tsx @@ -263,7 +263,7 @@ export function BoardHeader({ /> {/* Plan Button with Settings - only show on desktop, mobile has it in the menu */} - {!isMobile && ( + {isMounted && !isMobile && (