diff --git a/web/apps/photos/src/components/PhotoFrame.tsx b/web/apps/photos/src/components/PhotoFrame.tsx index b68f603744..d15a2c42b4 100644 --- a/web/apps/photos/src/components/PhotoFrame.tsx +++ b/web/apps/photos/src/components/PhotoFrame.tsx @@ -553,17 +553,15 @@ const PhotoFrame = ({ user={galleryContext.user ?? undefined} files={files} initialIndex={currentIndex} - isInTrashSection={activeCollectionID === TRASH_SECTION} - isInHiddenSection={isInHiddenSection} disableDownload={!enableDownload} + isInHiddenSection={isInHiddenSection} + isInTrashSection={activeCollectionID === TRASH_SECTION} onTriggerSyncWithRemote={handleTriggerSyncWithRemote} onSaveEditedImageCopy={handleSaveEditedImageCopy} {...{ favoriteFileIDs, fileCollectionIDs, allCollectionsNameByID, - onMarkUnsyncedFavoriteUpdate, - onMarkTempDeleted, onSelectCollection, onSelectPerson, }} diff --git a/web/packages/gallery/components/viewer/FileViewer.tsx b/web/packages/gallery/components/viewer/FileViewer.tsx index cc4f9cc19b..57c4831fd4 100644 --- a/web/packages/gallery/components/viewer/FileViewer.tsx +++ b/web/packages/gallery/components/viewer/FileViewer.tsx @@ -37,6 +37,7 @@ import { fileInfoExifForFile } from "./data-source"; import { FileViewerPhotoSwipe, type FileViewerAnnotatedFile, + type FileViewerFileAnnotation, type FileViewerPhotoSwipeDelegate, } from "./photoswipe"; @@ -120,28 +121,6 @@ export type FileViewerProps = ModalVisibilityProps & { * not defined, then this prop is not used. */ onTriggerSyncWithRemote?: () => void; - /** - * Called when the viewer wants to update the in-memory, unsynced, favorite - * status of a file maintained by the top level gallery. For more details, - * see {@link unsyncedFavoriteUpdates} in the gallery reducer's - * documentation. - * - * If this is not provided then the toggle favorite action will not be - * shown. - */ - onMarkUnsyncedFavoriteUpdate?: ( - fileID: number, - isFavorite: boolean, - ) => void; - /** - * Called when the viewer wants to mark the given files as deleted in the - * the in-memory, unsynced, state maintained by the top level gallery. For - * more details, see {@link unsyncedFavoriteUpdates} in the gallery - * reducer's documentation. - * - * If this is not provided then the delete action will not be shown. - */ - onMarkTempDeleted?: (files: EnteFile[]) => void; /** * Called when the user edits an image in the image editor and asks us to * save their edits as a copy. @@ -175,39 +154,36 @@ const FileViewer: React.FC = ({ favoriteFileIDs, fileCollectionIDs, allCollectionsNameByID, + onTriggerSyncWithRemote, onSelectCollection, onSelectPerson, - onTriggerSyncWithRemote, - onMarkUnsyncedFavoriteUpdate, - // TODO - // onMarkTempDeleted, onSaveEditedImageCopy, }) => { - // [Note: FileViewer architecture] - // - // There are 3 parties involved. + // There are 3 things involved in this dance: // // 1. Us, "FileViewer". We're a React component. + // 2. The custom PhotoSwipe wrapper, "FileViewerPhotoSwipe". It is a class. + // 3. The delegate, "FileViewerPhotoSwipeDelegate". // - // 2. The custom PhotoSwipe wrapper, "FileViewerPhotoSwipe". It is a class, - // and its currently active instance is maintained in `psRef`. + // The delegate acts as a bridge between us and (our custom) photoswipe + // class, to avoid recreating the class each time a "dynamic" prop changes. + // The delegate has a stable identity, we just keep updating the callback + // functions that it holds. // - // 3. The delegate, "FileViewerPhotoSwipeDelegate". The delegate acts as a - // bridge between us and `psRef`. It is created once as an object with a - // stable identity (and stored in `psDelegateRef`), but its properties - // keep changing as our props change. - // - // The `psRef` is recreated each time open / close changes. The - // `psDelegateRef` remains the same. - - const psRef = useRef(undefined); - const psDelegateRef = useRef( + // The word "dynamic" here means a prop on whose change we should not + // recreate the photoswipe dialog. + const delegateRef = useRef( undefined, ); + // We also need to maintain a ref to the currently displayed dialog since we + // might need to ask it to refresh its contents. + const psRef = useRef(undefined); + // Whenever we get a callback from our custom PhotoSwipe instance, we also - // get the active file on which that action was performed as an argument. - // Save it as a prop so that the rest of our React tree can use it. + // get the active file on which that action was performed as an argument. We + // save it as the `activeAnnotatedFile` state so that the rest of our React + // tree can use it. // // This is not guaranteed, or even intended, to be in sync with the active // file shown within the file viewer. All that this guarantees is this will @@ -215,8 +191,9 @@ const FileViewer: React.FC = ({ const [activeAnnotatedFile, setActiveAnnotatedFile] = useState< FileViewerAnnotatedFile | undefined >(undefined); - // With semantics similar to activeFile, this is the exif data associated - // with the activeAnnotatedFile, if any. + + // With semantics similar to `activeAnnotatedFile`, this is the exif data + // associated with the `activeAnnotatedFile`, if any. const [activeFileExif, setActiveFileExif] = useState< FileInfoExif | undefined >(undefined); @@ -238,55 +215,22 @@ const FileViewer: React.FC = ({ }, [onTriggerSyncWithRemote, onClose]); const handleAnnotate = useCallback( - (file: EnteFile) => { + (file: EnteFile): FileViewerFileAnnotation => { log.debug(() => ["viewer", { action: "annotate", file }]); const fileID = file.id; const isOwnFile = file.ownerID == user?.id; const canModify = isOwnFile && !isInTrashSection && !isInHiddenSection; - const isFavorite = - favoriteFileIDs && onMarkUnsyncedFavoriteUpdate && canModify - ? favoriteFileIDs.has(file.id) - : undefined; + const showFavorite = canModify; const isEditableImage = onSaveEditedImageCopy && canModify ? fileIsEditableImage(file) : undefined; - return { fileID, isOwnFile, isFavorite, isEditableImage }; + return { fileID, isOwnFile, showFavorite, isEditableImage }; }, - [ - user, - isInTrashSection, - isInHiddenSection, - favoriteFileIDs, - onMarkUnsyncedFavoriteUpdate, - onSaveEditedImageCopy, - ], + [user, isInTrashSection, isInHiddenSection, onSaveEditedImageCopy], ); - const handleToggleFavorite = useMemo(() => { - return favoriteFileIDs - ? ({ file, annotation }: FileViewerAnnotatedFile) => { - console.log({ file, annotation }); - // TODO - // const isFavorite = annotation.isFavorite; - // if (isFavorite === undefined) { - // assertionFailed(); - // return; - // } - - // onMarkUnsyncedFavoriteUpdate(file.id, !isFavorite); - // void (isFavorite ? removeFromFavorites : addToFavorites)( - // file, - // ).catch((e: unknown) => { - // log.error("Failed to remove favorite", e); - // onMarkUnsyncedFavoriteUpdate(file.id, undefined); - // }); - } - : undefined; - // }, [favoriteFileIDs, onMarkUnsyncedFavoriteUpdate]); - }, [favoriteFileIDs]); - const handleViewInfo = useCallback( (annotatedFile: FileViewerAnnotatedFile) => { setActiveAnnotatedFile(annotatedFile); @@ -344,70 +288,85 @@ const FileViewer: React.FC = ({ [onSaveEditedImageCopy, handleImageEditorClose, handleClose], ); - // Initial value of psDelegateRef. - if (!psDelegateRef.current) { - psDelegateRef.current = { - onClose: handleClose, - onAnnotate: handleAnnotate, - onToggleFavorite: handleToggleFavorite, - onViewInfo: handleViewInfo, - onEditImage: handleEditImage, - }; + const haveUser = !!user; + const showModifyActions = haveUser; + + const getFiles = useCallback(() => files, [files]); + + const isFavorite = useMemo(() => { + return showModifyActions && favoriteFileIDs + ? (annotatedFile: FileViewerAnnotatedFile): boolean => + favoriteFileIDs.has(annotatedFile.file.id) + : undefined; + }, [showModifyActions, favoriteFileIDs]); + + const toggleFavorite = useMemo(() => { + return favoriteFileIDs + ? ({ file, annotation }: FileViewerAnnotatedFile) => { + console.log({ file, annotation }); + // TODO + // const isFavorite = annotation.isFavorite; + // if (isFavorite === undefined) { + // assertionFailed(); + // return; + // } + + // onMarkUnsyncedFavoriteUpdate(file.id, !isFavorite); + // void (isFavorite ? removeFromFavorites : addToFavorites)( + // file, + // ).catch((e: unknown) => { + // log.error("Failed to remove favorite", e); + // onMarkUnsyncedFavoriteUpdate(file.id, undefined); + // }); + } + : undefined; + // }, [favoriteFileIDs, onMarkUnsyncedFavoriteUpdate]); + }, [favoriteFileIDs]); + + // Initial value of delegate. + if (!delegateRef.current) { + delegateRef.current = { getFiles, isFavorite, toggleFavorite }; } - // Updates to callbacks held by psDelegateRef. + // Updates to delegate callbacks. useEffect(() => { - const delegate = psDelegateRef.current!; - delegate.onClose = handleClose; - delegate.onAnnotate = handleAnnotate; - delegate.onToggleFavorite = handleToggleFavorite; - delegate.onViewInfo = handleViewInfo; - delegate.onEditImage = handleEditImage; - }, [ - handleClose, - handleAnnotate, - handleToggleFavorite, - handleViewInfo, - handleEditImage, - ]); + const delegate = delegateRef.current!; + delegate.getFiles = getFiles; + delegate.isFavorite = isFavorite; + delegate.toggleFavorite = toggleFavorite; + }, [getFiles, isFavorite, toggleFavorite]); useEffect(() => { - if (open && !psRef.current) { - // We're open, but we don't have a PhotoSwipe instance. Create - // one. This will show the file viewer dialog. - // - // Before creating it, also create a delegate. The delegate has - // a stable identity so that PhotoSwipe callbacks can be routed - // via it. When any of the - // callbacks change, we update the props of the delegate instead - // of changing the delegate itself. - log.debug(() => ["viewer", "open"]); + if (open) { + // We're open. Create psRef. This will show the file viewer dialog. + log.debug(() => ["viewer", { action: "open" }]); + const pswp = new FileViewerPhotoSwipe({ - files, initialIndex, - showModifyActions: !!user, disableDownload, + showModifyActions, + delegate: delegateRef.current!, onClose: handleClose, onAnnotate: handleAnnotate, onViewInfo: handleViewInfo, onEditImage: handleEditImage, - delegate: psDelegateRef.current!, }); + psRef.current = pswp; - } else if (!open && psRef.current) { - // We're closed, but we still have a PhotoSwipe instance. Cleanup. - log.debug(() => ["viewer", "close"]); - psRef.current.closeIfNeeded(); - psRef.current = undefined; + + return () => { + // Close dialog in the effect callback. + log.debug(() => ["viewer", { action: "close" }]); + pswp.closeIfNeeded(); + }; } - // TODO: How to relay files updates? - // eslint-disable-next-line react-hooks/exhaustive-deps }, [ open, onClose, user, initialIndex, disableDownload, + showModifyActions, handleClose, handleAnnotate, handleViewInfo, @@ -415,7 +374,7 @@ const FileViewer: React.FC = ({ ]); const handleRefreshPhotoswipe = useCallback(() => { - psRef.current.refreshCurrentSlideContent(); + psRef.current!.refreshCurrentSlideContent(); }, []); log.debug(() => ["viewer", { action: "render", psRef: psRef.current }]); @@ -429,8 +388,8 @@ const FileViewer: React.FC = ({ file={activeAnnotatedFile?.file} exif={activeFileExif} allowEdits={!!activeAnnotatedFile?.annotation.isOwnFile} - allowMap={!!user} - showCollections={!!user} + allowMap={haveUser} + showCollections={haveUser} scheduleUpdate={handleScheduleUpdate} refreshPhotoswipe={handleRefreshPhotoswipe} onSelectCollection={handleSelectCollection} diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index dc0fe62ad7..f8bf5e1a09 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -14,7 +14,6 @@ import { itemDataForFile, updateFileInfoExifIfNeeded, } from "./data-source"; -import type { FileViewerProps } from "./FileViewer"; import { createPSRegisterElementIconHTML } from "./icons"; // TODO(PS): WIP gallery using upstream photoswipe @@ -66,29 +65,39 @@ export interface FileViewerFileAnnotation { export interface FileViewerPhotoSwipeDelegate { /** * Called to obtain the latest list of files. + * + * [Note: Changes to underlying files when file viewer is open] + * + * The list of files shown by the viewer might change while the viewer is + * open. We do not actively refresh the viewer when this happens since that + * would result in the user's zoom / pan state being lost. + * + * However, we always read the latest list via the delegate, so any + * subsequent user initiated slide navigation (e.g. moving to the next + * slide) will use the new list. */ - onGetFiles?: unknown; // TODO + getFiles?: () => EnteFile[]; + /** + * Return `true` if the provided file has been marked as a favorite by the + * user. + * + * The toggle favorite button will not be shown for the file if this returns + * `undefined`. Otherwise the return value determines the toggle state of + * the toggle favorite button for the file. + */ + isFavorite?: ( + annotatedFile: FileViewerAnnotatedFile, + ) => boolean | undefined; /** * Called when the user activates the toggle favorite action on a file. - * - * If this callback is not provided, then the toggle favorite button is not - * shown. If this callback is provided, then the favorite button is shown if - * the {@link isFavorite} property of {@link FileViewerFileAnnotation} for - * the file is provided. In that case, the value of the {@link isFavorite} - * property will determine the current toggle state of the favorite button. */ - onToggleFavorite?: (annotatedFile: FileViewerAnnotatedFile) => void; - // TODO(PS) - /** - * `true` if this file has been marked as a favorite by the user. - * - * The toggle favorite button will not be shown if this is not defined. - * Otherwise it determines the toggle state of the toggle favorite button. - */ - isFavorite?: boolean | undefined; + toggleFavorite?: (annotatedFile: FileViewerAnnotatedFile) => void; } -type FileViewerPhotoSwipeOptions = { +type FileViewerPhotoSwipeOptions = Pick< + FileViewerProps, + "initialIndex" | "disableDownload" +> & { /** * `true` if various actions that modify the file should be shown. * @@ -101,6 +110,13 @@ type FileViewerPhotoSwipeOptions = { * {@link showFavorite} file annotation are true. */ showModifyActions: boolean; + /** + * Dynamic callbacks. + * + * The extra level of indirection allows these to be updated without + * recreating us. + */ + delegate: FileViewerPhotoSwipeDelegate; /** * Called when the file viewer is closed. */ @@ -124,14 +140,7 @@ type FileViewerPhotoSwipeOptions = { * for the file. */ onEditImage?: (annotatedFile: FileViewerAnnotatedFile) => void; - /** - * Dynamic callbacks. - * - * The extra level of indirection allows these to be updated without - * recreating us. - */ - delegate: FileViewerPhotoSwipeDelegate; -} & Pick; +}; /** * A file and its annotation, in a nice cosy box. @@ -172,10 +181,6 @@ export class FileViewerPhotoSwipe { * The options with which we were initialized. */ private opts: Pick; - /** - * An object via which we should route various dynamic callbacks. - */ - private delegate: FileViewerPhotoSwipeDelegate; /** * An interval that invokes a periodic check of whether we should the hide * controls if the user does not perform any pointer events for a while. @@ -207,19 +212,16 @@ export class FileViewerPhotoSwipe { private activeFileAnnotation: FileViewerFileAnnotation | undefined; constructor({ - files, initialIndex, disableDownload, showModifyActions, + delegate, onClose, onAnnotate, onViewInfo, onEditImage, - delegate, }: FileViewerPhotoSwipeOptions) { - this.files = files; this.opts = { disableDownload }; - this.delegate = delegate; this.lastActivityDate = new Date(); const pswp = new PhotoSwipe({ @@ -279,15 +281,38 @@ export class FileViewerPhotoSwipe { this.pswp = pswp; + // Various helper routines to obtain the file at `currIndex`. + + const currentFile = () => delegate.getFiles()[pswp.currIndex]!; + + const currentAnnotatedFile = () => { + const file = currentFile(); + let annotation = this.activeFileAnnotation; + if (annotation?.fileID != file.id) { + annotation = onAnnotate(file); + this.activeFileAnnotation = annotation; + } + return { + file, + // The above condition implies that annotation can never be + // undefined, but it doesn't seem to be enough to convince + // TypeScript. Writing the condition in a more unnatural way + // `(!(annotation && annotation?.fileID == file.id))` works, but + // instead we use a non-null assertion here. + annotation: annotation!, + }; + }; + + const currentFileAnnotation = () => currentAnnotatedFile().annotation; + // Provide data about slides to PhotoSwipe via callbacks // https://photoswipe.com/data-sources/#dynamically-generated-data - pswp.addFilter("numItems", () => { - return this.files.length; - }); + pswp.addFilter("numItems", () => delegate.getFiles().length); pswp.addFilter("itemData", (_, index) => { - const file = this.files[index]!; + const files = delegate.getFiles(); + const file = files[index]!; let itemData = itemDataForFile(file, () => pswp.refreshSlideContent(index), @@ -378,8 +403,8 @@ export class FileViewerPhotoSwipe { // // See: [Note: File viewer error handling] // TODO - console.log(this.currentFile(), e); - forgetFailedItemDataForFile(this.currentFile()); + console.log(currentFile(), e); + forgetFailedItemDataForFile(currentFile()); // Pause the video element, if any, when we move away from the // slide. @@ -510,7 +535,7 @@ export class FileViewerPhotoSwipe { order: 9, isButton: true, html: createPSRegisterElementIconHTML("info"), - onClick: () => onViewInfo(this.currentAnnotatedFile()), + onClick: () => onViewInfo(currentAnnotatedFile()), }); // TODO(PS): @@ -589,38 +614,6 @@ export class FileViewerPhotoSwipe { this.pswp.refreshSlideContent(this.pswp.currIndex); } - updateFiles(files: EnteFile[]) { - // TODO(PS) - } - - // Various helper routines to obtain the file at `currIndex`. - - private currentFile() { - return this.files[this.pswp.currIndex]!; - } - - private currentAnnotatedFile() { - const file = this.currentFile(); - let annotation = this.activeFileAnnotation; - if (annotation?.fileID != file.id) { - annotation = this.delegate.onAnnotate(file); - this.activeFileAnnotation = annotation; - } - return { - file, - // The above condition implies that annotation can never be - // undefined, but it doesn't seem to be enough to convince - // TypeScript. Writing the condition in a more unnatural way - // `(!(annotation && annotation?.fileID == file.id))` works, but - // instead we use a non-null assertion here. - annotation: annotation!, - }; - } - - private currentFileAnnotation() { - return this.currentAnnotatedFile().annotation; - } - private clearAutoHideIntervalIfNeeded() { if (this.autoHideCheckIntervalId) { clearInterval(this.autoHideCheckIntervalId);