Skip to content

Commit fcfba0d

Browse files
nshcrPavelLaptevByronCopilot
authored
feat: add ImageDiff component to load and show before/after images (#11456)
* feat: add ImageDiff component to load and show before/after images * Add documentation for get_blob_file function Added a doc comment to the get_blob_file function explaining its purpose, usage, and arguments. This improves code clarity for developers working with image diff previews and blob retrieval. * Improve alt text for image diff panels Updated the alt attributes for before and after images to include the file path and context, enhancing accessibility and clarity for screen readers. * Improve image load error messages in ImageDiff Enhanced error handling to provide more specific messages when before or after images fail to load. Error messages now indicate which image failed and include error details from exceptions. * Add abort support to image loading in ImageDiff Introduces AbortSignal handling to image loading functions, allowing image fetches to be cancelled if the component unmounts or the effect is cleaned up. This improves resource management and prevents unnecessary work when the component is no longer active. * fix formatting * Refactor and enhance ImageDiff component Replaces the inline image diff logic in the desktop app with a new reusable ImageDiff component in the UI package, supporting 2-up, swipe, and onion-skin comparison modes, metadata display, and loading states. Updates RangeInput to use a simpler, more accessible range fill style. Adjusts EmptyStatePlaceholder and SkeletonBone styles for consistency. Adds Storybook stories for ImageDiff. Updates exports in the UI index. * refactor Rust * use `open_isolated_repo()` to indicate we just need ODB access and nothing else matters. * don't make the impression we support anything but full hashes to refer to objects. The UI knows these. * Fix typo in ImageDiff placeholder class name * Handle image metadata loading errors in ImageDiff Added error handling to the image metadata loading logic in ImageDiff.svelte. Now, if loading metadata fails for either image, an error is logged and the corresponding metadata is set to null. * Update ImageDiff.svelte * Add note on AbortSignal usage in loadImage function Clarified that backend service methods do not support AbortSignal for IO cancellation, and the signal is only used to prevent processing after abortion in the loadImage function. * Improve swipe image comparison interaction Refactored the swipe comparison mode to support drag interaction using mouse and touch events. Added a visual handle for the swipe divider, improved accessibility with ARIA attributes, and separated swipe and onion-skin modes for clearer logic and styling. * Refactor isImageFile to shared utils Moved the isImageFile function from UnifiedDiffView.svelte to packages/shared/src/lib/utils/file.ts for reuse and improved maintainability. UnifiedDiffView now imports isImageFile from the shared utils module. * Refactor ImageDiff story to use sample images Moves sample image data URLs to the top of the file and sets them as default args for the ImageDiff story. This improves demonstration clarity and removes redundant code. * Add keyboard controls to image diff slider Introduces keyboard navigation for the image comparison slider, allowing users to adjust the swipe position using arrow keys, Home, End, and Shift for larger steps. Enhances accessibility and usability for keyboard users. * Remove col-resize cursor from ImageDiff header Eliminates the col-resize cursor style from the header element in ImageDiff.svelte, likely to improve UX or clarify that the header is not resizable. * Update apps/desktop/src/components/ImageDiff.svelte Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update crates/but-api/src/legacy/repo.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix TouchEvent check in handleDragMove Added a typeof check for TouchEvent before using it in handleDragMove to prevent errors in environments where TouchEvent is undefined. --------- Co-authored-by: Pavel Laptev <pawellaptew@gmail.com> Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 66e5cb4 commit fcfba0d

File tree

13 files changed

+912
-128
lines changed

13 files changed

+912
-128
lines changed
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
<script lang="ts">
2+
import emptyFileSvg from '$lib/assets/empty-state/empty-file.svg?raw';
3+
import { FILE_SERVICE } from '$lib/files/fileService';
4+
import { inject } from '@gitbutler/core/context';
5+
import { ImageDiff, EmptyStatePlaceholder } from '@gitbutler/ui';
6+
import type { TreeChange } from '$lib/hunks/change';
7+
8+
type Props = {
9+
projectId: string;
10+
change: TreeChange;
11+
/** If provided, this is a commit diff (not a workspace diff). */
12+
commitId?: string;
13+
};
14+
15+
type ImageSource =
16+
| { type: 'workspace'; path: string }
17+
| { type: 'commit'; path: string; commitId: string }
18+
| { type: 'blob'; path: string; blobId: string };
19+
20+
type LoadStrategy = {
21+
before: ImageSource | null;
22+
after: ImageSource | null;
23+
};
24+
25+
const { projectId, change, commitId }: Props = $props();
26+
const fileService = inject(FILE_SERVICE);
27+
28+
let beforeImageUrl = $state<string | null>(null);
29+
let afterImageUrl = $state<string | null>(null);
30+
let loadError = $state<string | null>(null);
31+
let isLoading = $state<boolean>(true);
32+
33+
// Decide image sources for before/after panels without changing logic.
34+
function getLoadStrategy(): LoadStrategy {
35+
const { status, path } = change;
36+
const isCommitDiff = !!commitId;
37+
38+
switch (status.type) {
39+
case 'Addition':
40+
return isCommitDiff
41+
? { before: null, after: { type: 'commit' as const, path, commitId: commitId! } }
42+
: { before: null, after: { type: 'workspace' as const, path } };
43+
44+
case 'Deletion':
45+
return isCommitDiff
46+
? { before: { type: 'commit' as const, path, commitId: `${commitId}^` }, after: null }
47+
: {
48+
before: { type: 'blob' as const, path, blobId: status.subject.previousState.id },
49+
after: null
50+
};
51+
52+
case 'Modification':
53+
return isCommitDiff
54+
? {
55+
before: { type: 'commit' as const, path, commitId: `${commitId}^` },
56+
after: { type: 'commit' as const, path, commitId: commitId! }
57+
}
58+
: {
59+
before: { type: 'blob' as const, path, blobId: status.subject.previousState.id },
60+
after: { type: 'workspace' as const, path }
61+
};
62+
63+
case 'Rename':
64+
return isCommitDiff
65+
? {
66+
before: {
67+
type: 'commit' as const,
68+
path: status.subject.previousPath,
69+
commitId: `${commitId}^`
70+
},
71+
after: { type: 'commit' as const, path, commitId: commitId! }
72+
}
73+
: {
74+
before: {
75+
type: 'blob' as const,
76+
path: status.subject.previousPath,
77+
blobId: status.subject.previousState.id
78+
},
79+
after: { type: 'workspace' as const, path }
80+
};
81+
}
82+
}
83+
84+
/**
85+
* Load image from workspace, commit, or blob.
86+
*
87+
* @param source - The image source to load.
88+
* @param signal - Optional AbortSignal. Note: The backend service methods
89+
* (readFromWorkspace, readFromCommit, readFromBlob) do NOT support AbortSignal,
90+
* so the actual IO operations cannot be cancelled once started. The signal is
91+
* only used to prevent processing results after abortion, not to abort the IO itself.
92+
*
93+
* @returns A data URL string or null.
94+
*
95+
* @remarks
96+
* This function cannot cancel backend IO operations. If abort support is needed,
97+
* the backend service methods must be updated to accept and honor AbortSignal.
98+
* See: https://github.com/gitbutlerapp/gitbutler/issues (file a follow-up if needed)
99+
*/
100+
async function loadImage(
101+
source: ImageSource | null,
102+
signal?: AbortSignal
103+
): Promise<string | null> {
104+
if (!source) return null;
105+
106+
try {
107+
let fileInfo;
108+
109+
if (source.type === 'workspace') {
110+
const { data } = await fileService.readFromWorkspace(source.path, projectId);
111+
fileInfo = data;
112+
} else if (source.type === 'commit') {
113+
fileInfo = await fileService.readFromCommit(source.path, projectId, source.commitId);
114+
} else {
115+
// type === 'blob'
116+
fileInfo = await fileService.readFromBlob(source.path, projectId, source.blobId);
117+
}
118+
119+
if (signal?.aborted) return null;
120+
121+
if (fileInfo?.content && fileInfo?.mimeType) {
122+
return `data:${fileInfo.mimeType};base64,${fileInfo.content}`;
123+
}
124+
125+
return null;
126+
} catch (err) {
127+
if (signal?.aborted) return null;
128+
console.warn(`Failed to load image from ${source.type}: ${source.path}`, err);
129+
return null;
130+
}
131+
}
132+
133+
// Load both images according to the strategy.
134+
async function loadImages(signal?: AbortSignal) {
135+
isLoading = true;
136+
loadError = null;
137+
beforeImageUrl = null;
138+
afterImageUrl = null;
139+
140+
try {
141+
const strategy = getLoadStrategy();
142+
143+
const [before, after] = await Promise.all([
144+
loadImage(strategy.before, signal),
145+
loadImage(strategy.after, signal)
146+
]);
147+
148+
if (signal?.aborted) return;
149+
150+
beforeImageUrl = before;
151+
afterImageUrl = after;
152+
153+
if (!before && !after) {
154+
loadError = 'Failed to load both images (before and after).';
155+
} else if (!before && strategy.before) {
156+
loadError = 'Failed to load before image.';
157+
} else if (!after && strategy.after) {
158+
loadError = 'Failed to load after image.';
159+
}
160+
} catch (err) {
161+
console.error('Failed to load images:', err);
162+
loadError = `Failed to load images: ${err instanceof Error ? err.message : String(err)}`;
163+
} finally {
164+
isLoading = false;
165+
}
166+
}
167+
168+
$effect(() => {
169+
const abortController = new AbortController();
170+
loadImages(abortController.signal);
171+
return () => abortController.abort();
172+
});
173+
</script>
174+
175+
{#if loadError}
176+
<div class="imagediff-placeholder">
177+
<EmptyStatePlaceholder image={emptyFileSvg} gap={12} topBottomPadding={34}>
178+
{#snippet caption()}
179+
Can't preview this file type
180+
{/snippet}
181+
</EmptyStatePlaceholder>
182+
</div>
183+
{:else}
184+
<ImageDiff {beforeImageUrl} {afterImageUrl} fileName={change.path} {isLoading} />
185+
{/if}
186+
187+
<style lang="scss">
188+
.imagediff-placeholder {
189+
display: flex;
190+
align-items: center;
191+
justify-content: center;
192+
width: 100%;
193+
height: 200px;
194+
border: 1px solid var(--clr-border-3);
195+
border-radius: var(--radius-m);
196+
}
197+
</style>

apps/desktop/src/components/UnifiedDiffView.svelte

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<script lang="ts">
22
import HunkContextMenu from '$components/HunkContextMenu.svelte';
3+
import ImageDiff from '$components/ImageDiff.svelte';
34
import LargeDiffMessage from '$components/LargeDiffMessage.svelte';
45
import LineLocksWarning from '$components/LineLocksWarning.svelte';
56
import ReduxResult from '$components/ReduxResult.svelte';
@@ -21,7 +22,7 @@
2122
import { SETTINGS } from '$lib/settings/userSettings';
2223
import { UI_STATE } from '$lib/state/uiState.svelte';
2324
import { inject } from '@gitbutler/core/context';
24-
25+
import { isImageFile } from '@gitbutler/shared/utils/file';
2526
import { EmptyStatePlaceholder, HunkDiff, TestId } from '@gitbutler/ui';
2627
import { DRAG_STATE_SERVICE } from '@gitbutler/ui/drag/dragStateService.svelte';
2728
import { parseHunk } from '@gitbutler/ui/utils/diffParsing';
@@ -317,13 +318,17 @@
317318
</EmptyStatePlaceholder>
318319
</div>
319320
{:else if diff.type === 'Binary'}
320-
<div class="hunk-placehoder">
321-
<EmptyStatePlaceholder image={binarySvg} gap={12} topBottomPadding={34}>
322-
{#snippet caption()}
323-
Binary! Not for human eyes
324-
{/snippet}
325-
</EmptyStatePlaceholder>
326-
</div>
321+
{#if isImageFile(change.path)}
322+
<ImageDiff {projectId} {change} {commitId} />
323+
{:else}
324+
<div class="hunk-placehoder">
325+
<EmptyStatePlaceholder image={binarySvg} gap={12} topBottomPadding={34}>
326+
{#snippet caption()}
327+
Binary! Not for human eyes
328+
{/snippet}
329+
</EmptyStatePlaceholder>
330+
</div>
331+
{/if}
327332
{/if}
328333
</div>
329334
{/snippet}

apps/desktop/src/lib/files/fileService.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,22 @@ export class FileService {
2626
};
2727
}
2828

29+
async readFromCommit(filePath: string, projectId: string, commitId: string): Promise<FileInfo> {
30+
return await this.backend.invoke('get_commit_file', {
31+
relativePath: filePath,
32+
projectId: projectId,
33+
commitId: commitId
34+
});
35+
}
36+
37+
async readFromBlob(filePath: string, projectId: string, blobId: string): Promise<FileInfo> {
38+
return await this.backend.invoke('get_blob_file', {
39+
relativePath: filePath,
40+
projectId: projectId,
41+
blobId: blobId
42+
});
43+
}
44+
2945
async readFile(path: string): Promise<Uint8Array> {
3046
return await this.backend.readFile(path);
3147
}

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

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::path::PathBuf;
22

3+
use crate::json::HexHash;
34
use anyhow::{Context as _, Result};
45
use but_api_macros::but_api;
56
use but_core::DiffSpec;
@@ -70,11 +71,10 @@ pub fn get_uncommitted_files(project_id: ProjectId) -> Result<Vec<RemoteBranchFi
7071
pub fn get_commit_file(
7172
project_id: ProjectId,
7273
relative_path: PathBuf,
73-
commit_id: String,
74+
commit_id: HexHash,
7475
) -> Result<FileInfo> {
7576
let project = gitbutler_project::get(project_id)?;
76-
let commit_id = git2::Oid::from_str(&commit_id).map_err(anyhow::Error::from)?;
77-
project.read_file_from_commit(commit_id, &relative_path)
77+
project.read_file_from_commit(commit_id.0.to_git2(), &relative_path)
7878
}
7979

8080
#[but_api]
@@ -84,6 +84,28 @@ pub fn get_workspace_file(project_id: ProjectId, relative_path: PathBuf) -> Resu
8484
project.read_file_from_workspace(&relative_path)
8585
}
8686

87+
/// Retrieves file content directly from a Git blob object by its blob ID.
88+
///
89+
/// This function is used for displaying image diff previews when the file
90+
/// isn't available in the current workspace or a specific commit (e.g., for
91+
/// deleted files or when comparing against a previous state).
92+
///
93+
/// # Arguments
94+
/// * `blob_id` - Git blob object ID as a hexadecimal string
95+
#[but_api]
96+
#[instrument(err(Debug))]
97+
pub fn get_blob_file(
98+
project_id: ProjectId,
99+
relative_path: PathBuf,
100+
blob_id: HexHash,
101+
) -> Result<FileInfo> {
102+
let project = gitbutler_project::get(project_id)?;
103+
let repo = project.open_isolated_repo()?;
104+
let object = repo.find_object(blob_id).context("Failed to find blob")?;
105+
let blob = object.try_into_blob().context("Object is not a blob")?;
106+
Ok(FileInfo::from_content(&relative_path, &blob.data))
107+
}
108+
87109
#[but_api]
88110
#[instrument(err(Debug))]
89111
pub fn pre_commit_hook(

crates/but-server/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,7 @@ async fn handle_command(
346346
"get_uncommitted_files" => legacy::repo::get_uncommitted_files_cmd(request.params),
347347
"get_commit_file" => legacy::repo::get_commit_file_cmd(request.params),
348348
"get_workspace_file" => legacy::repo::get_workspace_file_cmd(request.params),
349+
"get_blob_file" => legacy::repo::get_blob_file_cmd(request.params),
349350
"find_files" => legacy::repo::find_files_cmd(request.params),
350351
"pre_commit_hook" => legacy::repo::pre_commit_hook_cmd(request.params),
351352
"pre_commit_hook_diffspecs" => legacy::repo::pre_commit_hook_diffspecs_cmd(request.params),

crates/gitbutler-tauri/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ fn main() -> anyhow::Result<()> {
240240
legacy::repo::tauri_get_uncommitted_files::get_uncommitted_files,
241241
legacy::repo::tauri_get_commit_file::get_commit_file,
242242
legacy::repo::tauri_get_workspace_file::get_workspace_file,
243+
legacy::repo::tauri_get_blob_file::get_blob_file,
243244
legacy::repo::tauri_find_files::find_files,
244245
legacy::repo::tauri_pre_commit_hook::pre_commit_hook,
245246
legacy::repo::tauri_pre_commit_hook_diffspecs::pre_commit_hook_diffspecs,

packages/shared/src/lib/utils/file.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,21 @@ export function getFilePathInfo(filePath: string): FilePathInfo | undefined {
4040

4141
return { fileName, extension, directoryPath };
4242
}
43+
44+
const IMAGE_EXTENSIONS = [
45+
'.png',
46+
'.jpg',
47+
'.jpeg',
48+
'.gif',
49+
'.webp',
50+
'.bmp',
51+
'.ico',
52+
'.heic',
53+
'.heif',
54+
'.avif'
55+
];
56+
57+
export function isImageFile(path: string): boolean {
58+
const lowerPath = path.toLowerCase();
59+
return IMAGE_EXTENSIONS.some((ext) => lowerPath.endsWith(ext));
60+
}

packages/ui/src/lib/components/EmptyStatePlaceholder.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696
}
9797
9898
.empty-state__caption {
99-
color: var(--clr-scale-ntrl-50);
99+
color: var(--clr-text-2);
100100
opacity: 0.6;
101101
}
102102

0 commit comments

Comments
 (0)