diff --git a/web/packages/gallery/components/viewer/FileViewer.tsx b/web/packages/gallery/components/viewer/FileViewer.tsx index e9d5b7bb28..2bae740a44 100644 --- a/web/packages/gallery/components/viewer/FileViewer.tsx +++ b/web/packages/gallery/components/viewer/FileViewer.tsx @@ -781,30 +781,32 @@ export const FileViewer: React.FC = ({ // Modify the active annotated file if we found a file with // the same ID in the (possibly) updated files array. // - // Note the omission of the PhotoSwipe refresh: we don't - // refresh the PhotoSwipe dialog itself since that would - // cause the user to lose their pan / zoom etc. - // // This is not correct in its full generality, but it works - // fine in the specific cases we would need to handle: + // fine in the specific cases we would need to handle (and + // we want to avoid refreshing the entire UI unnecessarily + // lest the user lose their zoom/pan etc): // - // - In case of delete, we'll not get to this code branch. + // - In case of delete or toggling archived that caused the + // file is no longer part of the list that is shown, we'll + // not get to this code branch. // - // - In case of toggling archive, just updating the file - // attribute is enough, the UI state is derived from it; - // none of the other attributes of the annotated file - // currently depend on the archive status change. + // - In case of toggling archive otherwise, just updating + // the file attribute is enough, the UI state is derived + // from it; none of the other attributes of the annotated + // file currently depend on the archive status change. af = { ...af, file: updatedFile }; } else { - // Refreshing the current slide after the current file has - // gone will show the subsequent slide (since that would've - // now moved down to the current index). + // The file we were displaying is no longer part of the list + // of files that should be displayed. Refresh the slides, + // adjusting the indexes as necessary. // - // However, we might've been the last slide, in which case - // we need to go back one slide first. To determine this, - // also pass the expected count of files to our PhotoSwipe - // wrapper. - psRef.current?.refreshCurrentSlideContent(files.length); + // A special case is when we might've been the last slide, + // in which case we need to go back one slide first. To + // determine this, also pass the expected count of files to + // our PhotoSwipe wrapper. + psRef.current?.refreshCurrentSlideContentAfterRemove( + files.length, + ); } } else { // If there are no more files left, close the viewer. diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 971be1f046..13b590d206 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -1390,17 +1390,43 @@ export class FileViewerPhotoSwipe { /** * Reload the current slide, asking the data source for its data afresh. + */ + refreshCurrentSlideContent() { + this.pswp.refreshSlideContent(this.pswp.currIndex); + } + + /** + * Reload the PhotoSwipe dialog (without recreating it) if the current slide + * that was being viewed is no longer part of the list of files that should + * be shown. This can happen when the user deleted the file, or if they + * marked it archived in a context (like "All") where archived files are not + * being shown. * * @param expectedFileCount The count of files that we expect to show after - * the refresh. If provided, this is used to (circle) go back to the first - * slide when the slide which we were at previously is not available anymore - * (e.g. when deleting the last file in a sequence). + * the refresh. */ - refreshCurrentSlideContent(expectedFileCount?: number) { - if (expectedFileCount && this.pswp.currIndex >= expectedFileCount) { + refreshCurrentSlideContentAfterRemove(newFileCount: number) { + // Refresh the slide, and its subsequent neighbour. + // + // To see why, consider item at index 3 was removed. After refreshing, + // the contents of the item previously at index 4, and now at index 3, + // would be displayed. But the preloaded slide next to us (showing item + // at index 4) would already be displaying the same item, so that also + // needs to be refreshed to displaying the item previously at index 5 + // (now at index 4). + const refreshSlideAndNextNeighbour = (i: number) => { + this.pswp.refreshSlideContent(i); + this.pswp.refreshSlideContent(i + 1 == newFileCount ? 0 : i + 1); + }; + + if (this.pswp.currIndex >= newFileCount) { + // If the last slide was removed, go back to the beginning (the code + // that calls us ensures that we don't get called if there are no + // more slides left). this.pswp.goTo(0); + refreshSlideAndNextNeighbour(0); } else { - this.pswp.refreshSlideContent(this.pswp.currIndex); + refreshSlideAndNextNeighbour(this.pswp.currIndex); } }