diff --git a/web/apps/photos/src/styles/photoswipe.css b/web/apps/photos/src/styles/photoswipe.css index af8fe69137..3a5016d21c 100644 --- a/web/apps/photos/src/styles/photoswipe.css +++ b/web/apps/photos/src/styles/photoswipe.css @@ -175,8 +175,31 @@ } } -/* Change the cursor on the thumbnail to the default arrow to indicate that it is - not interactable (PhotoSwipe by default shows the zoom out icon). */ +.pswp-ente .pswp__caption.ente-video { + /* Add extra offset for video captions so that they do not overlap with the + video controls. The constant was picked such that it lay above the media + controls. */ + bottom: 48px; + /* Adding a caption with a blur backdrop filter above the video element + contained in a media-controller causes a subtle tint to be overlaid on + the entire video. This is visible on both Chrome and Firefox as of + writing. + + We'll hide the caption anyways when hovering on the controls or when the + video is playing, so remove the backdrop filter and rely on the + translucent background. Note that usually the video might not even extent + to this part of the screen on desktop sized screens. */ + backdrop-filter: none; + /* Since there is too much going on in this part of the screen now, also + reduce the maximum number of lines for the caption. */ + p { + -webkit-line-clamp: 2; + line-clamp: 2; + } +} + +/* Change the cursor on the thumbnail to the default arrow to indicate that it + is not interactable (PhotoSwipe by default shows the zoom out icon). */ .pswp-ente .pswp__img { cursor: auto; } @@ -190,6 +213,27 @@ height: 100%; } +/* + I tried various ways to get media-controller to embed a normal video, but in + all methods I kept getting a flickering at the edges on Chrome. My best guess + is that it happens when the resolved size of the video is a fractional value. + + This is the current magic incantation that seems to get them to play without + the video's frame jittering. Note that the streaming hls-video elements work + fine without any workarounds, it is only the vanilla video elements that have + this issue and necessiate this workaround. The downside is that clicking + "outside" the video works for hls-video, but not for vanilla videos. + */ +.pswp-ente media-controller.ente-vanilla-video { + width: 100%; + height: 100%; + + video { + width: calc(100% - 1px); + height: calc(100% - 1px); + } +} + /* Style the custom video controls we provide. @@ -307,13 +351,9 @@ media-settings-menu { z-index: 1; } -/* Hide the caption when hovering over the video controls since they occupy - similar real estate */ -body:has( - media-control-bar:hover, - media-settings-menu:hover, - media-settings-menu:focus-within - ) { +/* Hide the caption when hovering over the video controls (and when the settings + menu is open) since they occupy similar real estate as the caption. */ +body:has(media-control-bar:hover, media-settings-menu:not([hidden])) { & .pswp-ente .pswp__caption { display: none; } diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 2a42caa492..c56f3c1275 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -1,4 +1,5 @@ import { pt } from "ente-base/i18n"; +import log from "ente-base/log"; import type { EnteFile } from "ente-media/file"; import { FileType } from "ente-media/file-type"; import "hls-video-element"; @@ -327,18 +328,32 @@ export class FileViewerPhotoSwipe { ); if (itemData.fileType === FileType.video) { - const { videoURL, videoPlaylistURL } = itemData; + const { videoPlaylistURL, videoURL } = itemData; if (videoPlaylistURL) { - const mcID = `ente-mc-${file.id}`; + const mcID = `ente-mc-hls-${file.id}`; return { ...itemData, html: hlsVideoHTML(videoPlaylistURL, mcID), mediaControllerID: mcID, }; + } else if ( + videoURL && + // TODO(HLS): + process.env.NEXT_PUBLIC_ENTE_WIP_VIDEO_STREAMING + ) { + const mcID = `ente-mc-orig-${file.id}`; + return { + ...itemData, + html: videoHTML(videoURL, mcID), + mediaControllerID: mcID, + }; } else if (videoURL) { return { ...itemData, - html: videoHTML(videoURL, !!disableDownload), + html: videoHTMLBrowserControls( + videoURL, + !!disableDownload, + ), }; } } @@ -474,18 +489,31 @@ export class FileViewerPhotoSwipe { let shouldIgnoreNextVideoQualityChange = false; /** - * If a {@link mediaControllerID} is provided, then make the media - * controls visible and link the media-control-bars (and other - * containers that house controls) to the given controller. Otherwise - * hide the media controls. + * If a {@link mediaControllerID} is present in the given + * {@link itemData}, then make the media controls visible and link the + * media-control-bars (and other containers that house controls) to the + * given controller. Otherwise hide the media controls. */ - const updateMediaControls = (mediaControllerID: string | undefined) => { + const updateMediaControls = (itemData: ItemData) => { + // For reasons possibily related to the 1 tick wait in the hls-video + // implementation (`await Promise.resolve()`), the association + // between media-controller and media-control-bar doesn't get + // established on the first slide if we reopen the file viewer. + // + // See also: https://github.com/muxinc/media-chrome/issues/940 + // + // As a workaround, defer the association to the next tick. + setTimeout(() => _updateMediaControls(itemData), 0); + }; + + const _updateMediaControls = (itemData: ItemData) => { const container = mediaControlsContainerElement; const controls = container?.querySelectorAll( "media-control-bar, media-playback-rate-menu", ) ?? []; for (const control of controls) { + const { mediaControllerID } = itemData; if (mediaControllerID) { control.setAttribute("mediacontroller", mediaControllerID); } else { @@ -493,12 +521,13 @@ export class FileViewerPhotoSwipe { } } - const qualityMenu = container?.querySelector("#et-quality-menu"); + // TODO(HLS): Temporary gate + if (!process.env.NEXT_PUBLIC_ENTE_WIP_VIDEO_STREAMING) return; + + const qualityMenu = container?.querySelector("#ente-quality-menu"); if (qualityMenu instanceof MediaChromeMenu) { - const value = - videoQualityForFile(currentFile()) == "auto" - ? pt("Auto") - : pt("Original"); + const { videoPlaylistURL } = itemData; + const value = videoPlaylistURL ? pt("Auto") : pt("Original"); // Check first, and set a flag, to avoid infinite update loop. if (qualityMenu.value != value) { shouldIgnoreNextVideoQualityChange = true; @@ -508,23 +537,27 @@ export class FileViewerPhotoSwipe { }; pswp.on("contentAppend", (e) => { - const { fileID, fileType, videoURL, mediaControllerID } = - asItemData(e.content.data); + // PhotoSwipe emits stale contentAppend events. e.g. when changing + // the video quality, we'll first get "contentAppend" (and "change") + // with the latest item data, but then later PhotoSwipe will call + // "contentAppend" again with stale data. + // + // To ignore these, we check the `hasSlide` attribute. I'm not sure + // if this is a foolproof workaround. + // + // See also https://github.com/dimsemenov/PhotoSwipe/issues/2045. + if (!e.content.hasSlide) { + log.debug(() => ["Ignoring stale contentAppend", e]); + return; + } + + const { fileID, fileType, videoURL } = asItemData(e.content.data); // For the initial slide, "contentAppend" will get called after the // "change" event, so we need to wire up the controls, or hide them, // for the initial slide here also (in addition to in "change"). if (currSlideData().fileID == fileID) { - // For reasons possibily related to the 1 tick waits in the - // hls-video implementation (`await Promise.resolve()`), the - // association between media-controller and media-control-bar - // doesn't get established on the first slide if we reopen the - // file viewer. - // - // See also: https://github.com/muxinc/media-chrome/issues/940 - // - // As a workaround, defer the association to the next tick. - setTimeout(() => updateMediaControls(mediaControllerID), 0); + updateMediaControls(currSlideData()); } // Rest of this function deals with live photos. @@ -621,8 +654,9 @@ export class FileViewerPhotoSwipe { video?.pause(); }); - pswp.on("loadComplete", (e) => - updateFileInfoExifIfNeeded(asItemData(e.content.data)), + pswp.on( + "loadComplete", + (e) => void updateFileInfoExifIfNeeded(asItemData(e.content.data)), ); pswp.on( @@ -634,21 +668,6 @@ export class FileViewerPhotoSwipe { forgetExifForItemData(asItemData(e.content.data)), ); - /** - * The media-chrome-button elements (e.g the play button) retain focus - * after clicking on them. e.g., if I click the "media-mute-button" to - * activate it, then later press Space or Enter, then the mute button - * activates again instead of toggling video playback. - * - * I'm not sure who is at fault here, but this behaviour ends up being - * irritating. To prevent this from happening, drop the focus from any - * media chrome button when playback starts. - */ - const resetFocus = () => { - const activeElement = document.activeElement; - if (activeElement instanceof HTMLElement) activeElement.blur(); - }; - /** * If the current slide is showing a video, then the DOM video element * showing that video. @@ -712,7 +731,6 @@ export class FileViewerPhotoSwipe { if (videoVideoEl) { onVideoPlayback = () => { - resetFocus(); showIf(captionElement!, !!videoVideoEl?.paused); }; @@ -759,14 +777,6 @@ export class FileViewerPhotoSwipe { if (muteButton instanceof MediaMuteButton) muteButton.handleClick(); }; - // The PhotoSwipe dialog has being closed and the animations have - // completed. - pswp.on("destroy", () => { - fileViewerDidClose(); - // Let our parent know that we have been closed. - onClose(); - }); - const handleViewInfo = () => onViewInfo(currentAnnotatedFile()); let favoriteButtonElement: HTMLButtonElement | undefined; @@ -846,12 +856,26 @@ export class FileViewerPhotoSwipe { const menuButton = document.querySelector( "media-settings-menu-button", ); - if (menuButton instanceof MediaChromeMenuButton) + if (menuButton instanceof MediaChromeMenuButton) { menuButton.handleClick(); + // See: [Note: Media chrome focus workaround] + // + // Whatever media chrome is doing internally, it requires us to + // drop the focus multiple times (Removing either of these calls + // is not enough). + const blurAllFocused = () => + document + .querySelectorAll(":focus") + .forEach((e) => e instanceof HTMLElement && e.blur()); + + blurAllFocused(); + setTimeout(blurAllFocused, 0); + } + // Refresh the slide so that the video is fetched afresh, but using // the updated `originalVideoFileIDs` value for it. - this.refreshCurrentSlideContent(); + pswp.refreshSlideContent(pswp.currIndex); }; const showIf = (element: HTMLElement, condition: boolean) => @@ -1017,14 +1041,13 @@ export class FileViewerPhotoSwipe { html: hlsVideoControlsHTML(), onInit: (element, pswp) => { mediaControlsContainerElement = element; - const menu = element.querySelector("#et-quality-menu"); + const menu = element.querySelector("#ente-quality-menu"); if (menu instanceof MediaChromeMenu) { menu.addEventListener("change", onVideoQualityChange); } - pswp.on("change", () => { - const { mediaControllerID } = currSlideData(); - updateMediaControls(mediaControllerID); - }); + pswp.on("change", () => + updateMediaControls(currSlideData()), + ); }, }); @@ -1048,11 +1071,10 @@ export class FileViewerPhotoSwipe { const { fileType, alt } = currSlideData(); element.querySelector("p")!.innerText = alt ?? ""; element.style.visibility = alt ? "visible" : "hidden"; - // Add extra offset for video captions so that they do - // not overlap with the video controls. The constant is - // such that it lies above the media controls. - element.style.bottom = - fileType === FileType.video ? "44px" : "0"; + element.classList.toggle( + "ente-video", + fileType == FileType.video, + ); }); }, }); @@ -1189,7 +1211,8 @@ export class FileViewerPhotoSwipe { // indicator, we want the Escape key to blur its focus instead of // closing the PhotoSwipe dialog. if (isFocusVisibledOnUIControl() && key == "Escape") { - resetFocus(); + const activeElement = document.activeElement; + if (activeElement instanceof HTMLElement) activeElement.blur(); pswpEvent.preventDefault(); return; } @@ -1289,6 +1312,60 @@ export class FileViewerPhotoSwipe { cb?.(); }); + /** + * [Note: Media chrome focus workaround] + * + * The media-chrome-button elements (e.g, the play button, but also + * others, including the menu) retain focus after clicking on them. + * e.g., if I click the "media-mute-button" to activate it, then the + * mute button grabs focus (but not :focus-visible). So it doesn't + * appear focused visually, but then later if I press Space or Enter, + * then the mute button activates again instead of toggling video + * playback (as our keyboard shortcut is meant to do). + * + * I'm not sure who is at fault here, but this behaviour ends up being + * irritating. e.g. say I change the quality in the menu, and press + * space to play - well, the space no longer works because the media + * chrome has grabbed focus and instead activates itself, reopening the + * settings menu. + * + * As a workaround, we ask media chrome to drop focus on mouse clicks. + * This should not impact keyboard activations. + * + * This workaround is likely to cause problems in the future, but I + * can't find a better way short of upstream media chrome changes. + */ + const blurMediaChromeFocus = (e: MouseEvent) => { + const target = e.target; + if (target instanceof HTMLElement) { + switch (target.tagName) { + case "MEDIA-TIME-RANGE": + case "MEDIA-PLAY-BUTTON": + case "MEDIA-MUTE-BUTTON": + case "MEDIA-PIP-BUTTON": + case "MEDIA-FULLSCREEN-BUTTON": + setTimeout(() => target.blur(), 0); + break; + } + } + }; + + pswp.on("initialLayout", () => { + pswp.element!.addEventListener("mousedown", blurMediaChromeFocus); + }); + + // The PhotoSwipe dialog has being closed and the animations have + // completed. + pswp.on("destroy", () => { + pswp.element?.removeEventListener( + "mousedown", + blurMediaChromeFocus, + ); + fileViewerDidClose(); + // Let our parent know that we have been closed. + onClose(); + }); + // Let our data source know that we're about to open. fileViewerWillOpen(); @@ -1341,7 +1418,7 @@ export class FileViewerPhotoSwipe { refreshCurrentSlideFavoriteButtonIfNeeded: () => void; } -const videoHTML = (url: string, disableDownload: boolean) => ` +const videoHTMLBrowserControls = (url: string, disableDownload: boolean) => `