Skip to content
Merged
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
6 changes: 6 additions & 0 deletions apps/desktop/src/main/bootstrap/os-startup-tweaks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,16 @@ function applyWindowsStartupTweaks(): void {
* real keychain. Cost: cookie encryption degrades to a static key, but the key was already stored in
* plaintext, so no real loss. Removable once there's a proper Developer ID signature. Must be before
* app.whenReady().
* - disable-features MediaSessionService/HardwareMediaKeyHandling: Chromium's macOS "Now Playing" /
* media-session integration queries the MediaPlayer framework, which makes macOS prompt for "access
* Apple Music / your media library" on launch. We play no media and expose no now-playing controls, so
* this permission is unexpected and confusing; disabling the features stops Chromium from ever touching
* the media library. Must be before app.whenReady().
* - Prepend common CLI dirs to PATH (see augmentMacPath): must be before pr-agent probing / running.
*/
function applyMacStartupTweaks(): void {
app.commandLine.appendSwitch('use-mock-keychain');
app.commandLine.appendSwitch('disable-features', 'MediaSessionService,HardwareMediaKeyHandling');
augmentMacPath();
}

Expand Down
6 changes: 5 additions & 1 deletion apps/desktop/src/main/services/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@ function activateOnClick(e: PollNotificationEvent): () => void {
anchor:
// File-level comments (no line) aren't line-navigable → no anchor (clicking just opens the PR).
e.comment?.anchor && e.comment.anchor.line != null
? { path: e.comment.anchor.path, line: e.comment.anchor.line }
? {
path: e.comment.anchor.path,
line: e.comment.anchor.line,
side: e.comment.anchor.side,
}
: null,
});
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ export interface PrPanelProps {
pendingDiffNav?: {
runId?: string;
findingId?: string;
anchor: { path: string; startLine: number; endLine: number };
anchor: { path: string; startLine: number; endLine: number; side?: 'old' | 'new' };
} | null;
onDiffNavConsumed?: () => void;
onRequestDiffNav?: (target: {
runId?: string;
findingId?: string;
anchor: { path: string; startLine: number; endLine: number };
anchor: { path: string; startLine: number; endLine: number; side?: 'old' | 'new' };
}) => void;
/** External request to switch to a given tab (e.g. clicking a summary comment notification → 'activity'); cleared via onPendingTabConsumed after consumption. */
pendingTab?: PrTab | null;
Expand Down Expand Up @@ -233,7 +233,7 @@ export function PrPanel({
// File-level anchors (no line) aren't line-navigable; CommentItem doesn't make them clickable, guard anyway.
if (a.line == null) return;
onRequestDiffNav?.({
anchor: { path: a.path, startLine: a.line, endLine: a.line },
anchor: { path: a.path, startLine: a.line, endLine: a.line, side: a.side },
});
}}
/>
Expand All @@ -252,6 +252,7 @@ export function PrPanel({
path: d.anchor.path,
startLine: d.anchor.startLine,
endLine: d.anchor.endLine,
side: d.anchor.side,
},
});
}}
Expand Down Expand Up @@ -281,6 +282,7 @@ export function PrPanel({
path: d.anchor.path,
startLine: d.anchor.startLine,
endLine: d.anchor.endLine,
side: d.anchor.side,
},
});
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,43 +22,9 @@ const InlineCodeContext = lazy(() =>
import('./InlineCodeContext').then((m) => ({ default: m.InlineCodeContext })),
);

/**
* Structural equality comparison of the comment tree (by remoteId + body + version + edit/delete permissions + recursive replies). poll mostly returns
* comments with unchanged content: on equality, skip setState and keep the old reference so React bails out, avoiding pointless re-render of the whole
* comment tree (including inline Monaco) (refresh flicker).
*/
export function sameCommentList(a: readonly PrComment[], b: readonly PrComment[]): boolean {
if (a.length !== b.length) return false;
for (let i = 0; i < a.length; i++) {
const x = a[i]!;
const y = b[i]!;
if (
x.remoteId !== y.remoteId ||
x.body !== y.body ||
x.version !== y.version ||
x.canEdit !== y.canEdit ||
x.canDelete !== y.canDelete ||
!sameReactions(x.reactions, y.reactions) ||
!sameCommentList(x.replies, y.replies)
) {
return false;
}
}
return true;
}

/** Equality comparison of the reactions array (emoji + count + mine triple matching item by item): lets reaction changes after a toggle trigger a re-render. */
function sameReactions(a: PrComment['reactions'], b: PrComment['reactions']): boolean {
const x = a ?? [];
const y = b ?? [];
if (x.length !== y.length) return false;
for (let i = 0; i < x.length; i++) {
if (x[i]!.emoji !== y[i]!.emoji || x[i]!.count !== y[i]!.count || x[i]!.mine !== y[i]!.mine) {
return false;
}
}
return true;
}
// Structural comment-tree equality lives in the shared module so the diff view (useDiffComments) can reuse the same poll bail; re-exported here for the
// activity/comments-page importers that historically pulled it from this file.
export { sameCommentList } from '../shared/commentEquality';

/**
* Maximum indent level for nested replies: past this level recursion continues but **no further indent is added** (flattened display), avoiding
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { BackendErrorBanner, BackendErrorView, SyncProgress } from './DiffStatus
import { BlameColumn } from './blame/BlameColumn';
import { fileKey, type PendingCommitView } from './diff-types';
import {
useActualRenderSideBySide,
useBlame,
useChangedFiles,
useCommentZones,
Expand Down Expand Up @@ -60,7 +61,7 @@ interface DiffViewProps {
pendingNav?: {
runId?: string;
findingId?: string;
anchor: { path: string; startLine: number; endLine: number };
anchor: { path: string; startLine: number; endLine: number; side?: 'old' | 'new' };
} | null;
onNavConsumed?: () => void;
/**
Expand Down Expand Up @@ -107,6 +108,10 @@ export function DiffView({
const drafts = useDraftsForPr(pr.localId);
// Use state, not a ref: onMount fires asynchronously, so a state change is required to re-run the subsequent useEffect decoration logic.
const [diffEditor, setDiffEditor] = useState<MonacoEditor.IStandaloneDiffEditor | null>(null);
// The ACTUAL render mode: renderSideBySide is only the toolbar intent, but Monaco auto-degrades to inline when the
// pane is too narrow. Positioning (which inner editor a comment/draft zone, glyph, or "+" adder targets) must use
// this, not the intent, or old-side items land on the original editor Monaco has hidden. See useActualRenderSideBySide.
const actualSideBySide = useActualRenderSideBySide(diffEditor, renderSideBySide);

const progress = useSyncProgress(pr);
const { fileListWidth, startFileListResize } = useFileListWidth();
Expand Down Expand Up @@ -168,9 +173,16 @@ export function DiffView({
pendingNav,
onNavConsumed,
triggerAutoEdit,
// Intent prop: useDiffNav reads the actual mode live at reveal time to place old-side targets correctly.
renderSideBySide,
});
// Capture the Diff selection → selectionStore, so ChatPane can carry the selected code as implicit context into agent/ask questions.
useSelectionCapture({ diffEditor, selected, prLocalId: pr.localId, renderSideBySide });
useSelectionCapture({
diffEditor,
selected,
prLocalId: pr.localId,
renderSideBySide: actualSideBySide,
});

// sidebar mode: 'tree' (file tree) / 'search' (cross-file search), defaults to the file tree. Returns to 'tree' on PR switch.
const [sidebarMode, setSidebarMode] = useState<'tree' | 'search'>('tree');
Expand Down Expand Up @@ -237,7 +249,9 @@ export function DiffView({
attachmentBase,
prLocalId: pr.localId,
prWebUrl: pr.url,
renderSideBySide,
// The actual render mode, so old-side comment zones + glyph/tick markers land on the visible editor after an
// auto-degrade to unified (the reported "published comment lacks position" case).
renderSideBySide: actualSideBySide,
commentHardBreaks,
reactionsMode,
attachmentsEnabled,
Expand All @@ -254,7 +268,7 @@ export function DiffView({
selected,
prLocalId: pr.localId,
registerEditTrigger,
renderSideBySide,
renderSideBySide: actualSideBySide,
commentHardBreaks,
attachmentsEnabled,
mentionCandidates,
Expand All @@ -271,7 +285,8 @@ export function DiffView({
prLocalId: pr.localId,
platform: pr.platform,
scopeKind: scope.kind,
renderSideBySide,
// Actual mode: wire the old-side "+" adder only when the original editor is actually visible (not auto-degraded).
renderSideBySide: actualSideBySide,
readOnly,
triggerAutoEdit,
t,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import { DraftZone } from '../drafts/DraftZone';
/**
* Container for multiple drafts on the same line; each is an independent DraftZone (maintaining its own read/edit), separated by hr.
* onSave / onDelete call IPC drafts:update / drafts:delete here; after writing to disk the main side
* broadcasts a drafts:changed event → drafts-store refetches → DiffView's top-level useEffect rebuilds the
* zones (this component unmounts/remounts along with it).
* broadcasts a drafts:changed event → drafts-store refetches → useDraftZones' content effect calls the zone
* controller's update(), which **reconciles** this zone in place (keyed by draft id) rather than unmounting it, so a
* DraftZone whose editor is open keeps its text / focus across the refresh. Only a removed draft (deleted / published)
* unmounts.
*/
export function DraftZoneList({
drafts,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ export { useBlame, type BlameState } from './useBlame';
export { useDraftAutoEdit, type DraftAutoEdit } from './useDraftAutoEdit';
export { useDiffNav, type PendingNav, type PendingScroll } from './useDiffNav';
export { useCommentZones } from './useCommentZones';
export {
useActualRenderSideBySide,
isActualSideBySide,
} from './useActualRenderSideBySide';
export { useDiffOverviewMarks } from './useDiffOverviewMarks';
export { useDraftZones } from './useDraftZones';
export { useLineCommentAdder } from './useLineCommentAdder';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { useEffect, useState } from 'react';
import { type editor as MonacoEditor } from 'monaco-editor';

/**
* `renderSideBySide` on the toolbar is the user's *intent* (persisted toggle). Monaco auto-degrades side-by-side →
* inline when the pane is too narrow (`useInlineViewWhenSpaceIsLimited`, on by default), leaving the intent `true`
* while the actual layout is unified. Monaco reflects the real mode in the `.monaco-diff-editor` root node's
* `side-by-side` class (removed when downgrading to inline).
*
* Anything that positions content by editor side — which inner editor a comment/draft zone, glyph dot, overview tick,
* or reveal targets — must key off the **actual** mode, not the intent: in the auto-degraded state the original editor
* is hidden, so an old-side item routed there by intent lands on an invisible editor ("missing position"). This
* returns the actual mode: the intent AND the class being present.
*/
export function isActualSideBySide(
diffEditor: MonacoEditor.IStandaloneDiffEditor,
renderSideBySide: boolean,
): boolean {
if (!renderSideBySide) return false;
const el = diffEditor.getContainerDomNode().querySelector('.monaco-diff-editor');
return el ? el.classList.contains('side-by-side') : true;
}

/**
* Reactive {@link isActualSideBySide}: recomputes when Monaco's layout crosses the side-by-side ↔ inline breakpoint
* (`onDidLayoutChange`) or the diff recomputes (`onDidUpdateDiff`). rAF-coalesced and deferred so the class is read
* after Monaco has switched it (the layout event may precede the class flip). Returns the intent prop directly until
* the editor is available.
*/
export function useActualRenderSideBySide(
diffEditor: MonacoEditor.IStandaloneDiffEditor | null,
renderSideBySide: boolean,
): boolean {
const [actual, setActual] = useState(renderSideBySide);
useEffect(() => {
if (!diffEditor) {
setActual(renderSideBySide);
return;
}
const read = (): void => setActual(isActualSideBySide(diffEditor, renderSideBySide));
read();
let raf = 0;
const schedule = (): void => {
if (raf) return;
raf = requestAnimationFrame(() => {
raf = 0;
read();
});
};
// The original editor still emits layout changes as it collapses to / expands from hidden at the breakpoint;
// onDidUpdateDiff covers the first async diff settling after a file switch.
const layoutDisp = diffEditor.getOriginalEditor().onDidLayoutChange(schedule);
const diffDisp = diffEditor.onDidUpdateDiff(schedule);
return () => {
if (raf) cancelAnimationFrame(raf);
layoutDisp.dispose();
diffDisp.dispose();
};
}, [diffEditor, renderSideBySide]);
return actual;
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
renderHoverMd,
} from '../inline-comments/InlineCommentZone';
import { createInlineZones, type InlineZonesController } from '../zones/mountInlineZones';
import { remapOldByLineToModified } from '../zones/line-mapping';
import type { LoadedContent } from '../diff-types';

/**
Expand Down Expand Up @@ -158,16 +159,29 @@ export function useCommentZones(opts: {

const originalEditor = diffEditor.getOriginalEditor();
const modifiedEditor = diffEditor.getModifiedEditor();
const originalDecorations = originalEditor.createDecorationsCollection(
buildDecorations(oldByLine),
);
// Old-side markers: in side-by-side they sit on the (visible) original editor; in unified — including
// auto-degraded-from-side-by-side, since renderSideBySide here is the ACTUAL render mode — the original editor is
// hidden, so remap them onto the modified editor at the mapped line (mirroring how the zone bodies are routed in
// computeDesired), otherwise the glyph dot + overview tick vanish with the hidden editor.
const modifiedLines = new Map(newByLine);
let originalLines: Map<number, PrComment[]> | null = oldByLine;
if (!renderSideBySide && oldByLine.size > 0) {
originalLines = null;
const remappedOld = remapOldByLineToModified(diffEditor.getLineChanges() ?? [], oldByLine);
for (const [line, cs] of remappedOld) {
modifiedLines.set(line, [...(modifiedLines.get(line) ?? []), ...cs]);
}
}
const originalDecorations = originalLines
? originalEditor.createDecorationsCollection(buildDecorations(originalLines))
: null;
const modifiedDecorations = modifiedEditor.createDecorationsCollection(
buildDecorations(newByLine),
buildDecorations(modifiedLines),
);

return () => {
try {
originalDecorations.clear();
originalDecorations?.clear();
modifiedDecorations.clear();
} catch {
// editor already disposed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useEffect, useState } from 'react';
import type { PrComment, StoredPullRequest } from '@meebox/shared';
import { invoke, subscribe } from '../../../../../../api';
import { formatBackendError, type FormattedError } from '../../../../../../errors';
import { sameCommentList } from '../../shared/commentEquality';

export interface DiffCommentsState {
comments: PrComment[];
Expand Down Expand Up @@ -37,7 +38,9 @@ export function useDiffComments(
const fetchList = (force: boolean): void => {
invoke('diff:listComments', { localId: pr.localId, force })
.then((cs) => {
if (!cancelled) setComments(cs);
// poll mostly returns unchanged comments: keep the old array reference on structural equality so downstream memos
// (mentionCandidates) stay identity-stable and the inline draft/comment zones aren't torn down mid-edit (see useDraftZones).
if (!cancelled) setComments((prev) => (sameCommentList(prev, cs) ? prev : cs));
})
.catch((e: unknown) => {
if (!cancelled) {
Expand Down
Loading
Loading