From 5a7f83212babdfe7c1884059b7b248eff59fb78f Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 09:14:45 +0530 Subject: [PATCH 01/12] Tweak --- web/apps/photos/src/styles/photoswipe.css | 12 ++++++++---- web/packages/gallery/components/viewer/photoswipe.ts | 8 ++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/web/apps/photos/src/styles/photoswipe.css b/web/apps/photos/src/styles/photoswipe.css index d2276b61c9..1b03c87e4f 100644 --- a/web/apps/photos/src/styles/photoswipe.css +++ b/web/apps/photos/src/styles/photoswipe.css @@ -221,10 +221,14 @@ [Note: Showing menus outside of media-controller] - This is all very hacky. None of the media controller examples currently - show menus work for our specific scenario (the controls detached from the - the media controller itself, and shown at the bottom of the screen). As - such, a lot of undocumented and arbitrary tweaks are needed. This is a + This is all very hacky. The standalone control example + (https://media-chrome.mux.dev/examples/vanilla/standalone-controls.html), + doesn't (as of now) include a menu, so it is possible this is just not + supported. None of the media controller examples I could find show menus + work for our specific scenario (the controls detached from the the media + controller itself, and shown at the bottom of the screen). + + As such, a lot of undocumented and arbitrary tweaks are needed. This is a house of cards, and might fall when the media-chrome version is updated; on the other hand, they might also add better support for our scenario, in which case this should be possible straightforwardly. diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 37e1fcbff4..aea0ee6834 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -470,9 +470,9 @@ export class FileViewerPhotoSwipe { const { fileID, fileType, videoURL, mediaControllerID } = asItemData(e.content.data); - // For the initial slide, "contentAppend" will get called after - // "change", so we need to wire up the controls (or hide them) for - // the initial slide here also (in addition to in "change"). + // 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 @@ -483,11 +483,11 @@ export class FileViewerPhotoSwipe { // See also: https://github.com/muxinc/media-chrome/issues/940 // // As a workaround, defer the association to the next tick. - // setTimeout(() => updateMediaControls(mediaControllerID), 0); } // Rest of this function deals with live photos. + if (fileType != FileType.livePhoto) return; if (!videoURL) return; From b9bb7c074d441ccd8dc8d0c9fd278a841c17c17b Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 10:17:19 +0530 Subject: [PATCH 02/12] Pause --- web/packages/gallery/components/viewer/photoswipe.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index aea0ee6834..a5eff4060a 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -558,8 +558,9 @@ export class FileViewerPhotoSwipe { // Pause the video element, if any, when we move away from the // slide. - const video = - e.content.slide?.container.getElementsByTagName("video")[0]; + const video = e.content.slide?.container.querySelector( + "video, hls-video", + ) as HTMLVideoElement | null; video?.pause(); }); From 6bbfcb1d136cb7cee11d060e066cfb47ef457787 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 10:26:44 +0530 Subject: [PATCH 03/12] abs ref https://github.com/muxinc/media-chrome/discussions/890 --- .../gallery/components/viewer/photoswipe.ts | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index a5eff4060a..901406b123 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -544,6 +544,24 @@ export class FileViewerPhotoSwipe { video.style.height = `${height}px`; }); + /** + * Get the video element, if any, that is a descendant of the given HTML + * element. + * + * While the return type is an {@link HTMLVideoElement}, the result can + * also be an instance of a media-chrome `CustomVideoElement`, + * specifically a {@link HlsVideoElement}. + * https://github.com/muxinc/media-elements/blob/main/packages/hls-video-element/hls-video-element.js#L284 + * + * The media-chrome `CustomVideoElement`s provide the same API as the + * browser's built-in {@link HTMLVideoElement}s, so we can use the same + * methods on them. + * + * For ergonomic use at call sites, it accepts an optional. + */ + const queryVideoElement = (element: HTMLElement | undefined) => + element?.querySelector("video, hls-video"); + pswp.on("contentDeactivate", (e) => { // Reset failures, if any, for this file so that the fetch is tried // again when we come back to it^. @@ -558,9 +576,7 @@ export class FileViewerPhotoSwipe { // Pause the video element, if any, when we move away from the // slide. - const video = e.content.slide?.container.querySelector( - "video, hls-video", - ) as HTMLVideoElement | null; + const video = queryVideoElement(e.content.slide?.container); video?.pause(); }); @@ -580,6 +596,8 @@ export class FileViewerPhotoSwipe { /** * If the current slide is showing a video, then the DOM video element * showing that video. + * + * See also {@link queryVideoElement}. */ let videoVideoEl: HTMLVideoElement | undefined; @@ -634,7 +652,7 @@ export class FileViewerPhotoSwipe { // It works subsequently, which is why, e.g., we can use it to // pause the video in "contentDeactivate". const contentElement = pswp.currSlide?.content.element; - videoVideoEl = contentElement?.getElementsByTagName("video")[0]; + videoVideoEl = queryVideoElement(contentElement) ?? undefined; if (videoVideoEl) { onVideoPlayback = () => From e15fb04ee0a4dec3b472c9b8a21c7a1338760da1 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 10:35:20 +0530 Subject: [PATCH 04/12] Caption positioning --- web/apps/photos/src/styles/photoswipe.css | 4 +- .../gallery/components/viewer/photoswipe.ts | 39 ++++++++++--------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/web/apps/photos/src/styles/photoswipe.css b/web/apps/photos/src/styles/photoswipe.css index 1b03c87e4f..dd072cc580 100644 --- a/web/apps/photos/src/styles/photoswipe.css +++ b/web/apps/photos/src/styles/photoswipe.css @@ -155,8 +155,8 @@ bottom: 0px; right: 0; margin: 20px 24px; - padding: 6px 16px; - border-radius: 2px; + padding: 8px 16px; + border-radius: 3px; /* Same opacity as the other controls. */ color: rgb(255 255 255 / 0.85); background-color: rgb(0 0 0 / 0.2); diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 901406b123..4bfae068b0 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -910,11 +910,26 @@ export class FileViewerPhotoSwipe { }); ui.registerElement({ - name: "caption", - // Arbitrary order towards the end (it doesn't matter anyways - // since we're absolutely positioned). + name: "media-controls", + // Arbitrary order towards the end. order: 30, appendTo: "root", + html: hlsVideoControlsHTML(), + onInit: (element, pswp) => { + mediaControlsContainerElement = element; + pswp.on("change", () => { + const { mediaControllerID } = currSlideData(); + updateMediaControls(mediaControllerID); + }); + }, + }); + + ui.registerElement({ + name: "caption", + // After the video controls so that we don't get occluded by + // them (nb: the caption will hide when the video is playing). + order: 31, + appendTo: "root", tagName: "p", onInit: (element, pswp) => { captionElement = element; @@ -924,23 +939,9 @@ export class FileViewerPhotoSwipe { 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 - // an ad-hoc value that looked okay-ish across browsers. + // such that it lies above the media controls. element.style.bottom = - fileType === FileType.video ? "36px" : "0"; - }); - }, - }); - - ui.registerElement({ - name: "media-controls", - order: 31, - appendTo: "root", - html: hlsVideoControlsHTML(), - onInit: (element, pswp) => { - mediaControlsContainerElement = element; - pswp.on("change", () => { - const { mediaControllerID } = currSlideData(); - updateMediaControls(mediaControllerID); + fileType === FileType.video ? "44px" : "0"; }); }, }); From cfe6343d63e6c29741d1d7b80a3d25d83d5ecd80 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 10:52:23 +0530 Subject: [PATCH 05/12] Fix partial clipped lines showing through on the caption --- web/apps/photos/src/styles/photoswipe.css | 19 ++++++++++--------- .../gallery/components/viewer/photoswipe.ts | 11 +++++++++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/web/apps/photos/src/styles/photoswipe.css b/web/apps/photos/src/styles/photoswipe.css index dd072cc580..cdc709c6d7 100644 --- a/web/apps/photos/src/styles/photoswipe.css +++ b/web/apps/photos/src/styles/photoswipe.css @@ -155,23 +155,24 @@ bottom: 0px; right: 0; margin: 20px 24px; - padding: 8px 16px; + padding-inline: 16px; border-radius: 3px; /* Same opacity as the other controls. */ color: rgb(255 255 255 / 0.85); background-color: rgb(0 0 0 / 0.2); backdrop-filter: blur(10px); - /* 4 lines max, ellipsis on overflow. */ - word-break: break-word; text-align: right; max-width: 375px; max-height: 200px; - overflow: hidden; - text-overflow: ellipsis; - display: -webkit-box; - -webkit-box-orient: vertical; - -webkit-line-clamp: 4; - line-clamp: 4; + p { + /* 4 lines max, ellipsis on overflow. */ + word-break: break-word; + overflow: hidden; + display: -webkit-box; + -webkit-box-orient: vertical; + -webkit-line-clamp: 4; + line-clamp: 4; + } } /* Change the cursor on the thumbnail to the default arrow to indicate that it is diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 4bfae068b0..39cb87e085 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -930,12 +930,19 @@ export class FileViewerPhotoSwipe { // them (nb: the caption will hide when the video is playing). order: 31, appendTo: "root", - tagName: "p", + // The caption uses the line-clamp CSS property, which behaves + // unexpectedly when we also assign padding to the "p" element + // on which we're setting the line clamp: the "clipped" lines + // show through in the padding area. + // + // As a workaround, wrap the p in a div. Set the line-clamp on + // the p, and the padding on the div. + html: "

", onInit: (element, pswp) => { captionElement = element; pswp.on("change", () => { const { fileType, alt } = currSlideData(); - element.innerText = alt ?? ""; + 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 From 9d222cd007ddc691e101893581068f52f882ef3c Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 11:34:13 +0530 Subject: [PATCH 06/12] hk --- .../gallery/components/viewer/photoswipe.ts | 60 ++++++++++++++++--- 1 file changed, 51 insertions(+), 9 deletions(-) diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 39cb87e085..ac1ae807af 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -551,7 +551,7 @@ export class FileViewerPhotoSwipe { * While the return type is an {@link HTMLVideoElement}, the result can * also be an instance of a media-chrome `CustomVideoElement`, * specifically a {@link HlsVideoElement}. - * https://github.com/muxinc/media-elements/blob/main/packages/hls-video-element/hls-video-element.js#L284 + * https://github.com/muxinc/media-elements/blob/main/packages/hls-video-element/hls-video-element.js * * The media-chrome `CustomVideoElement`s provide the same API as the * browser's built-in {@link HTMLVideoElement}s, so we can use the same @@ -978,14 +978,36 @@ export class FileViewerPhotoSwipe { // Actions we handle ourselves. + const handlePreviousSlide = () => pswp.prev(); + + const handleNextSlide = () => pswp.next(); + + const handleSeekBackOrPreviousSlide = () => { + const vid = videoVideoEl; + if (vid) { + vid.currentTime = Math.max(vid.currentTime - 5, 0); + } else { + handlePreviousSlide(); + } + }; + + const handleSeekForwardOrNextSlide = () => { + const vid = videoVideoEl; + if (vid) { + vid.currentTime = vid.currentTime + 5; + } else { + handleNextSlide(); + } + }; + const handleTogglePlayIfPossible = () => { switch (currentAnnotatedFile().itemData.fileType) { case FileType.video: videoTogglePlayIfPossible(); - return; + break; case FileType.livePhoto: livePhotoTogglePlayIfPossible(); - return; + break; } }; @@ -993,10 +1015,10 @@ export class FileViewerPhotoSwipe { switch (currentAnnotatedFile().itemData.fileType) { case FileType.video: videoToggleMuteIfPossible(); - return; + break; case FileType.livePhoto: livePhotoToggleMuteIfPossible(); - return; + break; } }; @@ -1048,15 +1070,25 @@ export class FileViewerPhotoSwipe { // For example, Cmd-D adds a bookmark, which is why we don't use it // for download. // - // An exception is Ctrl/Cmd-C, which we intercept to copy the image - // since that should match the user's expectation. + // There are some exception, e.g. Ctrl/Cmd-C, which we intercept to + // copy the image since that should match the user's expectation. let cb: (() => void) | undefined; if (e.shiftKey) { // Ignore except "?" for help. if (key == "?") cb = handleHelp; } else if (e.altKey) { - // Ignore. + // Ignore except if for arrow keys since when showing a video, + // the arrow keys are used for seeking, and the normal arrow key + // function (slide movement) needs the Alt/Opt modifier. + switch (key) { + case "ArrowLeft": + cb = handlePreviousSlide; + break; + case "ArrowRight": + cb = handleNextSlide; + break; + } } else if (e.metaKey || e.ctrlKey) { // Ignore except Ctrl/Cmd-C for copy if (lkey == "c") cb = handleCopy; @@ -1073,6 +1105,16 @@ export class FileViewerPhotoSwipe { case "Delete": cb = handleDelete; break; + case "ArrowLeft": + cb = handleSeekBackOrPreviousSlide; + // Prevent PhotoSwipe's default handling of this key. + pswpEvent.preventDefault(); + break; + case "ArrowRight": + cb = handleSeekForwardOrNextSlide; + // Prevent PhotoSwipe's default handling of this key. + pswpEvent.preventDefault(); + break; // We check for "?"" both with an without shift, since some // keyboards might have it emittable without shift. case "?": @@ -1180,7 +1222,7 @@ const videoHTML = (url: string, disableDownload: boolean) => ` // // TODO(HLS): Update code above that searches for the video element const hlsVideoHTML = (url: string, mediaControllerID: string) => ` - + `; From 7b169fe90363a52602d588a51d6f960650decc5c Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 13:59:19 +0530 Subject: [PATCH 07/12] focus workaround --- .../gallery/components/viewer/photoswipe.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index ac1ae807af..f471457a46 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -593,6 +593,21 @@ 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. @@ -655,8 +670,10 @@ export class FileViewerPhotoSwipe { videoVideoEl = queryVideoElement(contentElement) ?? undefined; if (videoVideoEl) { - onVideoPlayback = () => + onVideoPlayback = () => { + resetFocus(); showIf(captionElement!, !!videoVideoEl?.paused); + }; videoVideoEl.addEventListener("play", onVideoPlayback); videoVideoEl.addEventListener("pause", onVideoPlayback); From 09b4025f11d48957972a826e536d721583f1cd4f Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 14:25:18 +0530 Subject: [PATCH 08/12] cap --- web/apps/photos/src/styles/photoswipe.css | 8 ++++++++ .../gallery/components/viewer/photoswipe.ts | 13 ++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/web/apps/photos/src/styles/photoswipe.css b/web/apps/photos/src/styles/photoswipe.css index cdc709c6d7..680e7e033b 100644 --- a/web/apps/photos/src/styles/photoswipe.css +++ b/web/apps/photos/src/styles/photoswipe.css @@ -307,3 +307,11 @@ 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) { + & .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 f471457a46..bb99646a0c 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -1000,7 +1000,11 @@ export class FileViewerPhotoSwipe { const handleNextSlide = () => pswp.next(); const handleSeekBackOrPreviousSlide = () => { - const vid = videoVideoEl; + // TODO(HLS): Behind temporary flag + // const vid = videoVideoEl; + const vid = process.env.NEXT_PUBLIC_ENTE_WIP_VIDEO_STREAMING + ? videoVideoEl + : undefined; if (vid) { vid.currentTime = Math.max(vid.currentTime - 5, 0); } else { @@ -1009,7 +1013,11 @@ export class FileViewerPhotoSwipe { }; const handleSeekForwardOrNextSlide = () => { - const vid = videoVideoEl; + // TODO(HLS): Behind temporary flag + // const vid = videoVideoEl; + const vid = process.env.NEXT_PUBLIC_ENTE_WIP_VIDEO_STREAMING + ? videoVideoEl + : undefined; if (vid) { vid.currentTime = vid.currentTime + 5; } else { @@ -1237,7 +1245,6 @@ const videoHTML = (url: string, disableDownload: boolean) => ` // import "media-chrome"; // import "media-chrome/menu"; // -// TODO(HLS): Update code above that searches for the video element const hlsVideoHTML = (url: string, mediaControllerID: string) => ` From 40812ec7c376e410e8931958438adb1a025f15c9 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 14:44:57 +0530 Subject: [PATCH 09/12] More robust --- web/apps/photos/src/styles/photoswipe.css | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/web/apps/photos/src/styles/photoswipe.css b/web/apps/photos/src/styles/photoswipe.css index 680e7e033b..da2840260a 100644 --- a/web/apps/photos/src/styles/photoswipe.css +++ b/web/apps/photos/src/styles/photoswipe.css @@ -310,7 +310,11 @@ media-settings-menu { /* Hide the caption when hovering over the video controls since they occupy similar real estate */ -body:has(media-control-bar:hover) { +body:has( + media-control-bar:hover, + media-settings-menu:hover, + media-settings-menu:focus-within + ) { & .pswp-ente .pswp__caption { display: none; } From 2cbc4998df0418a1802a35f175f9f2e9e796d8dc Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 14:53:34 +0530 Subject: [PATCH 10/12] Couldn't connect when I tried it Cast is also not supported by hls-video, can investigate both these together. --- web/apps/photos/src/styles/photoswipe.css | 1 - web/packages/gallery/components/viewer/photoswipe.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/web/apps/photos/src/styles/photoswipe.css b/web/apps/photos/src/styles/photoswipe.css index da2840260a..af8fe69137 100644 --- a/web/apps/photos/src/styles/photoswipe.css +++ b/web/apps/photos/src/styles/photoswipe.css @@ -286,7 +286,6 @@ media-time-range:hover { they trigger. */ media-pip-button[mediapipunavailable], -media-airplay-button[mediaairplayunavailable], media-fullscreen-button[mediafullscreenunavailable] { display: none; } diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index bb99646a0c..9228075ee5 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -1295,7 +1295,6 @@ const hlsVideoControlsHTML = () => ` ${settingsSVGPath} - From fc480e8ce6fe21b39c433b8c02eb190f57a5f253 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 15:52:38 +0530 Subject: [PATCH 11/12] Tried tr https://www.media-chrome.org/docs/en/internationalization/adding-language-support --- .../gallery/components/viewer/photoswipe.ts | 19 +++++++++++++------ web/packages/gallery/package.json | 2 +- web/yarn.lock | 8 ++++---- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 9228075ee5..3830617c70 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -1257,8 +1257,6 @@ const hlsVideoHTML = (url: string, mediaControllerID: string) => ` * To make these functional, the `media-control-bar` requires the * `mediacontroller="${mediaControllerID}"` attribute. * - * TODO(HLS): Tooltips get clipped - * TODO(HLS): Translation * TODO(HLS): Spurious console warning * * Notes: @@ -1269,6 +1267,15 @@ const hlsVideoHTML = (url: string, mediaControllerID: string) => ` * indicator (browser specific) in the in-page video element. * * - The media-cast-button currently doesn't work with the `hls-video` player. + * + * - Media chrome has mechanism for statically providing translations but it + * wasn't working when I tried with 4.9.0. The media chrome tooltips also get + * clipped for the cornermost buttons. Finally, the rest of the buttons on + * this screen don't have a tooltip either. + * + * Revisit this when we have a custom tooltip element we can then also use on + * this screen, which can also be used enhancement for the other buttons on + * this screen which use "title" (which get clipped when they are multi-word). */ const hlsVideoControlsHTML = () => `
@@ -1287,15 +1294,15 @@ const hlsVideoControlsHTML = () => ` - - + + ${settingsSVGPath} - - + +
`; diff --git a/web/packages/gallery/package.json b/web/packages/gallery/package.json index c9a87b6ba3..c7a9ba0a4f 100644 --- a/web/packages/gallery/package.json +++ b/web/packages/gallery/package.json @@ -11,7 +11,7 @@ "hls-video-element": "^1.5.0", "leaflet": "^1.9.4", "leaflet-defaulticon-compatibility": "^0.1.2", - "media-chrome": "^4.8.0", + "media-chrome": "^4.9.0", "photoswipe": "^5.4.4", "react-dropzone": "14.2.10" }, diff --git a/web/yarn.lock b/web/yarn.lock index 997ce4d01f..50abd1b02e 100644 --- a/web/yarn.lock +++ b/web/yarn.lock @@ -2884,10 +2884,10 @@ math-intrinsics@^1.1.0: resolved "https://registry.yarnpkg.com/math-intrinsics/-/math-intrinsics-1.1.0.tgz#a0dd74be81e2aa5c2f27e65ce283605ee4e2b7f9" integrity sha512-/IXtbwEk5HTPyEwyKX6hGkYXxM9nbj64B+ilVJnC/R6B0pH5G4V3b0pVbL7DBj4tkhBAppbQUlf6F6Xl9LHu1g== -media-chrome@^4.8.0: - version "4.8.0" - resolved "https://registry.yarnpkg.com/media-chrome/-/media-chrome-4.8.0.tgz#ff881a5466fe02ad07344a370a8a1106988ca245" - integrity sha512-oioEGlluW+1RqknqsszrKHDs3NZ9AaatEaE2kYYOSWxnwvVmhRTfDWT4JeMgtUr5r3i2dAI3e/qbeb1j+a0MhA== +media-chrome@^4.9.0: + version "4.9.0" + resolved "https://registry.yarnpkg.com/media-chrome/-/media-chrome-4.9.0.tgz#05f692781234dfb02baf1bdbc578af62b7edeeb7" + integrity sha512-KSGZUDEt0vg0Ogx+B8YYj0O32l0ppPLR8s5rXBMOEpkLYbJrKfhSPfwTmjJGcp2Nf3F/X2ddwO96YvUXHYeYvw== dependencies: ce-la-react "^0.1.3" From 822285fd3d29b96ee93471fefcaafc5db9519c5e Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 2 Apr 2025 17:19:18 +0530 Subject: [PATCH 12/12] Fin --- web/packages/base/log-web.ts | 13 +++++++++++++ .../gallery/components/viewer/photoswipe.ts | 7 ++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/web/packages/base/log-web.ts b/web/packages/base/log-web.ts index 12801ab08c..be63612875 100644 --- a/web/packages/base/log-web.ts +++ b/web/packages/base/log-web.ts @@ -31,6 +31,19 @@ export const logStartupBanner = (userID?: number) => { */ export const logUnhandledErrorsAndRejections = (attach: boolean) => { const handleError = (event: ErrorEvent) => { + // [Note: Spurious media chrome resize observer errors] + // + // When attaching media chrome controls to the DOM, we get an (AFAICT) + // spurious error in the log. Ignore it. FWIW, the media control tests + // themselves do the same. + // https://github.com/muxinc/elements/blob/f602519f544509f15add8fcc8cbbf7379843dcd3/packages/mux-player/test/player.test.js#L6-L12C5 + if ( + event.message == + "ResizeObserver loop completed with undelivered notifications." + ) { + return; + } + log.error("Unhandled error", event.error ?? event.message); }; diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 3830617c70..19fbb14420 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -1257,14 +1257,13 @@ const hlsVideoHTML = (url: string, mediaControllerID: string) => ` * To make these functional, the `media-control-bar` requires the * `mediacontroller="${mediaControllerID}"` attribute. * - * TODO(HLS): Spurious console warning - * * Notes: * * - Examples: https://media-chrome.mux.dev/examples/vanilla/ * * - When PiP is active and the video moves out, the browser displays some - * indicator (browser specific) in the in-page video element. + * indicator (browser specific) in the in-page video element. This element and + * text is not under our control. * * - The media-cast-button currently doesn't work with the `hls-video` player. * @@ -1276,6 +1275,8 @@ const hlsVideoHTML = (url: string, mediaControllerID: string) => ` * Revisit this when we have a custom tooltip element we can then also use on * this screen, which can also be used enhancement for the other buttons on * this screen which use "title" (which get clipped when they are multi-word). + * + * - See: [Note: Spurious media chrome resize observer errors] */ const hlsVideoControlsHTML = () => `