diff --git a/frontend/src/components/SessionView.tsx b/frontend/src/components/SessionView.tsx index 5c5d76f0..f9c594b7 100644 --- a/frontend/src/components/SessionView.tsx +++ b/frontend/src/components/SessionView.tsx @@ -58,40 +58,17 @@ import { Kbd } from './ui/Kbd'; import { useErrorStore } from '../stores/errorStore'; import ProjectSettings from './ProjectSettings'; -const REVIEW_UNAVAILABLE_REASON = 'Open a PR for this branch to review changes'; - -function canSelectPanel(panel: ToolPanel | null | undefined, hasReviewPr: boolean): panel is ToolPanel { - return !!panel && (panel.type !== 'diff' || hasReviewPr); +function canSelectPanel(panel: ToolPanel | null | undefined): panel is ToolPanel { + return !!panel; } -function pickDefaultPanel(panelList: ToolPanel[], hasReviewPr: boolean): ToolPanel | undefined { - return (hasReviewPr ? panelList.find(p => p.type === 'diff') : undefined) +function pickDefaultPanel(panelList: ToolPanel[]): ToolPanel | undefined { + return panelList.find(p => p.type === 'diff') || panelList.find(p => p.type === 'explorer') || panelList.find(p => p.type !== 'diff') || panelList[0]; } -function sanitizeReviewActivePanels( - node: SessionPanelLayout['root'], - panelById: Map, - hasReviewPr: boolean -): SessionPanelLayout['root'] { - if (hasReviewPr) return node; - - if (node.type === 'group') { - const activePanel = node.activePanelId ? panelById.get(node.activePanelId) : undefined; - if (activePanel?.type !== 'diff') return node; - - const fallback = node.panelIds - .map(id => panelById.get(id)) - .find((panel): panel is ToolPanel => !!panel && panel.type !== 'diff'); - - return { ...node, activePanelId: fallback?.id ?? null }; - } - - return { ...node, children: node.children.map(child => sanitizeReviewActivePanels(child, panelById, hasReviewPr)) }; -} - export const SessionView = memo(() => { const { activeView, activeProjectId } = useNavigationStore(); const [projectData, setProjectData] = useState(null); @@ -250,17 +227,16 @@ export const SessionView = memo(() => { // Always reload panels from database when switching sessions panelApi.loadPanelsForSession(sid).then(async loadedPanels => { devLog.debug('[SessionView] Loaded panels:', loadedPanels); - const hasReviewPr = !!activeSession.gitStatus?.prUrl; const inFlight = (usePanelStore.getState().panels[sid] || []).filter( p => !preLoadIds.has(p.id) && !loadedPanels.some(lp => lp.id === p.id) ); setPanels(sid, inFlight.length > 0 ? [...loadedPanels, ...inFlight] : loadedPanels); - // Pick default active: Review is only selectable once a PR is known. - const fallback = pickDefaultPanel(loadedPanels, hasReviewPr); + // Pick default active panel. + const fallback = pickDefaultPanel(loadedPanels); const activePanelResult = await panelApi.getActivePanel(sid); - const effectiveActivePanel = canSelectPanel(activePanelResult, hasReviewPr) + const effectiveActivePanel = canSelectPanel(activePanelResult) ? activePanelResult : fallback; const fallbackActiveId = effectiveActivePanel?.id ?? null; @@ -307,22 +283,16 @@ export const SessionView = memo(() => { fallbackActiveId, ); const { layout } = reconcileLayout(base, liveIdsNow); - const panelById = new Map(nowPanels.map(panel => [panel.id, panel])); - const safeRoot = sanitizeReviewActivePanels(layout.root, panelById, hasReviewPr); - const safeLayout = safeRoot === layout.root ? layout : { ...layout, root: safeRoot }; - setLayoutInStore(sid, safeLayout); - setFocusedGroupInStore(sid, safeLayout.focusedGroupId ?? primaryGroup(safeLayout.root).id); + setLayoutInStore(sid, layout); + setFocusedGroupInStore(sid, layout.focusedGroupId ?? primaryGroup(layout.root).id); } catch (err) { console.warn('[SessionView] Failed to load layout, creating default:', err); const layout = createSingleGroupLayout( sortedLive.map(p => p.id), fallbackActiveId, ); - const panelById = new Map(loadedPanels.map(panel => [panel.id, panel])); - const safeRoot = sanitizeReviewActivePanels(layout.root, panelById, hasReviewPr); - const safeLayout = safeRoot === layout.root ? layout : { ...layout, root: safeRoot }; - setLayoutInStore(sid, safeLayout); - setFocusedGroupInStore(sid, safeLayout.focusedGroupId ?? primaryGroup(safeLayout.root).id); + setLayoutInStore(sid, layout); + setFocusedGroupInStore(sid, layout.focusedGroupId ?? primaryGroup(layout.root).id); } }); } @@ -331,7 +301,7 @@ export const SessionView = memo(() => { return () => { flushLayoutPersist(); }; - }, [activeSession?.id, activeSession?.gitStatus?.prUrl, setPanels, setActivePanelInStore, setLayoutInStore, setFocusedGroupInStore, flushLayoutPersist]); + }, [activeSession?.id, setPanels, setActivePanelInStore, setLayoutInStore, setFocusedGroupInStore, flushLayoutPersist]); // Listen for panel updates from the backend useEffect(() => { @@ -495,13 +465,8 @@ export const SessionView = memo(() => { const getPanelTabPresentation = useCallback((panel) => { if (panel.type !== 'diff') return undefined; - const hasReviewPr = !!activeSession?.gitStatus?.prUrl; - return { - title: 'Review', - disabled: !hasReviewPr, - disabledReason: hasReviewPr ? undefined : REVIEW_UNAVAILABLE_REASON, - }; - }, [activeSession?.gitStatus?.prUrl]); + return { title: 'Review' }; + }, []); // --- Drag & drop state --- const [draggedPanelId, setDraggedPanelId] = useState(null); @@ -553,7 +518,6 @@ export const SessionView = memo(() => { const handleGroupPanelSelect = useCallback( (groupId: string, panel: ToolPanel) => { if (!activeSession) return; - if (panel.type === 'diff' && !activeSession.gitStatus?.prUrl) return; const sid = activeSession.id; const currentLayout = usePanelStore.getState().layouts[sid]; if (!currentLayout) return; @@ -584,7 +548,6 @@ export const SessionView = memo(() => { const handlePanelSelect = useCallback( async (panel: ToolPanel) => { if (!activeSession) return; - if (panel.type === 'diff' && !activeSession.gitStatus?.prUrl) return; // Add to history when panel is selected addToHistory(activeSession.id, panel.id); @@ -608,7 +571,6 @@ export const SessionView = memo(() => { const handleCommitClick = useCallback( async (commitHash: string) => { if (!activeSession || sessionPanels.length === 0) return; - if (!activeSession.gitStatus?.prUrl) return; const diffPanel = sessionPanels.find(p => p.type === 'diff'); if (!diffPanel) return; // Store pending hash before dispatching — if the diff panel is not @@ -1033,10 +995,6 @@ export const SessionView = memo(() => { const currentLayout = usePanelStore.getState().layouts[sid]; if (currentLayout) { const group = findGroup(currentLayout.root, groupId); - const panel = group?.activePanelId - ? usePanelStore.getState().panels[sid]?.find(candidate => candidate.id === group.activePanelId) - : undefined; - if (panel?.type === 'diff' && !activeSession.gitStatus?.prUrl) return; if (group?.activePanelId) { setActivePanelInStore(sid, group.activePanelId); panelApi.setActivePanel(sid, group.activePanelId).catch(() => {}); diff --git a/frontend/src/components/panels/diff/CombinedDiffView.tsx b/frontend/src/components/panels/diff/CombinedDiffView.tsx index ac1b9e06..624cc87f 100644 --- a/frontend/src/components/panels/diff/CombinedDiffView.tsx +++ b/frontend/src/components/panels/diff/CombinedDiffView.tsx @@ -117,6 +117,7 @@ const CombinedDiffView = memo(forwardRef { + mountedRef.current = true; return () => { mountedRef.current = false; }; }, []); diff --git a/frontend/src/components/panels/diff/DiffPanel.tsx b/frontend/src/components/panels/diff/DiffPanel.tsx index 2cf82ebb..22c7649d 100644 --- a/frontend/src/components/panels/diff/DiffPanel.tsx +++ b/frontend/src/components/panels/diff/DiffPanel.tsx @@ -6,9 +6,11 @@ import type { GitStatus } from '../../../types/session'; import { AlertCircle, GitBranch, Globe } from 'lucide-react'; import { useSession } from '../../../contexts/SessionContext'; import { cn } from '../../../utils/cn'; +import { Tooltip } from '../../ui/Tooltip'; import BrowserSurface from '../browser/BrowserSurface'; import { consumeLocalReviewModeRequest, + getEffectiveReviewMode, getReviewDefaultMode, setReviewDefaultMode, subscribeReviewDefaultMode, @@ -76,6 +78,7 @@ export const DiffPanel: React.FC = ({ const lastGitFingerprintRef = useRef(null); const wasActiveRef = useRef(isActive); const reviewUrl = useMemo(() => buildGithubReviewUrl(session?.gitStatus?.prUrl), [session?.gitStatus?.prUrl]); + const effectiveReviewMode = getEffectiveReviewMode(reviewMode, !!reviewUrl); useEffect(() => subscribeReviewDefaultMode(setReviewModeState), []); @@ -154,7 +157,7 @@ export const DiffPanel: React.FC = ({ const becameActive = isActive && !wasActiveRef.current; wasActiveRef.current = isActive; - if (becameActive && isStale && reviewMode === 'local') { + if (becameActive && isStale && effectiveReviewMode === 'local') { setIsStale(false); combinedDiffRef.current?.refresh(); @@ -188,25 +191,30 @@ export const DiffPanel: React.FC = ({ return () => clearTimeout(timer); } // eslint-disable-next-line react-hooks/exhaustive-deps -- panel.state/diffState intentionally excluded: they are written inside this effect via IPC and must not re-trigger it - }, [isActive, isStale, panel.id, sessionId, reviewMode]); - - if (!reviewUrl) { - return ( -
-
-
- -

Review unavailable

-

- Open a PR for this branch to review changes. -

-
-
-
- ); - } + }, [effectiveReviewMode, isActive, isStale, panel.id, sessionId]); const prLabel = session?.gitStatus?.prNumber ? `#${session.gitStatus.prNumber}` : 'Pull Request'; + const githubModeButton = ( + + ); return (
@@ -214,37 +222,34 @@ export const DiffPanel: React.FC = ({
Review - {prLabel} - {session?.gitStatus?.prTitle && ( - {session.gitStatus.prTitle} + {reviewUrl ? ( + <> + {prLabel} + {session?.gitStatus?.prTitle && ( + {session.gitStatus.prTitle} + )} + + ) : ( + Local changes )}
- + {reviewUrl ? githubModeButton : ( + + {githubModeButton} + + )}
{/* Stale indicator bar */} - {reviewMode === 'local' && isStale && !isActive && ( + {effectiveReviewMode === 'local' && isStale && !isActive && (
@@ -264,7 +269,7 @@ export const DiffPanel: React.FC = ({ {/* Main diff view */}
- {reviewMode === 'github' ? ( + {effectiveReviewMode === 'github' && reviewUrl ? ( { + it('falls back from GitHub to local mode when no PR URL is available', () => { + expect(getEffectiveReviewMode('github', false)).toBe('local'); + }); + + it('keeps GitHub mode when a PR URL is available', () => { + expect(getEffectiveReviewMode('github', true)).toBe('github'); + }); + + it('restores GitHub mode when a PR URL becomes available later', () => { + const savedMode = 'github'; + + expect(getEffectiveReviewMode(savedMode, false)).toBe('local'); + expect(getEffectiveReviewMode(savedMode, true)).toBe('github'); + }); + + it('keeps explicit local mode regardless of PR URL availability', () => { + expect(getEffectiveReviewMode('local', false)).toBe('local'); + expect(getEffectiveReviewMode('local', true)).toBe('local'); + }); +}); diff --git a/frontend/src/components/panels/diff/reviewModePreference.ts b/frontend/src/components/panels/diff/reviewModePreference.ts index 446d3fff..eb9aaf99 100644 --- a/frontend/src/components/panels/diff/reviewModePreference.ts +++ b/frontend/src/components/panels/diff/reviewModePreference.ts @@ -15,6 +15,10 @@ export function getReviewDefaultMode(): ReviewMode { return isReviewMode(stored) ? stored : 'github'; } +export function getEffectiveReviewMode(mode: ReviewMode, hasReviewUrl: boolean): ReviewMode { + return mode === 'github' && !hasReviewUrl ? 'local' : mode; +} + export function setReviewDefaultMode(mode: ReviewMode): void { window.localStorage.setItem(REVIEW_MODE_STORAGE_KEY, mode); window.dispatchEvent(new CustomEvent(REVIEW_MODE_CHANGED_EVENT, { detail: { mode } }));