fix(gallery): PhotoDetailPane's currentFile went stale after any metadata edit

One root cause behind three visible symptoms: GalleryView.updateRating
(favorite toggle and MediaInfoPane's star control), openDateDialog and
openLocationDialog all completed by refreshing the info panes but
never told PhotoDetailPane its own currentFile had just changed.
MediaFile is a record, and MediaLibraryState replaces that file's
entry in the shared list every other view reads from -- so the stale
value left behind no longer equals the fresh one, breaking:

1. A second favorite-star press (reads the stale, pre-edit rating,
   computes the same target rating again -- can never toggle back off)
2. ThumbnailGalleryPane.previous/next (an equals-based lookup against
   the now-refreshed list; the stale file is never found in it)
3. revealAndPulse on Escape/close (same lookup, same failure -- lands
   back on the grid without scrolling to the photo that was open)

PhotoDetailPane.updateCurrentFile(MediaFile) swaps in the fresh value
(and refreshes the favorite icon) whenever it's the same file (by id)
this pane is already showing; all three completion handlers in
GalleryView now call it instead of just refreshing the info panes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JzvA5ySQUsYrMUTj7sHxFA
This commit is contained in:
2026-09-17 22:26:14 -04:00
co-authored by Claude Sonnet 5
parent 4ae89c55e3
commit 9bcee4ada7
3 changed files with 99 additions and 5 deletions
@@ -477,6 +477,7 @@ public class GalleryView extends HBox implements Disposable, SelectionSource, Na
log.warn("Could not update captured date for media file {}", file.id(), error);
} else {
refreshMediaInfoPanes(updated);
detailPane.updateCurrentFile(updated);
}
})),
modalService::hide);
@@ -500,6 +501,7 @@ public class GalleryView extends HBox implements Disposable, SelectionSource, Na
log.warn("Could not update location for media file {}", file.id(), error);
} else {
refreshMediaInfoPanes(updated);
detailPane.updateCurrentFile(updated);
}
})),
modalService::hide);
@@ -528,8 +530,10 @@ public class GalleryView extends HBox implements Disposable, SelectionSource, Na
/**
* {@link MediaInfoPane#setOnEditRating}'s target, also used by {@link #toggleFavorite}. Mirrors
* {@link #openDateDialog}'s completion pattern, minus the modal — {@link StarRatingControl} has already
* shown the clicked value optimistically, this only persists it — plus {@link PhotoDetailPane#setFavorite}
* so its own star reflects the change immediately when {@code file} is the one currently open there.
* shown the clicked value optimistically, this only persists it — plus {@link PhotoDetailPane#updateCurrentFile}
* so that pane's own notion of "the current file" (its star icon included) never goes stale the moment
* this succeeds — see that method's own javadoc for the previous/next and reveal breakage a plain
* {@link PhotoDetailPane#setFavorite} left behind instead.
*/
private void updateRating(MediaFile file, int rating) {
metadataEditService.updateRating(file, rating)
@@ -539,9 +543,7 @@ public class GalleryView extends HBox implements Disposable, SelectionSource, Na
return;
}
refreshMediaInfoPanes(updated);
if (file.equals(detailPane.currentFile())) {
detailPane.setFavorite(updated.metadata() != null && updated.metadata().isFavorite());
}
detailPane.updateCurrentFile(updated);
}));
}
@@ -36,6 +36,7 @@ import org.kordamp.ikonli.javafx.FontIcon;
import org.kordamp.ikonli.material2.Material2MZ;
import java.nio.file.Path;
import java.util.Objects;
import java.util.function.Consumer;
/**
@@ -492,6 +493,26 @@ public class PhotoDetailPane extends StackPane implements Disposable {
return currentFile;
}
/**
* Swaps in {@code updated} as the file this pane is currently showing, without redoing any of
* {@link #show}'s own image work — the completion of a metadata-only edit (rating, GPS, capture date,
* ...), never a navigation. {@link #currentFile} would otherwise go stale the moment such an edit
* lands: {@code MediaLibraryState} replaces that same file's entry in the shared list every other view
* reads from, and {@code MediaFile} being a record means the value still cached here no longer equals
* it — breaking {@code ThumbnailGalleryPane.previous}/{@code next} and {@code revealAndPulse} alike (both
* an {@code equals}-based lookup) the next time either is asked to find this file, and leaving a second
* {@code GalleryView.toggleFavorite} press reading this pane's own stale rating. A no-op if
* {@code updated} is not the same file this pane already holds (by id) — the edit came from
* {@code MediaInfoPane}'s own grid-side control instead, a different file than whatever this pane, if
* even open at all, happens to be showing.
*/
public void updateCurrentFile(MediaFile updated) {
if (currentFile != null && Objects.equals(currentFile.id(), updated.id())) {
currentFile = updated;
refreshFavorite(updated);
}
}
/**
* {@code consumer} runs with {@link #imageCache}'s decoded {@link Image} for {@code absolutePath} —
* immediately, if already cached, or once a fresh decode completes otherwise; always on the FX thread
@@ -0,0 +1,71 @@
package org.icroco.pholio.ui.view.gallery;
import org.icroco.pholio.domain.library.MediaFile;
import org.icroco.pholio.domain.media.MediaMetadata;
import org.icroco.pholio.ui.FxTestToolkit;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import java.nio.file.Path;
import java.time.Instant;
import java.util.BitSet;
import static org.assertj.core.api.Assertions.assertThat;
import static org.icroco.pholio.ui.FxTestToolkit.onFxThread;
import static org.icroco.pholio.ui.FxTestToolkit.runOnFxThread;
import static org.mockito.Mockito.mock;
/**
* {@link PhotoDetailPane#updateCurrentFile} — the fix for a bug with three visible symptoms, all one root
* cause: {@link GalleryView#updateRating}/{@code openDateDialog}/{@code openLocationDialog}'s own
* {@code whenComplete} used to leave {@link PhotoDetailPane#currentFile()} pointing at the pre-edit
* {@link MediaFile} forever. Since {@code MediaFile} is a record, that stale value stops
* {@code .equals()}-ing the fresh one {@code MediaLibraryState} splices into every other view's own list the
* moment an edit lands — breaking {@code ThumbnailGalleryPane.previous}/{@code next} (can't navigate any
* more), a second favorite toggle (reads the stale, pre-edit rating and computes the same target rating
* again), and {@code revealAndPulse} on close (can't find the file to scroll to) all at once.
*/
class PhotoDetailPaneTest {
private PhotoDetailPane pane;
@BeforeEach
void setUp() {
FxTestToolkit.requireToolkit();
pane = onFxThread(() -> new PhotoDetailPane(mock(FullImageCache.class)));
}
@Test
void updateCurrentFileReplacesItWhenIdsMatch() {
MediaFile original = mediaFile(1L, false);
runOnFxThread(() -> pane.show(original, null));
MediaFile favorited = mediaFile(1L, true);
runOnFxThread(() -> pane.updateCurrentFile(favorited));
assertThat(pane.currentFile()).isSameAs(favorited);
}
@Test
void updateCurrentFileIsANoOpForADifferentFile() {
MediaFile original = mediaFile(1L, false);
runOnFxThread(() -> pane.show(original, null));
MediaFile another = mediaFile(2L, true);
runOnFxThread(() -> pane.updateCurrentFile(another));
assertThat(pane.currentFile()).isSameAs(original);
}
@Test
void updateCurrentFileIsANoOpBeforeAnyFileHasEverBeenShown() {
runOnFxThread(() -> pane.updateCurrentFile(mediaFile(1L, true)));
assertThat(pane.currentFile()).isNull();
}
private static MediaFile mediaFile(long id, boolean favorite) {
MediaMetadata metadata = MediaMetadata.builder().rating(favorite ? 5 : 0).build();
return new MediaFile(id, 1L, Path.of(id + ".jpg"), Instant.now(), Instant.now(), "hash" + id, null, metadata, new BitSet());
}
}