Skip to content

fix: reset image diff preview on file change#2010

Merged
arnestrickmann merged 1 commit into
mainfrom
jan/eng-1236-image-diff-viewer-traps-navigation-after-opening-image
May 13, 2026
Merged

fix: reset image diff preview on file change#2010
arnestrickmann merged 1 commit into
mainfrom
jan/eng-1236-image-diff-viewer-traps-navigation-after-opening-image

Conversation

@janburzinski
Copy link
Copy Markdown
Collaborator

summary

  • fix you getting trapped

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 13, 2026

Greptile Summary

This PR fixes the image diff preview getting stuck when switching between files by adding a key prop to the wrapper div in FileDiffView, forcing React to fully remount child components whenever the active file changes.

  • The composite key (workspaceId:group:path) correctly identifies a unique file slot, so React destroys and recreates all children — including ImageDiffView — on every file switch, clearing any stale internal state.
  • The key is placed on the shared container, meaning StickyDiffEditor (Monaco) is also remounted on each file change, not just the image diff view; this is broader than the stated fix and may introduce a brief re-render cost on text-diff navigation.

Confidence Score: 4/5

Safe to merge; the change is a small, targeted fix using a well-understood React pattern.

The fix correctly uses a composite key to reset child state on file change. The only open question is whether remounting the Monaco editor on every text-file switch is intentional — it is broader than the image-preview bug it was written to address and could produce a visible flash on navigation, but it does not break correctness.

src/renderer/features/tasks/diff-view/main-panel/file-diff-view.tsx — worth confirming the Monaco remount behaviour on text-file switching is acceptable.

Important Files Changed

Filename Overview
src/renderer/features/tasks/diff-view/main-panel/file-diff-view.tsx Adds a composite key prop (workspaceId:group:path) to the wrapper div so React fully remounts all child components (including ImageDiffView and StickyDiffEditor) whenever the active file changes, resetting any stale local state.

Sequence Diagram

sequenceDiagram
    participant User
    participant FileDiffView
    participant ReactDOM
    participant ImageDiffView
    participant StickyDiffEditor

    User->>FileDiffView: Select a new file
    Note over FileDiffView: activeFileKey changes (workspaceId:group:path)
    FileDiffView->>ReactDOM: Render div with new key
    ReactDOM->>ImageDiffView: Unmount (old key)
    ReactDOM->>StickyDiffEditor: Unmount (old key)
    Note over ImageDiffView,StickyDiffEditor: Internal state cleared
    ReactDOM->>StickyDiffEditor: Remount (new key)
    ReactDOM->>ImageDiffView: Remount (new key)
    Note over ImageDiffView: Image diff preview reset
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/renderer/features/tasks/diff-view/main-panel/file-diff-view.tsx:182
**StickyDiffEditor also remounts on every file switch**

The `key` is placed on the shared wrapper div, so `StickyDiffEditor` is fully unmounted and remounted on each active-file change — not just `ImageDiffView`. Previously the text diff editor updated reactively through prop changes (`originalUri` / `modifiedUri`). After this change it tears down and rebuilds the Monaco editor instance on every file selection. This resets scroll position and editor-local state (likely desired) but also introduces a brief blank state while Monaco reinitialises. If only the image preview needed resetting, scoping the key directly to `ImageDiffView` would avoid this cost on the text-diff path.

Reviews (1): Last reviewed commit: "fix: reset image diff preview on file ch..." | Re-trigger Greptile

return (
<div className="file-diff-view flex h-full flex-col">
<div className="relative min-h-0 flex-1">
<div key={activeFileKey} className="relative min-h-0 flex-1">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 StickyDiffEditor also remounts on every file switch

The key is placed on the shared wrapper div, so StickyDiffEditor is fully unmounted and remounted on each active-file change — not just ImageDiffView. Previously the text diff editor updated reactively through prop changes (originalUri / modifiedUri). After this change it tears down and rebuilds the Monaco editor instance on every file selection. This resets scroll position and editor-local state (likely desired) but also introduces a brief blank state while Monaco reinitialises. If only the image preview needed resetting, scoping the key directly to ImageDiffView would avoid this cost on the text-diff path.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/renderer/features/tasks/diff-view/main-panel/file-diff-view.tsx
Line: 182

Comment:
**StickyDiffEditor also remounts on every file switch**

The `key` is placed on the shared wrapper div, so `StickyDiffEditor` is fully unmounted and remounted on each active-file change — not just `ImageDiffView`. Previously the text diff editor updated reactively through prop changes (`originalUri` / `modifiedUri`). After this change it tears down and rebuilds the Monaco editor instance on every file selection. This resets scroll position and editor-local state (likely desired) but also introduces a brief blank state while Monaco reinitialises. If only the image preview needed resetting, scoping the key directly to `ImageDiffView` would avoid this cost on the text-diff path.

How can I resolve this? If you propose a fix, please make it concise.

@janburzinski janburzinski force-pushed the jan/eng-1236-image-diff-viewer-traps-navigation-after-opening-image branch from d52ca09 to e097780 Compare May 13, 2026 17:20
@arnestrickmann arnestrickmann merged commit 583d62c into main May 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants