From 43c0d8a6ad48e12b7c1c08efb5874323ea72e8b9 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Fri, 14 Feb 2025 10:39:49 +0530 Subject: [PATCH 01/19] Revert "Revert "Workbench"" This reverts commit 20fea517cecefc236f88bc4e7e77bccca88c9ce6. --- web/apps/photos/src/pages/_app.tsx | 4 ++-- web/apps/photos/src/styles/global.css | 3 ++- web/packages/new/photos/components/FileViewerPhotoSwipe.tsx | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/web/apps/photos/src/pages/_app.tsx b/web/apps/photos/src/pages/_app.tsx index e5b919e622..c514df101f 100644 --- a/web/apps/photos/src/pages/_app.tsx +++ b/web/apps/photos/src/pages/_app.tsx @@ -51,9 +51,9 @@ import { useCallback, useEffect, useMemo, useState } from "react"; import { resumeExportsIfNeeded } from "services/export"; import { photosLogout } from "services/logout"; -import "photoswipe/dist/photoswipe.css"; +// import "photoswipe/dist/photoswipe.css"; // TODO(PS): Note, auto hide only works with the new CSS. -// import "../../../../packages/new/photos/components/ps5/dist/photoswipe.css"; +import "../../../../packages/new/photos/components/ps5/dist/photoswipe.css"; import "styles/global.css"; diff --git a/web/apps/photos/src/styles/global.css b/web/apps/photos/src/styles/global.css index 38d9241ba2..e15e672a24 100644 --- a/web/apps/photos/src/styles/global.css +++ b/web/apps/photos/src/styles/global.css @@ -12,7 +12,7 @@ body { flex-direction: column; height: 100%; } - +@media DISABLED { .pswp__button--custom { width: 48px; height: 48px; @@ -114,6 +114,7 @@ body { .pswp__caption--empty { display: none; } +} /* Make the controllable video elements we render as custom PhotoSwipe content diff --git a/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx b/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx index db1771f379..22b37ca2fd 100644 --- a/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx +++ b/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx @@ -28,7 +28,7 @@ if (process.env.NEXT_PUBLIC_ENTE_WIP_PS5) { let PhotoSwipe; if (process.env.NEXT_PUBLIC_ENTE_WIP_PS5) { // TODO(PS): Comment me before merging into main. - // PhotoSwipe = require("./ps5/dist/photoswipe.esm.js").default; + PhotoSwipe = require("./ps5/dist/photoswipe.esm.js").default; } // TODO(PS): //import { type SlideData } from "./ps5/dist/types/slide/" From 8e90541d871b65f4a6bc0ddfb5755433273bcbee Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 13:57:07 +0530 Subject: [PATCH 02/19] Fix path for lps --- web/packages/new/photos/components/FileViewerPhotoSwipe.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx b/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx index 22b37ca2fd..5f50599274 100644 --- a/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx +++ b/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx @@ -301,8 +301,10 @@ export class FileViewerPhotoSwipe { }); pswp.on("contentDeactivate", (e) => { - // Pause the video tag (if any) for a slide when we move away from it. - const video = e.content?.element?.getElementsByTagName("video")[0]; + // Pause the video element (if any) on a slide when we move away + // from it. + const video = + e.content?.slide?.container?.getElementsByTagName("video")[0]; video?.pause(); }); From dc0450b155385d41640181c8dff3aad1a0e5f3c2 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 14:05:55 +0530 Subject: [PATCH 03/19] Modal --- .../new/photos/components/FileViewer5.tsx | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/web/packages/new/photos/components/FileViewer5.tsx b/web/packages/new/photos/components/FileViewer5.tsx index 0b678f881d..a4d6e3677c 100644 --- a/web/packages/new/photos/components/FileViewer5.tsx +++ b/web/packages/new/photos/components/FileViewer5.tsx @@ -13,6 +13,7 @@ if (process.env.NEXT_PUBLIC_ENTE_WIP_PS5) { throw new Error("Whoa"); } +import { useModalVisibility } from "@/base/components/utils/modal"; import type { EnteFile } from "@/media/file.js"; import { Button, styled } from "@mui/material"; import { useCallback, useEffect, useRef } from "react"; @@ -57,8 +58,23 @@ const FileViewer: React.FC = ({ }) => { const pswpRef = useRef(); + // 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. + // + // 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 + // refer to the file on which the last user initiated action was performed. + const [activeFile, setActiveFile] = setState( + undefined, + ); + + const { show: showFileInfo, props: fileInfoVisibilityProps } = + useModalVisibility(); + const handleViewInfo = useCallback((file: EnteFile) => { - console.log("view-info", file); + setActiveFile(file); + showFileInfo(); }, []); useEffect(() => { From 1c322a9c62e5e544f17df4f9c6f7cb2c9dcf1ec7 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 14:11:16 +0530 Subject: [PATCH 04/19] Modal --- .../photos/src/components/PhotoViewer/index.tsx | 4 ++-- web/packages/gallery/components/FileInfo.tsx | 16 +++++++--------- .../new/photos/components/FileViewer5.tsx | 6 ++++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/web/apps/photos/src/components/PhotoViewer/index.tsx b/web/apps/photos/src/components/PhotoViewer/index.tsx index 48b484bb7e..a9fe6a4d8c 100644 --- a/web/apps/photos/src/components/PhotoViewer/index.tsx +++ b/web/apps/photos/src/components/PhotoViewer/index.tsx @@ -992,8 +992,8 @@ export const PhotoViewer: React.FC = ({ onConfirm={handleDeleteFile} /> void; exif: FileInfoExif | undefined; /** * TODO: Rename and flip to allowEdits. @@ -141,11 +139,11 @@ export interface FileInfoProps { } export const FileInfo: React.FC = ({ + open, + onClose, file, shouldDisableEdits, allowMap, - showInfo, - handleCloseInfo, exif, scheduleUpdate, refreshPhotoswipe, @@ -209,8 +207,8 @@ export const FileInfo: React.FC = ({ onSelectPerson?.(personID); return ( - - + + = ({ diff --git a/web/packages/new/photos/components/FileViewer5.tsx b/web/packages/new/photos/components/FileViewer5.tsx index a4d6e3677c..2e019202c5 100644 --- a/web/packages/new/photos/components/FileViewer5.tsx +++ b/web/packages/new/photos/components/FileViewer5.tsx @@ -14,9 +14,10 @@ if (process.env.NEXT_PUBLIC_ENTE_WIP_PS5) { } import { useModalVisibility } from "@/base/components/utils/modal"; +import { FileInfo } from "@/gallery/components/FileInfo"; import type { EnteFile } from "@/media/file.js"; import { Button, styled } from "@mui/material"; -import { useCallback, useEffect, useRef } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { FileViewerPhotoSwipe } from "./FileViewerPhotoSwipe"; export interface FileViewerProps { @@ -65,7 +66,7 @@ const FileViewer: React.FC = ({ // 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 // refer to the file on which the last user initiated action was performed. - const [activeFile, setActiveFile] = setState( + const [activeFile, setActiveFile] = useState( undefined, ); @@ -101,6 +102,7 @@ const FileViewer: React.FC = ({ return ( + ); }; From e9d63dfea98269b7f0eaeab0ce6a807ee5c1c2a7 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 14:39:49 +0530 Subject: [PATCH 05/19] zi --- web/apps/photos/src/styles/global.css | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/apps/photos/src/styles/global.css b/web/apps/photos/src/styles/global.css index e15e672a24..6de4c6f5d0 100644 --- a/web/apps/photos/src/styles/global.css +++ b/web/apps/photos/src/styles/global.css @@ -116,6 +116,12 @@ body { } } +.pswp-ente { + /* The default z-index for PhotoSwipe is 10k, way beyond everything else. + Give it a more moderate value so that MUI elements can be used with it. */ + z-index: calc(var(--mui-zIndex-drawer) - 1); +} + /* Make the controllable video elements we render as custom PhotoSwipe content take up the entire container. From fe86075868051e7aaa486518175ac110d5a4a0ac Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 14:54:11 +0530 Subject: [PATCH 06/19] Namespace --- web/apps/photos/src/components/PhotoFrame.tsx | 2 +- web/apps/photos/src/components/PhotoViewer/index.tsx | 2 +- .../components/{FileViewer.tsx => FileViewerComponents.tsx} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename web/packages/new/photos/components/{FileViewer.tsx => FileViewerComponents.tsx} (100%) diff --git a/web/apps/photos/src/components/PhotoFrame.tsx b/web/apps/photos/src/components/PhotoFrame.tsx index 25576c0be6..7cd68255ab 100644 --- a/web/apps/photos/src/components/PhotoFrame.tsx +++ b/web/apps/photos/src/components/PhotoFrame.tsx @@ -7,7 +7,7 @@ import { } from "@/gallery/services/download"; import { EnteFile } from "@/media/file"; import { FileType } from "@/media/file-type"; -import { FileViewer } from "@/new/photos/components/FileViewer"; +import { FileViewer } from "@/new/photos/components/FileViewerComponents"; import type { GalleryBarMode } from "@/new/photos/components/gallery/reducer"; import { TRASH_SECTION } from "@/new/photos/services/collection"; import { styled } from "@mui/material"; diff --git a/web/apps/photos/src/components/PhotoViewer/index.tsx b/web/apps/photos/src/components/PhotoViewer/index.tsx index a9fe6a4d8c..41b83cc307 100644 --- a/web/apps/photos/src/components/PhotoViewer/index.tsx +++ b/web/apps/photos/src/components/PhotoViewer/index.tsx @@ -19,7 +19,7 @@ import type { Collection } from "@/media/collection"; import { fileLogID, type EnteFile } from "@/media/file"; import { FileType } from "@/media/file-type"; import { isHEICExtension, needsJPEGConversion } from "@/media/formats"; -import { ConfirmDeleteFileDialog } from "@/new/photos/components/FileViewer"; +import { ConfirmDeleteFileDialog } from "@/new/photos/components/FileViewerComponents"; import { ImageEditorOverlay } from "@/new/photos/components/ImageEditorOverlay"; import { moveToTrash } from "@/new/photos/services/collection"; import { extractRawExif, parseExif } from "@/new/photos/services/exif"; diff --git a/web/packages/new/photos/components/FileViewer.tsx b/web/packages/new/photos/components/FileViewerComponents.tsx similarity index 100% rename from web/packages/new/photos/components/FileViewer.tsx rename to web/packages/new/photos/components/FileViewerComponents.tsx From 0eba6b9c98eb22b62cd593cf835e58b20386b622 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 14:58:07 +0530 Subject: [PATCH 07/19] Move --- web/apps/photos/src/pages/_app.tsx | 2 +- .../components => gallery/components/viewer}/.gitignore | 0 .../components/viewer/FileViewer.tsx} | 2 +- .../components/viewer/photoswipe.ts} | 2 +- web/packages/new/photos/components/FileViewerComponents.tsx | 4 +++- 5 files changed, 6 insertions(+), 4 deletions(-) rename web/packages/{new/photos/components => gallery/components/viewer}/.gitignore (100%) rename web/packages/{new/photos/components/FileViewer5.tsx => gallery/components/viewer/FileViewer.tsx} (98%) rename web/packages/{new/photos/components/FileViewerPhotoSwipe.tsx => gallery/components/viewer/photoswipe.ts} (99%) diff --git a/web/apps/photos/src/pages/_app.tsx b/web/apps/photos/src/pages/_app.tsx index c514df101f..c2ed6d7c81 100644 --- a/web/apps/photos/src/pages/_app.tsx +++ b/web/apps/photos/src/pages/_app.tsx @@ -53,7 +53,7 @@ import { photosLogout } from "services/logout"; // import "photoswipe/dist/photoswipe.css"; // TODO(PS): Note, auto hide only works with the new CSS. -import "../../../../packages/new/photos/components/ps5/dist/photoswipe.css"; +import "../../../../packages/gallery/components/viewer/ps5/dist/photoswipe.css"; import "styles/global.css"; diff --git a/web/packages/new/photos/components/.gitignore b/web/packages/gallery/components/viewer/.gitignore similarity index 100% rename from web/packages/new/photos/components/.gitignore rename to web/packages/gallery/components/viewer/.gitignore diff --git a/web/packages/new/photos/components/FileViewer5.tsx b/web/packages/gallery/components/viewer/FileViewer.tsx similarity index 98% rename from web/packages/new/photos/components/FileViewer5.tsx rename to web/packages/gallery/components/viewer/FileViewer.tsx index 2e019202c5..9deedd9180 100644 --- a/web/packages/new/photos/components/FileViewer5.tsx +++ b/web/packages/gallery/components/viewer/FileViewer.tsx @@ -18,7 +18,7 @@ import { FileInfo } from "@/gallery/components/FileInfo"; import type { EnteFile } from "@/media/file.js"; import { Button, styled } from "@mui/material"; import { useCallback, useEffect, useRef, useState } from "react"; -import { FileViewerPhotoSwipe } from "./FileViewerPhotoSwipe"; +import { FileViewerPhotoSwipe } from "./photoswipe"; export interface FileViewerProps { /** diff --git a/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx b/web/packages/gallery/components/viewer/photoswipe.ts similarity index 99% rename from web/packages/new/photos/components/FileViewerPhotoSwipe.tsx rename to web/packages/gallery/components/viewer/photoswipe.ts index 5f50599274..f112e712de 100644 --- a/web/packages/new/photos/components/FileViewerPhotoSwipe.tsx +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -9,7 +9,7 @@ import { } from "@/gallery/services/download"; import type { EnteFile } from "@/media/file"; import { FileType } from "@/media/file-type"; -import type { FileViewerProps } from "./FileViewer5"; +import type { FileViewerProps } from "./FileViewer"; // import { renderToString } from "react-dom/server"; diff --git a/web/packages/new/photos/components/FileViewerComponents.tsx b/web/packages/new/photos/components/FileViewerComponents.tsx index 3835fae56a..120b3a4c2c 100644 --- a/web/packages/new/photos/components/FileViewerComponents.tsx +++ b/web/packages/new/photos/components/FileViewerComponents.tsx @@ -13,7 +13,9 @@ import { aboveFileViewerContentZ } from "./utils/z-index"; import dynamic from "next/dynamic"; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore -const FV5 = dynamic(() => import("./FileViewer5"), { ssr: false }); +const FV5 = dynamic(() => import("@/gallery/components/viewer/FileViewer"), { + ssr: false, +}); const FVD = () => <>; export const FileViewer: React.FC = (props) => { From 6140f35e69ca04355847e00de2574d3c2faa1ad6 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 15:27:39 +0530 Subject: [PATCH 08/19] Move --- .../gallery/components/viewer/icons.tsx | 19 +++++++++++++++++++ .../gallery/components/viewer/photoswipe.ts | 6 ------ 2 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 web/packages/gallery/components/viewer/icons.tsx diff --git a/web/packages/gallery/components/viewer/icons.tsx b/web/packages/gallery/components/viewer/icons.tsx new file mode 100644 index 0000000000..1238f7f619 --- /dev/null +++ b/web/packages/gallery/components/viewer/icons.tsx @@ -0,0 +1,19 @@ +/** + * @file [Note: SVG paths of MUI icons] + * + * When creating buttons for use with PhotoSwipe, we need to provide just the + * contents of the SVG element (e.g. paths) as an HTML string. + * + * Since we only need a handful, these strings were created by temporarily + * adding the following code in some existing React component to render the + * corresponding MUI icon React component to a string, and retain the path. + * + * 1. import { renderToString } from "react-dom/server"; + * + * 2. console.log(renderToString()); + */ + +// const html = +// console.log(renderToString()); +// const path = +// ''; diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index f112e712de..9f2b73d84b 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -11,8 +11,6 @@ import type { EnteFile } from "@/media/file"; import { FileType } from "@/media/file-type"; import type { FileViewerProps } from "./FileViewer"; -// import { renderToString } from "react-dom/server"; - // TODO(PS): WIP gallery using upstream photoswipe // // Needs (not committed yet): @@ -326,10 +324,6 @@ export class FileViewerPhotoSwipe { // - zoom: 10 // - close: 20 pswp.on("uiRegister", () => { - // const html = ; - // console.log(renderToString(html)); - // const path = - // ''; const pathWithIDAndTransform = ''; pswp.ui.registerElement({ From 84aeb7941221d31f79276bfd316761e2951a360c Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 15:44:05 +0530 Subject: [PATCH 09/19] Move out icons --- .../gallery/components/viewer/icons.tsx | 37 ++++++++++++++++--- .../gallery/components/viewer/photoswipe.ts | 9 +---- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/web/packages/gallery/components/viewer/icons.tsx b/web/packages/gallery/components/viewer/icons.tsx index 1238f7f619..ef4fadd8e1 100644 --- a/web/packages/gallery/components/viewer/icons.tsx +++ b/web/packages/gallery/components/viewer/icons.tsx @@ -8,12 +8,37 @@ * adding the following code in some existing React component to render the * corresponding MUI icon React component to a string, and retain the path. * - * 1. import { renderToString } from "react-dom/server"; * - * 2. console.log(renderToString()); + * import { renderToString } from "react-dom/server"; + * import InfoOutlinedIcon from ; + * + * console.log(renderToString()); */ -// const html = -// console.log(renderToString()); -// const path = -// ''; +const paths = { + // "@mui/icons-material/InfoOutlined" + info: '', + * outlineID: "pswp__icn-info", + * } + * + */ +export const createPSRegisterElementIconHTML = (name: "info") => ({ + isCustomSVG: true, + // TODO(PS): This transform is temporary, audit later. + inner: `${paths[name]} transform="translate(3.5, 3.5)" id="pswp__icn-${name}"`, + outlineID: `pswp__icn-${name}`, +}); diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 9f2b73d84b..aea060e0ce 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -10,6 +10,7 @@ import { import type { EnteFile } from "@/media/file"; import { FileType } from "@/media/file-type"; import type { FileViewerProps } from "./FileViewer"; +import { createPSRegisterElementIconHTML } from "./icons"; // TODO(PS): WIP gallery using upstream photoswipe // @@ -324,19 +325,13 @@ export class FileViewerPhotoSwipe { // - zoom: 10 // - close: 20 pswp.on("uiRegister", () => { - const pathWithIDAndTransform = - ''; pswp.ui.registerElement({ name: "info", title: "Info", ariaLabel: "Info", order: 15, isButton: true, - html: { - isCustomSVG: true, - inner: pathWithIDAndTransform, - outlineID: "pswp__icn-info", - }, + html: createPSRegisterElementIconHTML("info"), onClick: (e, element, pswp) => { const file = this.files[pswp.currIndex]; if (!file) { From e93702766704a0865dbeff597ea8dd742b58118f Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 16:04:51 +0530 Subject: [PATCH 10/19] Title gets used as ariaLabel --- web/packages/gallery/components/viewer/photoswipe.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index aea060e0ce..30ffeafee7 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -11,6 +11,7 @@ import type { EnteFile } from "@/media/file"; import { FileType } from "@/media/file-type"; import type { FileViewerProps } from "./FileViewer"; import { createPSRegisterElementIconHTML } from "./icons"; +import { t } from "i18next"; // TODO(PS): WIP gallery using upstream photoswipe // @@ -327,8 +328,7 @@ export class FileViewerPhotoSwipe { pswp.on("uiRegister", () => { pswp.ui.registerElement({ name: "info", - title: "Info", - ariaLabel: "Info", + title: t("info"), order: 15, isButton: true, html: createPSRegisterElementIconHTML("info"), From f6d949db38fab66173d23e51e4359648776229f4 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 16:09:18 +0530 Subject: [PATCH 11/19] wrap --- .../gallery/components/viewer/photoswipe.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 30ffeafee7..dba046614b 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -9,9 +9,9 @@ import { } from "@/gallery/services/download"; import type { EnteFile } from "@/media/file"; import { FileType } from "@/media/file-type"; +import { t } from "i18next"; import type { FileViewerProps } from "./FileViewer"; import { createPSRegisterElementIconHTML } from "./icons"; -import { t } from "i18next"; // TODO(PS): WIP gallery using upstream photoswipe // @@ -316,6 +316,9 @@ export class FileViewerPhotoSwipe { onClose(); }); + const withCurrentFile = (cb: (file: EnteFile) => void) => () => + cb(this.files[this.pswp.currIndex]!); + // Add our custom UI elements to inside the PhotoSwipe dialog. // // API docs for registerElement: @@ -332,15 +335,7 @@ export class FileViewerPhotoSwipe { order: 15, isButton: true, html: createPSRegisterElementIconHTML("info"), - onClick: (e, element, pswp) => { - const file = this.files[pswp.currIndex]; - if (!file) { - assertionFailed(); - return; - } - - onViewInfo(file); - }, + onClick: withCurrentFile(onViewInfo), }); }); From bb2bbb56556fa261952666983857f94df9db55b1 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 16:32:13 +0530 Subject: [PATCH 12/19] Workaround for an aria issue... ...somewhere (In something we're doing? In PS? In Chrome? In ARIA?) --- web/packages/gallery/components/viewer/photoswipe.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index dba046614b..31a198ec38 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -195,6 +195,10 @@ export class FileViewerPhotoSwipe { // Taking a step back though, the PhotoSwipe viewport is fixed, so // we can just directly map wheel / trackpad scrolls to zooming. wheelToZoom: true, + // Chrome yells about incorrectly mixing focus and aria-hidden if we + // leave this at the default (true) and then swipe between slides + // fast, or show MUI drawers etc. + trapFocus: false, // Set the index within files that we should open to. Subsequent // updates to the index will be tracked by PhotoSwipe internally. index: initialIndex, From a851caf78fb9c2f86478544c073f585f4852da22 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 17:11:14 +0530 Subject: [PATCH 13/19] Another ARIA workaround --- web/packages/base/components/utils/theme.ts | 2 ++ web/packages/gallery/components/FileInfo.tsx | 18 +++++++++++++++--- .../gallery/components/viewer/photoswipe.ts | 3 +++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/web/packages/base/components/utils/theme.ts b/web/packages/base/components/utils/theme.ts index 1e511b4ac7..1106644235 100644 --- a/web/packages/base/components/utils/theme.ts +++ b/web/packages/base/components/utils/theme.ts @@ -541,6 +541,8 @@ const components: Components = { MuiDialog: { defaultProps: { + // [Note: Overjealous Chrome? Complicated ARIA?] + // // This is required to prevent console errors about aria-hiding a // focused button when the dialog is closed. // diff --git a/web/packages/gallery/components/FileInfo.tsx b/web/packages/gallery/components/FileInfo.tsx index b22fb97a99..5357653e87 100644 --- a/web/packages/gallery/components/FileInfo.tsx +++ b/web/packages/gallery/components/FileInfo.tsx @@ -18,7 +18,10 @@ import { ActivityIndicator } from "@/base/components/mui/ActivityIndicator"; import { SidebarDrawer } from "@/base/components/mui/SidebarDrawer"; import { Titlebar } from "@/base/components/Titlebar"; import { EllipsizedTypography } from "@/base/components/Typography"; -import { useModalVisibility, type ModalVisibilityProps } from "@/base/components/utils/modal"; +import { + useModalVisibility, + type ModalVisibilityProps, +} from "@/base/components/utils/modal"; import { useBaseContext } from "@/base/context"; import { haveWindow } from "@/base/env"; import { nameAndExtension } from "@/base/file-name"; @@ -136,7 +139,7 @@ export type FileInfoProps = ModalVisibilityProps & { * Called when the user selects a person in the file info panel. */ onSelectPerson?: ((personID: string) => void) | undefined; -} +}; export const FileInfo: React.FC = ({ open, @@ -398,7 +401,16 @@ const parseExifInfo = ( const FileInfoSidebar = styled( (props: Pick) => ( - + ), )(({ theme }) => ({ zIndex: fileInfoDrawerZ, diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 31a198ec38..2951386288 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -198,6 +198,9 @@ export class FileViewerPhotoSwipe { // Chrome yells about incorrectly mixing focus and aria-hidden if we // leave this at the default (true) and then swipe between slides // fast, or show MUI drawers etc. + // + // See: [Note: Overjealous Chrome? Complicated ARIA?], but time with + // a different library. trapFocus: false, // Set the index within files that we should open to. Subsequent // updates to the index will be tracked by PhotoSwipe internally. From 12a96b68ba2bf8305383cc164b7b3c46c1a52d61 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 18:39:18 +0530 Subject: [PATCH 14/19] wip ds --- .../gallery/components/viewer/data-source.ts | 170 ++++++++++++++++++ .../gallery/components/viewer/photoswipe.ts | 63 +------ 2 files changed, 176 insertions(+), 57 deletions(-) create mode 100644 web/packages/gallery/components/viewer/data-source.ts diff --git a/web/packages/gallery/components/viewer/data-source.ts b/web/packages/gallery/components/viewer/data-source.ts new file mode 100644 index 0000000000..4a1d35c591 --- /dev/null +++ b/web/packages/gallery/components/viewer/data-source.ts @@ -0,0 +1,170 @@ +/* xeslint-disable */ +// x-@ts-nocheck + +import { + downloadManager, + type LivePhotoSourceURL, +} from "@/gallery/services/download"; +import type { EnteFile } from "@/media/file"; +import { FileType } from "@/media/file-type"; + +// TODO(PS): +//import { type SlideData } from "./ps5/dist/types/slide/" +export interface SlideData { + /** + * image URL + */ + src?: string | undefined; + /** + * image width + */ + width?: number | undefined; + /** + * image height + */ + height?: number | undefined; + /** + * html content of a slide + */ + html?: string | undefined; + + // Our props. TODO(PS) document if end up using these. + + isContentLoading?: boolean; + isContentZoomable?: boolean; + isFinal?: boolean; +} + +/** + * A class that stores and serves data required by our custom PhotoSwipe + * instance, effectively acting as an in-memory cache. + * + * By keeping this independent of the lifetime of the PhotoSwipe instance, we + * can reuse the same cache for multiple displays of our file viewer. + */ +export class FileViewerDataSource { + /** + * The best available SlideData for rendering the file with the given ID. + * + * If an entry does not exist for a particular fileID, then it is lazily + * added on demand, and updated as we keep getting better data (thumbnail, + * original) for the file. + */ + private itemDataByFileID = new Map(); + + /** + * + * The {@link onUpdate} callback is invoked each time we have data about the + * given {@link file}. + * + * If we already have the final data about file, then {@link onUpdate} will + * be called once with this final {@link itemData}. Otherwise it'll be + * called multiple times. + * + * 1. First with empty itemData. + * + * 2. Then with the thumbnail data. + * + * 3. Then with the original. For live photos, this will happen twice, first + * with the original image, then again with the video component. + * + * 4. At this point, the data for this file will be considered final, and + * subsequent calls for the same file will return this same value unless + * it is invalidated. + * + * The same entry might get updated multiple times, as we start with the + * thumbnail but then also update this as we keep getting more of the + * original (e.g. for a live photo, it'll be updated once when we get the + * original image, and then again later once we get the original video). + * + * @param index + * @param file + * @param onUpdate Callback invoked each time we have data about the given + * {@link file}. + */ + private async enqueueUpdates( + file: EnteFile, + onUpdate: (itemData: SlideData) => void, + ) { + const update = (itemData: SlideData) => { + this.itemDataByFileID.set(file.id, itemData); + onUpdate(itemData); + }; + + // We might not have anything to show immediately, though in most cases + // a cached renderable thumbnail URL will be available shortly. + // + // Meanwhile, + // + // 1. Return empty slide data; PhotoSwipe will not show anything in the + // image area but will otherwise render UI controls properly. + // + // 2. Insert empty data so that we don't enqueue multiple updates. + + const itemData = this.itemDataByFileID.get(file.id); + if (itemData) { + itemData = {}; + this.itemDataByFileID.set(file.id, itemData); + this.enqueueUpdates(index, file); + } + } + + const thumbnailURL = await downloadManager.renderableThumbnailURL(file); + const thumbnailData = await augmentedWithDimensions(thumbnailURL); + update({ + ...thumbnailData, + isContentLoading: true, + isContentZoomable: false, + }); + + switch (file.metadata.fileType) { + case FileType.image: { + const sourceURLs = + await downloadManager.renderableSourceURLs(file); + update(await augmentedWithDimensions(sourceURLs.url)); + break; + } + + case FileType.video: { + const sourceURLs = + await downloadManager.renderableSourceURLs(file); + const disableDownload = !!this.opts.disableDownload; + update({ html: videoHTML(sourceURLs.url, disableDownload) }); + break; + } + + default: { + const sourceURLs = + await downloadManager.renderableSourceURLs(file); + const livePhotoSourceURLs = + sourceURLs.url as LivePhotoSourceURL; + const imageURL = await livePhotoSourceURLs.image(); + const imageData = await augmentedWithDimensions(imageURL); + update(imageData); + const livePhotoVideoURL = await livePhotoSourceURLs.video(); + update({ ...imageData, livePhotoVideoURL }); + break; + } + } + } +} + +/** + * Take a image URL, determine its dimensions using browser APIs, and return the URL + * and its dimensions in a form that can directly be passed to PhotoSwipe as + * {@link SlideData}. + */ +const augmentedWithDimensions = (imageURL: string): Promise => + new Promise((resolve) => { + const image = new Image(); + image.onload = () => { + resolve({ + src: imageURL, + width: image.naturalWidth, + height: image.naturalHeight, + }); + }; + // image.onerror = () + // TODO(PS): Handle imageElement.onerror + image.src = imageURL; + }); diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 2951386288..d05e7a6304 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -10,6 +10,7 @@ import { import type { EnteFile } from "@/media/file"; import { FileType } from "@/media/file-type"; import { t } from "i18next"; +import { FileViewerDataSource, type SlideData } from "./data-source"; import type { FileViewerProps } from "./FileViewer"; import { createPSRegisterElementIconHTML } from "./icons"; @@ -30,58 +31,6 @@ if (process.env.NEXT_PUBLIC_ENTE_WIP_PS5) { // TODO(PS): Comment me before merging into main. PhotoSwipe = require("./ps5/dist/photoswipe.esm.js").default; } -// TODO(PS): -//import { type SlideData } from "./ps5/dist/types/slide/" -type SlideData = { - /** - * thumbnail element - */ - element?: HTMLElement | undefined; - /** - * image URL - */ - src?: string | undefined; - /** - * image srcset - */ - srcset?: string | undefined; - /** - * image width (deprecated) - */ - w?: number | undefined; - /** - * image height (deprecated) - */ - h?: number | undefined; - /** - * image width - */ - width?: number | undefined; - /** - * image height - */ - height?: number | undefined; - /** - * placeholder image URL that's displayed before large image is loaded - */ - msrc?: string | undefined; - /** - * image alt text - */ - alt?: string | undefined; - /** - * whether thumbnail is cropped client-side or not - */ - thumbCropped?: boolean | undefined; - /** - * html content of a slide - */ - html?: string | undefined; - /** - * slide type - */ - type?: string | undefined; -}; type FileViewerPhotoSwipeOptions = FileViewerProps & { /** @@ -126,13 +75,12 @@ export class FileViewerPhotoSwipe { */ private opts: Pick; /** - * The best available SlideData for rendering the file with the given ID. + * Our data source. * - * If an entry does not exist for a particular fileID, then it is lazily - * added on demand. The same entry might get updated multiple times, as we - * start with the thumbnail but then also update this with the original etc. + * TODO(PS): Move this elsewhere, or merge with download manager. */ - private itemDataByFileID: Map = new Map(); + private dataSource: FileViewerDataSource; + /** * 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. @@ -161,6 +109,7 @@ export class FileViewerPhotoSwipe { }: FileViewerPhotoSwipeOptions) { this.files = files; this.opts = { disableDownload }; + this.dataSource = new FileViewerDataSource(); const pswp = new PhotoSwipe({ // Opaque background. From 9d76d93254a2bcdc3696245094c8df6b758968f8 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Mon, 17 Feb 2025 19:10:32 +0530 Subject: [PATCH 15/19] Integrate --- .../gallery/components/viewer/data-source.ts | 138 ++++++++++-------- .../gallery/components/viewer/photoswipe.ts | 107 ++------------ 2 files changed, 91 insertions(+), 154 deletions(-) diff --git a/web/packages/gallery/components/viewer/data-source.ts b/web/packages/gallery/components/viewer/data-source.ts index 4a1d35c591..58876cd6d2 100644 --- a/web/packages/gallery/components/viewer/data-source.ts +++ b/web/packages/gallery/components/viewer/data-source.ts @@ -10,7 +10,7 @@ import { FileType } from "@/media/file-type"; // TODO(PS): //import { type SlideData } from "./ps5/dist/types/slide/" -export interface SlideData { +interface SlideData { /** * image URL */ @@ -27,13 +27,16 @@ export interface SlideData { * html content of a slide */ html?: string | undefined; +} +type ItemData = SlideData & { // Our props. TODO(PS) document if end up using these. - + videoURL?: string; + livePhotoVideoURL?: string; isContentLoading?: boolean; isContentZoomable?: boolean; isFinal?: boolean; -} +}; /** * A class that stores and serves data required by our custom PhotoSwipe @@ -43,74 +46,83 @@ export interface SlideData { * can reuse the same cache for multiple displays of our file viewer. */ export class FileViewerDataSource { + private itemDataByFileID = new Map(); + private needsRefreshByFileID = new Map void>(); + /** - * The best available SlideData for rendering the file with the given ID. + * Return the best available ItemData for rendering the given {@link file}. * - * If an entry does not exist for a particular fileID, then it is lazily - * added on demand, and updated as we keep getting better data (thumbnail, + * If an entry does not exist for a particular file, then it is lazily added + * on demand, and updated as we keep getting better data (thumbnail, * original) for the file. - */ - private itemDataByFileID = new Map(); - - /** * - * The {@link onUpdate} callback is invoked each time we have data about the - * given {@link file}. + * At each step, we call the provided callback so that file viewer can call + * us again to get the updated data. + * + * --- + * + * Detailed flow: + * + * If we already have the final data about the file, then this function will + * return it and do nothing subsequently. + * + * Otherwise, it will: * - * If we already have the final data about file, then {@link onUpdate} will - * be called once with this final {@link itemData}. Otherwise it'll be - * called multiple times. + * 1. Return empty slide data; PhotoSwipe will not show anything in the + * image area but will otherwise render UI controls properly (in most + * cases a cached renderable thumbnail URL will be available shortly) * - * 1. First with empty itemData. + * 2. Insert empty data so that we don't enqueue multiple updates, and + * return this empty data. * - * 2. Then with the thumbnail data. + * Then it we start fetching data for the file. * - * 3. Then with the original. For live photos, this will happen twice, first - * with the original image, then again with the video component. + * First it'll fetch the thumbnail. Once that is done, it'll update the data + * it has cached, and notify the caller (using the provided callback) so it + * can refresh the slide. * - * 4. At this point, the data for this file will be considered final, and - * subsequent calls for the same file will return this same value unless - * it is invalidated. + * Then it'll continue fetching the original. * - * The same entry might get updated multiple times, as we start with the - * thumbnail but then also update this as we keep getting more of the - * original (e.g. for a live photo, it'll be updated once when we get the - * original image, and then again later once we get the original video). + * - For images and videos, this will be the single original. * - * @param index - * @param file - * @param onUpdate Callback invoked each time we have data about the given - * {@link file}. + * - For live photos, this will also be a two step process, first with the + * original image, then again with the video component. + * + * At this point, the data for this file will be considered final, and + * subsequent calls for the same file will return this same value unless it + * is invalidated. + * + * If at any point an error occurs, we reset our cache so that the next time + * the data is requested we repeat the process instead of continuing to + * serve the incomplete result. */ - private async enqueueUpdates( - file: EnteFile, - onUpdate: (itemData: SlideData) => void, - ) { - const update = (itemData: SlideData) => { + itemDataForFile(file: EnteFile, needsRefresh: () => void) { + let itemData = this.itemDataByFileID.get(file.id); + if (itemData) { + if (!itemData.isFinal) { + // We assume that there is only one file viewer that is using us + // at a given point of time. This assumption is currently valid. + this.needsRefreshByFileID.set(file.id, needsRefresh); + } + } else { + itemData = {}; this.itemDataByFileID.set(file.id, itemData); - onUpdate(itemData); - }; + this.needsRefreshByFileID.set(file.id, needsRefresh); + void this.enqueueUpdates(file); + } - // We might not have anything to show immediately, though in most cases - // a cached renderable thumbnail URL will be available shortly. - // - // Meanwhile, - // - // 1. Return empty slide data; PhotoSwipe will not show anything in the - // image area but will otherwise render UI controls properly. - // - // 2. Insert empty data so that we don't enqueue multiple updates. + return itemData; + } - const itemData = this.itemDataByFileID.get(file.id); - if (itemData) { - itemData = {}; - this.itemDataByFileID.set(file.id, itemData); - this.enqueueUpdates(index, file); - } - } + private async enqueueUpdates(file: EnteFile) { + const update = (itemData: ItemData) => { + this.itemDataByFileID.set(file.id, itemData); + this.needsRefreshByFileID.get(file.id)?.(); + }; const thumbnailURL = await downloadManager.renderableThumbnailURL(file); - const thumbnailData = await augmentedWithDimensions(thumbnailURL); + // TODO(PS): + const thumbnailData = await withDimensions(thumbnailURL!); update({ ...thumbnailData, isContentLoading: true, @@ -121,25 +133,29 @@ export class FileViewerDataSource { case FileType.image: { const sourceURLs = await downloadManager.renderableSourceURLs(file); - update(await augmentedWithDimensions(sourceURLs.url)); + // TODO(PS): + update(await withDimensions(sourceURLs.url as string)); break; } case FileType.video: { const sourceURLs = await downloadManager.renderableSourceURLs(file); - const disableDownload = !!this.opts.disableDownload; - update({ html: videoHTML(sourceURLs.url, disableDownload) }); + // const disableDownload = !!this.opts.disableDownload; + // update({ html: videoHTML(sourceURLs.url, disableDownload) }); + // TODO(PS): + update({ videoURL: sourceURLs.url as string }); break; } - default: { + case FileType.livePhoto: { const sourceURLs = await downloadManager.renderableSourceURLs(file); const livePhotoSourceURLs = sourceURLs.url as LivePhotoSourceURL; const imageURL = await livePhotoSourceURLs.image(); - const imageData = await augmentedWithDimensions(imageURL); + // TODO(PS): + const imageData = await withDimensions(imageURL!); update(imageData); const livePhotoVideoURL = await livePhotoSourceURLs.video(); update({ ...imageData, livePhotoVideoURL }); @@ -152,9 +168,9 @@ export class FileViewerDataSource { /** * Take a image URL, determine its dimensions using browser APIs, and return the URL * and its dimensions in a form that can directly be passed to PhotoSwipe as - * {@link SlideData}. + * {@link ItemData}. */ -const augmentedWithDimensions = (imageURL: string): Promise => +const withDimensions = (imageURL: string): Promise => new Promise((resolve) => { const image = new Image(); image.onload = () => { diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index d05e7a6304..cb85d2be91 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -1,16 +1,10 @@ /* eslint-disable */ // @ts-nocheck -import { assertionFailed } from "@/base/assert"; import log from "@/base/log"; -import { - downloadManager, - type LivePhotoSourceURL, -} from "@/gallery/services/download"; import type { EnteFile } from "@/media/file"; -import { FileType } from "@/media/file-type"; import { t } from "i18next"; -import { FileViewerDataSource, type SlideData } from "./data-source"; +import { FileViewerDataSource } from "./data-source"; import type { FileViewerProps } from "./FileViewer"; import { createPSRegisterElementIconHTML } from "./icons"; @@ -167,36 +161,27 @@ export class FileViewerPhotoSwipe { }); pswp.addFilter("itemData", (_, index) => { - const file = files[index]; + const file = files[index]!; - // We might not have anything to show immediately, though in most - // cases a cached renderable thumbnail URL will be available - // shortly. - // - // Meanwhile, - // - // 1. Return empty slide data; PhotoSwipe will not show anything in - // the image area but will otherwise render UI controls properly. - // - // 2. Insert empty data so that we don't enqueue multiple updates. - - let itemData: SlideData | undefined; - if (file) { - itemData = this.itemDataByFileID.get(file.id); - if (!itemData) { - itemData = {}; - this.itemDataByFileID.set(file.id, itemData); - this.enqueueUpdates(index, file); - } + let itemData = this.dataSource.itemDataForFile(file, () => { + this.pswp.refreshSlideContent(index); + }); + + const { videoURL, ...rest } = itemData; + if (videoURL) { + const disableDownload = !!this.opts.disableDownload; + itemData = { + ...rest, + html: videoHTML(videoURL, disableDownload), + }; } log.debug(() => ["[viewer]", { index, itemData, file }]); - if (!file) assertionFailed(); if (this.lastActivityDate != "already-hidden") this.lastActivityDate = new Date(); - return itemData ?? {}; + return itemData; }); pswp.addFilter("isContentLoading", (isLoading, content) => { @@ -390,72 +375,8 @@ export class FileViewerPhotoSwipe { // TODO(PS): Commented during testing // this.pswp.element.classList.remove("pswp--ui-visible"); } - - private async enqueueUpdates(index: number, file: EnteFile) { - const update = (itemData: SlideData) => { - this.itemDataByFileID.set(file.id, itemData); - this.pswp.refreshSlideContent(index); - }; - - const thumbnailURL = await downloadManager.renderableThumbnailURL(file); - const thumbnailData = await augmentedWithDimensions(thumbnailURL); - update({ - ...thumbnailData, - isContentLoading: true, - isContentZoomable: false, - }); - - switch (file.metadata.fileType) { - case FileType.image: { - const sourceURLs = - await downloadManager.renderableSourceURLs(file); - update(await augmentedWithDimensions(sourceURLs.url)); - break; - } - - case FileType.video: { - const sourceURLs = - await downloadManager.renderableSourceURLs(file); - const disableDownload = !!this.opts.disableDownload; - update({ html: videoHTML(sourceURLs.url, disableDownload) }); - break; - } - - default: { - const sourceURLs = - await downloadManager.renderableSourceURLs(file); - const livePhotoSourceURLs = - sourceURLs.url as LivePhotoSourceURL; - const imageURL = await livePhotoSourceURLs.image(); - const imageData = await augmentedWithDimensions(imageURL); - update(imageData); - const livePhotoVideoURL = await livePhotoSourceURLs.video(); - update({ ...imageData, livePhotoVideoURL }); - break; - } - } - } } -/** - * Take a image URL, determine its dimensions using browser APIs, and return the URL - * and its dimensions in a form that can directly be passed to PhotoSwipe as - * {@link SlideData}. - */ -const augmentedWithDimensions = (imageURL: string): Promise => - new Promise((resolve) => { - let image = new Image(); - image.onload = () => { - resolve({ - src: imageURL, - width: image.naturalWidth, - height: image.naturalHeight, - }); - }; - // TODO(PS): Handle imageElement.onerror - image.src = imageURL; - }); - const videoHTML = (url: string, disableDownload: boolean) => `