Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 14 additions & 56 deletions frontend/src/components/SessionView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, ToolPanel>,
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<Project | null>(null);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
});
}
Expand All @@ -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(() => {
Expand Down Expand Up @@ -495,13 +465,8 @@ export const SessionView = memo(() => {

const getPanelTabPresentation = useCallback<PanelTabPresentationResolver>((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<string | null>(null);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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(() => {});
Expand Down
1 change: 1 addition & 0 deletions frontend/src/components/panels/diff/CombinedDiffView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ const CombinedDiffView = memo(forwardRef<CombinedDiffViewHandle, CombinedDiffVie
const showDiffSkeleton = (diffLoading || commitDiffLoading) && combinedDiff === null;

useEffect(() => {
mountedRef.current = true;
return () => { mountedRef.current = false; };
}, []);

Expand Down
83 changes: 44 additions & 39 deletions frontend/src/components/panels/diff/DiffPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -76,6 +78,7 @@ export const DiffPanel: React.FC<DiffPanelProps> = ({
const lastGitFingerprintRef = useRef<string | null>(null);
const wasActiveRef = useRef(isActive);
const reviewUrl = useMemo(() => buildGithubReviewUrl(session?.gitStatus?.prUrl), [session?.gitStatus?.prUrl]);
const effectiveReviewMode = getEffectiveReviewMode(reviewMode, !!reviewUrl);

useEffect(() => subscribeReviewDefaultMode(setReviewModeState), []);

Expand Down Expand Up @@ -154,7 +157,7 @@ export const DiffPanel: React.FC<DiffPanelProps> = ({
const becameActive = isActive && !wasActiveRef.current;
wasActiveRef.current = isActive;

if (becameActive && isStale && reviewMode === 'local') {
if (becameActive && isStale && effectiveReviewMode === 'local') {
setIsStale(false);
combinedDiffRef.current?.refresh();

Expand Down Expand Up @@ -188,63 +191,65 @@ export const DiffPanel: React.FC<DiffPanelProps> = ({
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 (
<div className="diff-panel h-full flex flex-col bg-bg-primary">
<div className="flex-1 flex items-center justify-center p-8 text-text-secondary">
<div className="max-w-sm text-center space-y-2">
<GitBranch className="w-8 h-8 mx-auto text-text-muted" />
<h2 className="text-sm font-medium text-text-primary">Review unavailable</h2>
<p className="text-xs text-text-tertiary">
Open a PR for this branch to review changes.
</p>
</div>
</div>
</div>
);
}
}, [effectiveReviewMode, isActive, isStale, panel.id, sessionId]);

const prLabel = session?.gitStatus?.prNumber ? `#${session.gitStatus.prNumber}` : 'Pull Request';
const githubModeButton = (
<button
type="button"
onClick={() => reviewUrl && handleReviewModeChange('github')}
disabled={!reviewUrl}
aria-disabled={!reviewUrl}
className={cn(
"inline-flex items-center gap-1 px-2 py-0.5 rounded text-xs transition-colors",
!reviewUrl
? "text-text-muted cursor-not-allowed opacity-50"
: effectiveReviewMode === 'github'
? "bg-interactive text-text-on-interactive"
: "text-text-secondary hover:bg-surface-hover"
)}
aria-pressed={effectiveReviewMode === 'github'}
title={!reviewUrl ? 'Open a PR to enable' : undefined}
>
<Globe className="w-3 h-3" />
GitHub
</button>
);

return (
<div className="diff-panel h-full flex flex-col bg-bg-primary">
<div className="flex items-center justify-between gap-3 px-3 py-1.5 border-b border-border-primary bg-surface-secondary flex-shrink-0">
<div className="flex items-center gap-2 min-w-0">
<GitBranch className="w-3.5 h-3.5 text-text-tertiary flex-shrink-0" />
<span className="text-xs font-medium text-text-secondary truncate">Review</span>
<span className="text-xs text-text-muted truncate">{prLabel}</span>
{session?.gitStatus?.prTitle && (
<span className="text-xs text-text-tertiary truncate">{session.gitStatus.prTitle}</span>
{reviewUrl ? (
<>
<span className="text-xs text-text-muted truncate">{prLabel}</span>
{session?.gitStatus?.prTitle && (
<span className="text-xs text-text-tertiary truncate">{session.gitStatus.prTitle}</span>
)}
</>
) : (
<span className="text-xs text-text-muted truncate">Local changes</span>
)}
</div>

<div className="inline-flex items-center rounded border border-border-primary bg-bg-primary p-0.5 flex-shrink-0">
<button
type="button"
onClick={() => handleReviewModeChange('github')}
className={cn(
"inline-flex items-center gap-1 px-2 py-0.5 rounded text-xs transition-colors",
reviewMode === 'github'
? "bg-interactive text-text-on-interactive"
: "text-text-secondary hover:bg-surface-hover"
)}
aria-pressed={reviewMode === 'github'}
>
<Globe className="w-3 h-3" />
GitHub
</button>
{reviewUrl ? githubModeButton : (
<Tooltip content="Open a PR to enable" side="bottom">
{githubModeButton}
</Tooltip>
)}
<button
type="button"
onClick={() => handleReviewModeChange('local')}
className={cn(
"inline-flex items-center gap-1 px-2 py-0.5 rounded text-xs transition-colors",
reviewMode === 'local'
effectiveReviewMode === 'local'
? "bg-interactive text-text-on-interactive"
: "text-text-secondary hover:bg-surface-hover"
)}
aria-pressed={reviewMode === 'local'}
aria-pressed={effectiveReviewMode === 'local'}
>
<GitBranch className="w-3 h-3" />
Local
Expand All @@ -253,7 +258,7 @@ export const DiffPanel: React.FC<DiffPanelProps> = ({
</div>

{/* Stale indicator bar */}
{reviewMode === 'local' && isStale && !isActive && (
{effectiveReviewMode === 'local' && isStale && !isActive && (
<div className="bg-status-warning/10 border-b border-status-warning/30 px-3 py-2 flex items-center justify-between">
<div className="flex items-center gap-2 text-status-warning text-sm">
<AlertCircle className="w-4 h-4" />
Expand All @@ -264,7 +269,7 @@ export const DiffPanel: React.FC<DiffPanelProps> = ({

{/* Main diff view */}
<div className="flex-1 overflow-hidden">
{reviewMode === 'github' ? (
{effectiveReviewMode === 'github' && reviewUrl ? (
<BrowserSurface
panelId={panel.id}
sessionId={sessionId}
Expand Down
24 changes: 24 additions & 0 deletions frontend/src/components/panels/diff/reviewModePreference.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { describe, expect, it } from 'vitest';
import { getEffectiveReviewMode } from './reviewModePreference';

describe('getEffectiveReviewMode', () => {
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');
});
});
4 changes: 4 additions & 0 deletions frontend/src/components/panels/diff/reviewModePreference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } }));
Expand Down
Loading