feat: add ImageDiff component to load and show before/after images #11456

nshcr · 2025-12-04T15:17:51Z

This PR adds before-and-after visual diff previews for image files in the diff view.

Most of the code was generated with the help of Claude Sonnet 4.5, and was reviewed by me before submitting the PR to ensure code quality and correct functionality.

It covers the following scenarios:

Adding

Modifying

Renaming

Deleting

Displayed side-by-side when the window width is sufficient

vercel · 2025-12-04T15:17:57Z

@nshcr is attempting to deploy a commit to the GitButler Team on Vercel.

A member of the Team first needs to authorize it.

nshcr · 2025-12-04T15:22:20Z

apps/desktop/src/components/UnifiedDiffView.svelte

+		const imageExtensions = [
+			'.png',
+			'.jpg',
+			'.jpeg',
+			'.gif',
+			'.webp',
+			'.bmp',
+			'.ico',
+			'.heic',
+			'.heif',
+			'.avif'
+		];


For some image formats such as heic, heif, and avif, they may not render correctly on systems with older OS versions.
Maybe should consider dropping support for them.

BTW, svg files could also be displayed as image diffs.
However, they are currently treated as text, and I didn't add a special-case handling for them.

nshcr · 2025-12-04T15:24:05Z

crates/but-api/src/legacy/repo.rs

I avoided using project.open_git2() as I saw it is marked deprecated, and used the gix API instead.
I'm not entirely sure if this is the correct approach here.

I think it's a good call. The repo needs to be opened anyway, and then using gix is faster.
Since it's all in legacy-land, I think it's fine to do what's needed.

However, I don't think that anything but full hashes may be supported here. The frontend already has blob hashes for anything one would want to diff.

Also I think there is a way to read an arbitrary file from within the worktree, in case of files that want comparison between worktree/Git.

nshcr · 2025-12-04T15:27:52Z

@Byron — could you please help review the Rust changes? I added a new function to but-api and modified an existing one.

@PavelLaptev — could you please help review the frontend changes and adjust the styling so that the UI better matches GitButler's design?

Thanks in advance for taking the time to review this and for any feedback or improvements you can suggest!

nshcr · 2025-12-04T16:32:01Z

Oh, this PR could fix #9946.

Byron

Thanks a lot for getting this started, that's such a useful feature!

I just went ahead and made the changes I'd need to see with some explanation in the commit message. Probably I don't understand where the need for arbitrary revspecs is coming from.
With that it looks good on the Rust side.

Something I could use your help with is to understand how your are diffing worktree/Git changes. ~~I also vaguely remember the presence of a function to obtain a worktree file, but I can't find it now.~~ Ah, right, it's readFromWorkspace and it's already used.

Byron · 2025-12-04T18:10:35Z

crates/but-api/src/legacy/repo.rs

I think it's a good call. The repo needs to be opened anyway, and then using gix is faster.
Since it's all in legacy-land, I think it's fine to do what's needed.

However, I don't think that anything but full hashes may be supported here. The frontend already has blob hashes for anything one would want to diff.

Also I think there is a way to read an arbitrary file from within the worktree, in case of files that want comparison between worktree/Git.

Byron · 2025-12-04T18:16:08Z

Oh, and allow me unleash Copilot on this PR as I actually value what it has to say, most of the time 😅. Please feel free to ignore entirely.

Copilot

Pull request overview

This PR adds visual diff previews for image files in the diff view, enabling before-and-after comparisons for image changes. It introduces a new ImageDiff Svelte component and supporting backend functions to load image content from commits, workspaces, and blob objects. The feature intelligently detects image files by extension and displays them with appropriate visual treatment for additions, modifications, renames, and deletions.

Key Changes

Added get_blob_file Rust API function to retrieve file content directly from Git blob objects
Extended FileService with readFromCommit and readFromBlob methods for flexible image loading
Created new ImageDiff component with responsive layout and support for all Git change types

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`crates/but-api/src/legacy/repo.rs`	Added `get_blob_file` function to retrieve files by blob ID; enhanced `get_commit_file` to support refs like "HEAD"
`crates/gitbutler-tauri/src/main.rs`	Registered new `get_blob_file` command for Tauri app
`crates/but-server/src/lib.rs`	Registered new `get_blob_file` command for CLI/server
`apps/desktop/src/lib/files/fileService.ts`	Added `readFromCommit` and `readFromBlob` methods to support different image loading strategies
`apps/desktop/src/components/UnifiedDiffView.svelte`	Added `isImageFile` helper and conditional rendering to show `ImageDiff` for image files
`apps/desktop/src/components/ImageDiff.svelte`	New component implementing visual diff with before/after panels, responsive layout, and error handling

Copilot · 2025-12-04T18:23:25Z

apps/desktop/src/components/ImageDiff.svelte

+<script lang="ts">
+	import { FILE_SERVICE } from '$lib/files/fileService';
+	import { inject } from '@gitbutler/core/context';
+	import type { TreeChange } from '$lib/hunks/change';
+
+	type Props = {
+		projectId: string;
+		change: TreeChange;
+		/** If provided, this is a commit diff (not a workspace diff). */
+		commitId?: string;
+	};
+
+	type ImageSource =
+		| { type: 'workspace'; path: string }
+		| { type: 'commit'; path: string; commitId: string }
+		| { type: 'blob'; path: string; blobId: string };
+
+	type LoadStrategy = {
+		before: ImageSource | null;
+		after: ImageSource | null;
+	};
+
+	const { projectId, change, commitId }: Props = $props();
+	const fileService = inject(FILE_SERVICE);
+
+	let beforeImageUrl = $state<string | null>(null);
+	let afterImageUrl = $state<string | null>(null);
+	let loadError = $state<string | null>(null);
+
+	// Decide image sources for before/after panels without changing logic.
+	function getLoadStrategy(): LoadStrategy {
+		const { status, path } = change;
+		const isCommitDiff = !!commitId;
+
+		switch (status.type) {
+			case 'Addition':
+				return isCommitDiff
+					? { before: null, after: { type: 'commit' as const, path, commitId: commitId! } }
+					: { before: null, after: { type: 'workspace' as const, path } };
+
+			case 'Deletion':
+				return isCommitDiff
+					? { before: { type: 'commit' as const, path, commitId: `${commitId}^` }, after: null }
+					: {
+							before: { type: 'blob' as const, path, blobId: status.subject.previousState.id },
+							after: null
+						};
+
+			case 'Modification':
+				return isCommitDiff
+					? {
+							before: { type: 'commit' as const, path, commitId: `${commitId}^` },
+							after: { type: 'commit' as const, path, commitId: commitId! }
+						}
+					: {
+							before: { type: 'blob' as const, path, blobId: status.subject.previousState.id },
+							after: { type: 'workspace' as const, path }
+						};
+
+			case 'Rename':
+				return isCommitDiff
+					? {
+							before: {
+								type: 'commit' as const,
+								path: status.subject.previousPath,
+								commitId: `${commitId}^`
+							},
+							after: { type: 'commit' as const, path, commitId: commitId! }
+						}
+					: {
+							before: {
+								type: 'blob' as const,
+								path: status.subject.previousPath,
+								blobId: status.subject.previousState.id
+							},
+							after: { type: 'workspace' as const, path }
+						};
+		}
+	}
+
+	// Load image from workspace, commit, or blob.
+	async function loadImage(source: ImageSource | null): Promise<string | null> {
+		if (!source) return null;
+
+		try {
+			let fileInfo;
+
+			if (source.type === 'workspace') {
+				const { data } = await fileService.readFromWorkspace(source.path, projectId);
+				fileInfo = data;
+			} else if (source.type === 'commit') {
+				fileInfo = await fileService.readFromCommit(source.path, projectId, source.commitId);
+			} else {
+				// type === 'blob'
+				fileInfo = await fileService.readFromBlob(source.path, projectId, source.blobId);
+			}
+
+			if (fileInfo?.content && fileInfo?.mimeType) {
+				return `data:${fileInfo.mimeType};base64,${fileInfo.content}`;
+			}
+
+			return null;
+		} catch (err) {
+			console.warn(`Failed to load image from ${source.type}: ${source.path}`, err);
+			return null;
+		}
+	}
+
+	// Load both images according to the strategy.
+	async function loadImages() {
+		loadError = null;
+		beforeImageUrl = null;
+		afterImageUrl = null;
+
+		try {
+			const strategy = getLoadStrategy();
+
+			const [before, after] = await Promise.all([
+				loadImage(strategy.before),
+				loadImage(strategy.after)
+			]);
+
+			beforeImageUrl = before;
+			afterImageUrl = after;
+
+			if (!before && !after) {
+				loadError = 'Failed to load images';
+			}
+		} catch (err) {
+			console.error('Failed to load images:', err);
+			loadError = 'Failed to load images';
+		}
+	}
+
+	$effect(() => {
+		loadImages();
+	});
+</script>
+
+{#if loadError}
+	<div class="error-message">
+		<p>{loadError}</p>
+	</div>
+{:else}
+	<div class="image-diff-container">
+		{#if beforeImageUrl || afterImageUrl}
+			<div class="image-comparison">
+				{#if beforeImageUrl}
+					<div class="image-panel before">
+						<div class="image-header">
+							<span class="label">Before</span>
+						</div>
+						<div class="image-wrapper">
+							<img src={beforeImageUrl} alt="Before" />
+						</div>
+					</div>
+				{/if}
+
+				{#if afterImageUrl}
+					<div class="image-panel after">
+						<div class="image-header">
+							<span class="label">After</span>
+						</div>
+						<div class="image-wrapper">
+							<img src={afterImageUrl} alt="After" />
+						</div>
+					</div>
+				{/if}
+			</div>
+		{:else}
+			<div class="loading">Loading images...</div>
+		{/if}
+	</div>
+{/if}
+
+<style lang="postcss">
+	.error-message {
+		padding: 20px;
+		color: var(--clr-scale-warn-40);
+		text-align: center;
+	}
+
+	.image-diff-container {
+		padding: 14px;
+		border: 1px solid var(--clr-border-3);
+		border-radius: var(--radius-m);
+		background: var(--clr-bg-2);
+	}
+
+	.image-comparison {
+		display: grid;
+		grid-template-columns: repeat(auto-fit, minmax(300px, 1fr));
+		gap: 14px;
+	}
+
+	.image-panel {
+		overflow: hidden;
+		border-radius: var(--radius-s);
+		background: var(--clr-bg-1);
+
+		&.before {
+			border: 2px solid var(--clr-scale-err-40);
+		}
+
+		&.after {
+			border: 2px solid var(--clr-scale-succ-40);
+		}
+	}
+
+	.image-header {
+		padding: 8px 12px;
+		font-weight: 600;
+		font-size: 12px;
+		letter-spacing: 0.5px;
+		text-transform: uppercase;
+
+		.before & {
+			background: var(--clr-scale-err-40);
+			color: var(--clr-core-ntrl-100);
+		}
+
+		.after & {
+			background: var(--clr-scale-succ-40);
+			color: var(--clr-core-ntrl-100);
+		}
+	}
+
+	.label {
+		display: inline-block;
+	}
+
+	.image-wrapper {
+		display: flex;
+		align-items: center;
+		justify-content: center;
+		min-height: 200px;
+		padding: 14px;
+		background-image:
+			linear-gradient(45deg, var(--clr-bg-3) 25%, transparent 25%),
+			linear-gradient(-45deg, var(--clr-bg-3) 25%, transparent 25%),
+			linear-gradient(45deg, transparent 75%, var(--clr-bg-3) 75%),
+			linear-gradient(-45deg, transparent 75%, var(--clr-bg-3) 75%);
+		background-position:
+			0 0,
+			0 10px,
+			10px -10px,
+			-10px 0px;
+		background-size: 20px 20px;
+	}
+
+	.image-wrapper img {
+		display: block;
+		max-width: 100%;
+		max-height: 600px;
+		object-fit: contain;
+		border-radius: var(--radius-s);
+	}
+
+	.loading {
+		padding: 40px;
+		color: var(--clr-scale-ntrl-50);
+		text-align: center;
+	}
+</style>


The new ImageDiff component lacks test coverage. Given that the desktop app has comprehensive E2E testing (e.g., unifiedDiffView.cy.ts), this new feature should have corresponding tests to verify:

Image loading for different change types (Addition, Deletion, Modification, Rename)

Both workspace and commit diff scenarios

Error handling when images fail to load

Layout behavior for different window sizes

Consider adding E2E tests in a file like apps/desktop/cypress/e2e/imageDiff.cy.ts to cover these scenarios.

Copilot · 2025-12-04T18:23:25Z

apps/desktop/src/components/ImageDiff.svelte

+							<img src={beforeImageUrl} alt="Before" />
+						</div>
+					</div>
+				{/if}
+
+				{#if afterImageUrl}
+					<div class="image-panel after">
+						<div class="image-header">
+							<span class="label">After</span>
+						</div>
+						<div class="image-wrapper">
+							<img src={afterImageUrl} alt="After" />


The image alt attributes are too generic ("Before" and "After"). For better accessibility, the alt text should describe what the image contains or at least include the file name. Consider using something like alt="{change.path} (Before)" and alt="{change.path} (After)" to provide more context for screen reader users.

Copilot · 2025-12-04T18:23:25Z

crates/but-api/src/legacy/repo.rs

+#[but_api]
+#[instrument(err(Debug))]
+pub fn get_blob_file(
+    project_id: ProjectId,
+    relative_path: PathBuf,
+    blob_id: String,
+) -> Result<FileInfo> {


The get_blob_file function lacks documentation. Consider adding a doc comment to explain:

The purpose of this function (retrieving file content by blob ID)

When it's used (e.g., for image diff previews when the file isn't in the current workspace or a commit)

The meaning of the blob_id parameter (Git blob object ID as hex string)

This would be consistent with other similar functions in the file and help future maintainers understand the API.

A valid point - this will help when we eventually port it to non-legacy, which would already be possible actually.

Copilot · 2025-12-04T18:23:26Z

apps/desktop/src/components/ImageDiff.svelte

+	$effect(() => {
+		loadImages();
+	});


The $effect hook runs loadImages() on every reactive dependency change without any cleanup or cancellation mechanism. If the component props change rapidly (e.g., user quickly navigating between files), this could lead to race conditions or unnecessary API calls.

Consider:

Adding a cleanup function to cancel ongoing requests when the effect re-runs

Using an abort controller to cancel the previous request

Adding a derived state to detect actual changes that require reloading

Example:

$effect(() => { const abortController = new AbortController(); loadImages(abortController.signal); return () => abortController.abort(); });

Copilot · 2025-12-04T18:23:26Z

apps/desktop/src/components/UnifiedDiffView.svelte

+	function isImageFile(path: string): boolean {
+		const imageExtensions = [
+			'.png',
+			'.jpg',
+			'.jpeg',
+			'.gif',
+			'.webp',
+			'.bmp',
+			'.ico',
+			'.heic',
+			'.heif',
+			'.avif'
+		];
+		const lowerPath = path.toLowerCase();
+		return imageExtensions.some((ext) => lowerPath.endsWith(ext));
+	}


[nitpick] The isImageFile function is defined inline within the component, but this utility function would be better placed in a shared utilities file since it could be reused elsewhere in the codebase. Consider extracting this to a shared file like apps/desktop/src/lib/utils/files.ts or similar.

Additionally, the function could benefit from a more comprehensive list of image extensions. Consider adding .tiff/.tif for completeness.

Copilot · 2025-12-04T18:23:26Z

apps/desktop/src/components/ImageDiff.svelte

+				loadError = 'Failed to load images';
+			}
+		} catch (err) {
+			console.error('Failed to load images:', err);
+			loadError = 'Failed to load images';


The error message 'Failed to load images' is generic and doesn't provide enough context for debugging. Consider providing more specific error information, such as which image (before/after) failed to load, or passing through the actual error message from the caught exception.

Suggested change

loadError = 'Failed to load images';

}

} catch (err) {

console.error('Failed to load images:', err);

loadError = 'Failed to load images';

loadError = 'Failed to load both images (before and after).';

} else if (!before) {

loadError = 'Failed to load before image.';

} else if (!after) {

loadError = 'Failed to load after image.';

}

} catch (err) {

console.error('Failed to load images:', err);

loadError = `Failed to load images: ${err instanceof Error ? err.message : String(err)}`;

Copilot · 2025-12-04T18:23:27Z

apps/desktop/src/components/ImageDiff.svelte

+			}
+		} catch (err) {
+			console.error('Failed to load images:', err);
+			loadError = 'Failed to load images';


The error message 'Failed to load images' is generic and doesn't provide enough context for debugging. Consider providing more specific error information, such as which image (before/after) failed to load, or passing through the actual error message from the caught exception.

feat: add ImageDiff component to load and show before/after images

d311099

github-actions bot added rust Pull requests that update Rust code @gitbutler/desktop labels Dec 4, 2025

nshcr commented Dec 4, 2025

View reviewed changes

Byron reviewed Dec 4, 2025

View reviewed changes

Byron requested a review from Copilot December 4, 2025 18:16

Copilot started reviewing on behalf of Byron December 4, 2025 18:16 View session

Copilot finished reviewing on behalf of Byron December 4, 2025 18:20

Copilot AI reviewed Dec 4, 2025

View reviewed changes

feat: add ImageDiff component to load and show before/after images #11456

Are you sure you want to change the base?

feat: add ImageDiff component to load and show before/after images #11456

Conversation

nshcr commented Dec 4, 2025

Uh oh!

vercel bot commented Dec 4, 2025

Uh oh!

nshcr Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

nshcr Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Byron Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

nshcr commented Dec 4, 2025

Uh oh!

nshcr commented Dec 4, 2025

Uh oh!

Byron left a comment

Choose a reason for hiding this comment

Uh oh!

Byron Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Byron commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Byron Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Byron commented Dec 4, 2025 •

edited

Loading