From 0804d4cc3f398e5e50bbee229a5876d6d499431b Mon Sep 17 00:00:00 2001 From: Alexander Harding Date: Wed, 11 Dec 2024 20:24:50 -0600 Subject: [PATCH 1/3] fix: nsfw blur sometimes not applied for crossposts in remote communities resolves #1770 --- src/features/media/InlineMedia.tsx | 21 ++++++++++-- src/features/media/useMediaLoadObserver.ts | 38 +++++++++++++++++----- 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/features/media/InlineMedia.tsx b/src/features/media/InlineMedia.tsx index defbd7b3e1..f5ae758333 100644 --- a/src/features/media/InlineMedia.tsx +++ b/src/features/media/InlineMedia.tsx @@ -1,4 +1,4 @@ -import { CSSProperties } from "react"; +import { CSSProperties, useEffect } from "react"; import Media, { MediaProps } from "#/features/media/Media"; import { cx } from "#/helpers/css"; @@ -38,6 +38,23 @@ export default function InlineMedia({ */ const aspectRatio = useLatch(currentAspectRatio); + // Workaround. useMediaLoadObserver won't fire if a new image is loaded (in Safari) + // because we have already forced its dimensions (via latching aspect ratio) + // + // This causes issues like https://github.com/aeharding/voyager/issues/1770 + // + // For now, assume that a swapped image will have the same dimensions + // + // In the future, useMediaLoadObserver should render the image somewhere + // in the document root separate from the image rendered by InlineMedia + useEffect(() => { + // If we have a src, and the latched aspect ratio is being used, update the store + // (src has changed) + if (src && aspectRatio && !currentAspectRatio) { + dispatch(imageLoaded({ src, aspectRatio })); + } + }, [src, aspectRatio, currentAspectRatio, dispatch]); + function buildPlaceholderState() { if (aspectRatio === IMAGE_FAILED) return "error"; if (!aspectRatio) return "loading"; @@ -74,7 +91,7 @@ export default function InlineMedia({ // TLDR Image loading should still work with this function commented out! onLoad={(event) => { if (!src) return; - if (isLoadedAspectRatio(aspectRatio)) return; + if (isLoadedAspectRatio(currentAspectRatio)) return; const dimensions = getTargetDimensions( event.target as HTMLImageElement, diff --git a/src/features/media/useMediaLoadObserver.ts b/src/features/media/useMediaLoadObserver.ts index a0c351c566..3b8631c5c4 100644 --- a/src/features/media/useMediaLoadObserver.ts +++ b/src/features/media/useMediaLoadObserver.ts @@ -16,15 +16,9 @@ export default function useMediaLoadObserver(src: string | undefined) { let destroyed = false; function setupObserver() { - if (destroyed) return; - - if (isLoadedAspectRatio(aspectRatio)) return; if (!src) return; - if (!mediaRef.current) { - // react-reverse-portal refs can take some time to setup. Try again on next paint - requestAnimationFrame(setupObserver); - return; - } + if (isLoadedAspectRatio(aspectRatio)) return; + if (!mediaRef.current) return; const handleResize = (entries: ResizeObserverEntry[]) => { for (const entry of entries) { @@ -45,7 +39,33 @@ export default function useMediaLoadObserver(src: string | undefined) { resizeObserverRef.current.observe(mediaRef.current); } - setupObserver(); + function setup() { + if (destroyed) return; + + if (!src) return; + if (isLoadedAspectRatio(aspectRatio)) return; + if (!mediaRef.current) { + // react-reverse-portal refs can take some time to setup. Try again on next paint + requestAnimationFrame(setup); + return; + } + + const dimensions = getTargetDimensions(mediaRef.current); + + if (dimensions) { + dispatch( + imageLoaded({ + src, + aspectRatio: dimensions.width / dimensions.height, + }), + ); + return; // found dimensions synchronously, no need to setup observer + } + + setupObserver(); + } + + setup(); return () => { destroyed = true; From 798b8cf4f84d4a34e77af2c876b8f64e18b2ada1 Mon Sep 17 00:00:00 2001 From: Alexander Harding Date: Wed, 11 Dec 2024 20:51:53 -0600 Subject: [PATCH 2/3] Better solution imo --- src/features/media/InlineMedia.tsx | 32 +------ src/features/media/useMediaLoadObserver.ts | 38 ++------ src/features/post/crosspost/Crosspost.tsx | 58 ++---------- ...odule.css => CrosspostContents.module.css} | 0 .../post/crosspost/CrosspostContents.tsx | 93 +++++++++++++++++++ src/features/post/inFeed/usePostSrc.ts | 8 +- src/helpers/useLatch.ts | 13 --- 7 files changed, 119 insertions(+), 123 deletions(-) rename src/features/post/crosspost/{Crosspost.module.css => CrosspostContents.module.css} (100%) create mode 100644 src/features/post/crosspost/CrosspostContents.tsx delete mode 100644 src/helpers/useLatch.ts diff --git a/src/features/media/InlineMedia.tsx b/src/features/media/InlineMedia.tsx index f5ae758333..9c72624326 100644 --- a/src/features/media/InlineMedia.tsx +++ b/src/features/media/InlineMedia.tsx @@ -1,8 +1,7 @@ -import { CSSProperties, useEffect } from "react"; +import { CSSProperties } from "react"; import Media, { MediaProps } from "#/features/media/Media"; import { cx } from "#/helpers/css"; -import useLatch from "#/helpers/useLatch"; import { useAppDispatch } from "#/store"; import { IMAGE_FAILED, imageFailed, imageLoaded } from "./imageSlice"; @@ -28,32 +27,7 @@ export default function InlineMedia({ ...props }: InlineMediaProps) { const dispatch = useAppDispatch(); - const [mediaRef, currentAspectRatio] = useMediaLoadObserver(src); - - /** - * Cross posts have different image thumbnail url when loaded, so prevent resizing by latching - * - * If the new image is different size (or errors), it will be properly updated then - * (IMAGE_FAILED is truthy) - */ - const aspectRatio = useLatch(currentAspectRatio); - - // Workaround. useMediaLoadObserver won't fire if a new image is loaded (in Safari) - // because we have already forced its dimensions (via latching aspect ratio) - // - // This causes issues like https://github.com/aeharding/voyager/issues/1770 - // - // For now, assume that a swapped image will have the same dimensions - // - // In the future, useMediaLoadObserver should render the image somewhere - // in the document root separate from the image rendered by InlineMedia - useEffect(() => { - // If we have a src, and the latched aspect ratio is being used, update the store - // (src has changed) - if (src && aspectRatio && !currentAspectRatio) { - dispatch(imageLoaded({ src, aspectRatio })); - } - }, [src, aspectRatio, currentAspectRatio, dispatch]); + const [mediaRef, aspectRatio] = useMediaLoadObserver(src); function buildPlaceholderState() { if (aspectRatio === IMAGE_FAILED) return "error"; @@ -91,7 +65,7 @@ export default function InlineMedia({ // TLDR Image loading should still work with this function commented out! onLoad={(event) => { if (!src) return; - if (isLoadedAspectRatio(currentAspectRatio)) return; + if (isLoadedAspectRatio(aspectRatio)) return; const dimensions = getTargetDimensions( event.target as HTMLImageElement, diff --git a/src/features/media/useMediaLoadObserver.ts b/src/features/media/useMediaLoadObserver.ts index 3b8631c5c4..a0c351c566 100644 --- a/src/features/media/useMediaLoadObserver.ts +++ b/src/features/media/useMediaLoadObserver.ts @@ -16,9 +16,15 @@ export default function useMediaLoadObserver(src: string | undefined) { let destroyed = false; function setupObserver() { - if (!src) return; + if (destroyed) return; + if (isLoadedAspectRatio(aspectRatio)) return; - if (!mediaRef.current) return; + if (!src) return; + if (!mediaRef.current) { + // react-reverse-portal refs can take some time to setup. Try again on next paint + requestAnimationFrame(setupObserver); + return; + } const handleResize = (entries: ResizeObserverEntry[]) => { for (const entry of entries) { @@ -39,33 +45,7 @@ export default function useMediaLoadObserver(src: string | undefined) { resizeObserverRef.current.observe(mediaRef.current); } - function setup() { - if (destroyed) return; - - if (!src) return; - if (isLoadedAspectRatio(aspectRatio)) return; - if (!mediaRef.current) { - // react-reverse-portal refs can take some time to setup. Try again on next paint - requestAnimationFrame(setup); - return; - } - - const dimensions = getTargetDimensions(mediaRef.current); - - if (dimensions) { - dispatch( - imageLoaded({ - src, - aspectRatio: dimensions.width / dimensions.height, - }), - ); - return; // found dimensions synchronously, no need to setup observer - } - - setupObserver(); - } - - setup(); + setupObserver(); return () => { destroyed = true; diff --git a/src/features/post/crosspost/Crosspost.tsx b/src/features/post/crosspost/Crosspost.tsx index 39700beb0a..50ddda4f96 100644 --- a/src/features/post/crosspost/Crosspost.tsx +++ b/src/features/post/crosspost/Crosspost.tsx @@ -1,16 +1,13 @@ -import { IonIcon, IonSkeletonText } from "@ionic/react"; -import { arrowUpSharp, chatbubbleOutline, repeat } from "ionicons/icons"; import { PostView } from "lemmy-js-client"; -import LargePostContents from "#/features/post/inFeed/large/LargePostContents"; import { cx } from "#/helpers/css"; -import { formatNumber } from "#/helpers/number"; import CrosspostContainer from "./CrosspostContainer"; +import CrosspostContents from "./CrosspostContents"; -import styles from "./Crosspost.module.css"; +import styles from "./CrosspostContents.module.css"; -interface CrosspostProps { +export interface CrosspostProps { post: PostView; url: string; className?: string; @@ -24,50 +21,11 @@ export default function Crosspost(props: CrosspostProps) { className={cx(styles.container, props.className)} > {({ crosspost, hasBeenRead }) => ( - <> - {crosspost ? ( -
- {crosspost.post.name} -
- ) : ( - - )} - -
- - {crosspost ? ( - crosspost.community.title - ) : ( - - )} -
- {" "} - {crosspost ? ( - formatNumber(crosspost.counts.score) - ) : ( - - )} -
-
- {" "} - {crosspost ? ( - formatNumber(crosspost.counts.comments) - ) : ( - - )} -
-
- + )} ); diff --git a/src/features/post/crosspost/Crosspost.module.css b/src/features/post/crosspost/CrosspostContents.module.css similarity index 100% rename from src/features/post/crosspost/Crosspost.module.css rename to src/features/post/crosspost/CrosspostContents.module.css diff --git a/src/features/post/crosspost/CrosspostContents.tsx b/src/features/post/crosspost/CrosspostContents.tsx new file mode 100644 index 0000000000..8a0aae2469 --- /dev/null +++ b/src/features/post/crosspost/CrosspostContents.tsx @@ -0,0 +1,93 @@ +import { IonIcon } from "@ionic/react"; +import { IonSkeletonText } from "@ionic/react"; +import { repeat } from "ionicons/icons"; +import { arrowUpSharp } from "ionicons/icons"; +import { chatbubbleOutline } from "ionicons/icons"; +import { PostView } from "lemmy-js-client"; +import { useEffect } from "react"; + +import { imageLoaded } from "#/features/media/imageSlice"; +import useAspectRatio from "#/features/media/useAspectRatio"; +import LargePostContents from "#/features/post/inFeed/large/LargePostContents"; +import usePostSrc from "#/features/post/inFeed/usePostSrc"; +import { cx } from "#/helpers/css"; +import { formatNumber } from "#/helpers/number"; +import { useAppDispatch } from "#/store"; + +import { CrosspostProps } from "./Crosspost"; + +import styles from "./CrosspostContents.module.css"; + +interface CrosspostContentsProps extends CrosspostProps { + crosspost: PostView | undefined; + hasBeenRead: boolean; +} + +export default function CrosspostContents({ + crosspost, + hasBeenRead, + post, +}: CrosspostContentsProps) { + const dispatch = useAppDispatch(); + const postAspectRatio = useAspectRatio(usePostSrc(post)); + const crosspostSrc = usePostSrc(crosspost); + const crosspostAspectRatio = useAspectRatio(crosspostSrc); + + // Workaround to immediately copy over the aspect ratio of the original image + // to avoid flickering when the new crosspost image src loads + useEffect(() => { + if (!crosspostSrc) return; + if (postAspectRatio && !crosspostAspectRatio) { + dispatch( + imageLoaded({ src: crosspostSrc, aspectRatio: postAspectRatio }), + ); + } + }, [crosspostSrc, postAspectRatio, crosspostAspectRatio, dispatch]); + + return ( + <> + {crosspost ? ( +
+ {crosspost.post.name} +
+ ) : ( + + )} + +
+ + {crosspost ? ( + crosspost.community.title + ) : ( + + )} +
+ {" "} + {crosspost ? ( + formatNumber(crosspost.counts.score) + ) : ( + + )} +
+
+ {" "} + {crosspost ? ( + formatNumber(crosspost.counts.comments) + ) : ( + + )} +
+
+ + ); +} diff --git a/src/features/post/inFeed/usePostSrc.ts b/src/features/post/inFeed/usePostSrc.ts index d279360936..cabe5451a7 100644 --- a/src/features/post/inFeed/usePostSrc.ts +++ b/src/features/post/inFeed/usePostSrc.ts @@ -6,7 +6,9 @@ import { findUrlMediaType } from "#/helpers/url"; import useSupported from "#/helpers/useSupported"; import { useAppSelector } from "#/store"; -export default function usePostSrc(post: PostView): string | undefined { +export default function usePostSrc( + post: PostView | undefined, +): string | undefined { const thumbnailIsFullsize = useSupported("Fullsize thumbnails"); const src = getPostMedia(post, thumbnailIsFullsize); @@ -22,9 +24,11 @@ export default function usePostSrc(post: PostView): string | undefined { } function getPostMedia( - post: PostView, + post: PostView | undefined, thumbnailIsFullsize: boolean, ): [string] | [string, string] | undefined { + if (!post) return; + if (post.post.url) { const isUrlMedia = findUrlMediaType( post.post.url, diff --git a/src/helpers/useLatch.ts b/src/helpers/useLatch.ts deleted file mode 100644 index 148f2ece91..0000000000 --- a/src/helpers/useLatch.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { useEffect, useState } from "react"; - -export default function useLatch(value: V): V { - const [latchedValue, setLatchedValue] = useState(value); - - useEffect(() => { - if (!value) return; - - setLatchedValue(value); - }, [value]); - - return latchedValue; -} From d2a6a14f402f59cd37cd75153e4cda46c8f6d675 Mon Sep 17 00:00:00 2001 From: Alexander Harding Date: Wed, 11 Dec 2024 20:57:27 -0600 Subject: [PATCH 3/3] cleanup --- .../post/crosspost/CrosspostContents.tsx | 22 ++------------ .../useCopyPostAspectRatioIfNeeded.ts | 30 +++++++++++++++++++ 2 files changed, 32 insertions(+), 20 deletions(-) create mode 100644 src/features/post/crosspost/useCopyPostAspectRatioIfNeeded.ts diff --git a/src/features/post/crosspost/CrosspostContents.tsx b/src/features/post/crosspost/CrosspostContents.tsx index 8a0aae2469..29e08b343f 100644 --- a/src/features/post/crosspost/CrosspostContents.tsx +++ b/src/features/post/crosspost/CrosspostContents.tsx @@ -4,17 +4,13 @@ import { repeat } from "ionicons/icons"; import { arrowUpSharp } from "ionicons/icons"; import { chatbubbleOutline } from "ionicons/icons"; import { PostView } from "lemmy-js-client"; -import { useEffect } from "react"; -import { imageLoaded } from "#/features/media/imageSlice"; -import useAspectRatio from "#/features/media/useAspectRatio"; import LargePostContents from "#/features/post/inFeed/large/LargePostContents"; -import usePostSrc from "#/features/post/inFeed/usePostSrc"; import { cx } from "#/helpers/css"; import { formatNumber } from "#/helpers/number"; -import { useAppDispatch } from "#/store"; import { CrosspostProps } from "./Crosspost"; +import { useCopyPostAspectRatioIfNeeded } from "./useCopyPostAspectRatioIfNeeded"; import styles from "./CrosspostContents.module.css"; @@ -28,21 +24,7 @@ export default function CrosspostContents({ hasBeenRead, post, }: CrosspostContentsProps) { - const dispatch = useAppDispatch(); - const postAspectRatio = useAspectRatio(usePostSrc(post)); - const crosspostSrc = usePostSrc(crosspost); - const crosspostAspectRatio = useAspectRatio(crosspostSrc); - - // Workaround to immediately copy over the aspect ratio of the original image - // to avoid flickering when the new crosspost image src loads - useEffect(() => { - if (!crosspostSrc) return; - if (postAspectRatio && !crosspostAspectRatio) { - dispatch( - imageLoaded({ src: crosspostSrc, aspectRatio: postAspectRatio }), - ); - } - }, [crosspostSrc, postAspectRatio, crosspostAspectRatio, dispatch]); + useCopyPostAspectRatioIfNeeded(post, crosspost); return ( <> diff --git a/src/features/post/crosspost/useCopyPostAspectRatioIfNeeded.ts b/src/features/post/crosspost/useCopyPostAspectRatioIfNeeded.ts new file mode 100644 index 0000000000..eb5b4a9bd3 --- /dev/null +++ b/src/features/post/crosspost/useCopyPostAspectRatioIfNeeded.ts @@ -0,0 +1,30 @@ +import { PostView } from "lemmy-js-client"; +import { useEffect } from "react"; + +import { imageLoaded } from "#/features/media/imageSlice"; +import useAspectRatio from "#/features/media/useAspectRatio"; +import usePostSrc from "#/features/post/inFeed/usePostSrc"; +import { useAppDispatch } from "#/store"; + +/** + * Workaround to immediately copy over the aspect ratio of the original image + * to avoid flickering when the new crosspost image src loads + */ +export function useCopyPostAspectRatioIfNeeded( + post: PostView | undefined, + crosspost: PostView | undefined, +) { + const dispatch = useAppDispatch(); + const postAspectRatio = useAspectRatio(usePostSrc(post)); + const crosspostSrc = usePostSrc(crosspost); + const crosspostAspectRatio = useAspectRatio(crosspostSrc); + + useEffect(() => { + if (!crosspostSrc) return; + if (postAspectRatio && !crosspostAspectRatio) { + dispatch( + imageLoaded({ src: crosspostSrc, aspectRatio: postAspectRatio }), + ); + } + }, [crosspostSrc, postAspectRatio, crosspostAspectRatio, dispatch]); +}