From 97cbfec38e73b77159a570360a67ca03f151c5a0 Mon Sep 17 00:00:00 2001
From: Edward Kung <edward.kung@gmail.com>
Date: Tue, 12 Aug 2025 16:30:46 -0700
Subject: [PATCH] Order carousel as images appear in items / markdown (#2239)

* carousel sort in deterministic order

* imgIndex 0 for ItemEmbed

* fix order for item-full

* fix indexing in ItemEmbed

* Revert "fix indexing in ItemEmbed"

This reverts commit f7863af30a1a02b189bfc79237606851c4da1abf.

* Revert "fix order for item-full"

This reverts commit 489e25ea82056bd83a818e581eb2bbfcf947e401.

* Revert "imgIndex 0 for ItemEmbed"

This reverts commit cd5fff1bae151e44db717f2a2173f673793bc6d0.

* carousel preserves ordering rendered on screen

* reorder carousel when sort changes

* fix cursor detected bugs

* register media to carousel before image load, confirm afterwards

* Remove unnecessary ref from dependencies

* Add missing dependencies

* Add missing dependencies

* Check if src was found in Carousel

---------

Co-authored-by: ekzyis <ek@stacker.news>
---
 components/carousel.js      | 24 ++++++++++++++++++++----
 components/item-full.js     |  6 +++++-
 components/media-or-link.js | 13 ++++++++++---
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/components/carousel.js b/components/carousel.js
index 3d69152e..e136e126 100644
--- a/components/carousel.js
+++ b/components/carousel.js
@@ -56,8 +56,9 @@ function useArrowKeys ({ moveLeft, moveRight }) {
 function Carousel ({ close, mediaArr, src, setOptions }) {
   const [index, setIndex] = useState(mediaArr.findIndex(([key]) => key === src))
   const [currentSrc, canGoLeft, canGoRight] = useMemo(() => {
+    if (index === -1) return [src, false, false]
     return [mediaArr[index][0], index > 0, index < mediaArr.length - 1]
-  }, [mediaArr, index])
+  }, [src, mediaArr, index])
 
   useEffect(() => {
     if (index === -1) return
@@ -115,8 +116,12 @@ export function CarouselProvider ({ children }) {
   const showModal = useShowModal()
 
   const showCarousel = useCallback(({ src }) => {
+    // only show confirmed entries
+    const confirmedEntries = Array.from(media.current.entries())
+      .filter(([, entry]) => entry.confirmed)
+
     showModal((close, setOptions) => {
-      return <Carousel close={close} mediaArr={Array.from(media.current.entries())} src={src} setOptions={setOptions} />
+      return <Carousel close={close} mediaArr={confirmedEntries} src={src} setOptions={setOptions} />
     }, {
       fullScreen: true,
       overflow: <CarouselOverflow {...media.current.get(src)} />
@@ -124,14 +129,25 @@ export function CarouselProvider ({ children }) {
   }, [showModal])
 
   const addMedia = useCallback(({ src, originalSrc, rel }) => {
-    media.current.set(src, { src, originalSrc, rel })
+    media.current.set(src, { src, originalSrc, rel, confirmed: false })
+  }, [])
+
+  const confirmMedia = useCallback((src) => {
+    const mediaItem = media.current.get(src)
+    if (mediaItem) {
+      mediaItem.confirmed = true
+      media.current.set(src, mediaItem)
+    }
   }, [])
 
   const removeMedia = useCallback((src) => {
     media.current.delete(src)
   }, [])
 
-  const value = useMemo(() => ({ showCarousel, addMedia, removeMedia }), [showCarousel, addMedia, removeMedia])
+  const value = useMemo(
+    () => ({ showCarousel, addMedia, confirmMedia, removeMedia }),
+    [showCarousel, addMedia, confirmMedia, removeMedia]
+  )
   return <CarouselContext.Provider value={value}>{children}</CarouselContext.Provider>
 }
 
diff --git a/components/item-full.js b/components/item-full.js
index 8150bfcb..8e8eb3f0 100644
--- a/components/item-full.js
+++ b/components/item-full.js
@@ -26,6 +26,7 @@ import { UNKNOWN_LINK_REL } from '@/lib/constants'
 import classNames from 'classnames'
 import { CarouselProvider } from './carousel'
 import Embed from './embed'
+import { useRouter } from 'next/router'
 
 function BioItem ({ item, handleClick }) {
   const { me } = useMe()
@@ -165,6 +166,9 @@ export default function ItemFull ({ item, fetchMoreComments, bio, rank, ...props
     commentsViewed(item)
   }, [item.lastCommentAt])
 
+  const router = useRouter()
+  const carouselKey = `${item.id}-${router.query?.sort || 'default'}`
+
   return (
     <>
       {rank
@@ -174,7 +178,7 @@ export default function ItemFull ({ item, fetchMoreComments, bio, rank, ...props
           </div>)
         : <div />}
       <RootProvider root={item.root || item}>
-        <CarouselProvider key={item.id}>
+        <CarouselProvider key={carouselKey}>
           {item.parentId
             ? <Comment topLevel item={item} replyOpen includeParent noComments {...props} />
             : (
diff --git a/components/media-or-link.js b/components/media-or-link.js
index 9569fbc1..b05d9d00 100644
--- a/components/media-or-link.js
+++ b/components/media-or-link.js
@@ -75,12 +75,19 @@ const Media = memo(function Media ({
 export default function MediaOrLink ({ linkFallback = true, ...props }) {
   const media = useMediaHelper(props)
   const [error, setError] = useState(false)
-  const { showCarousel, addMedia, removeMedia } = useCarousel()
+  const { showCarousel, addMedia, confirmMedia, removeMedia } = useCarousel()
 
+  // register placeholder immediately on mount if we have a src
+  useEffect(() => {
+    if (!media.bestResSrc) return
+    addMedia({ src: media.bestResSrc, originalSrc: media.originalSrc, rel: props.rel })
+  }, [addMedia, media.bestResSrc, media.originalSrc, props.rel])
+
+  // confirm media for carousel based on image detection
   useEffect(() => {
     if (!media.image) return
-    addMedia({ src: media.bestResSrc, originalSrc: media.originalSrc, rel: props.rel })
-  }, [media.image])
+    confirmMedia(media.bestResSrc)
+  }, [confirmMedia, media.image, media.bestResSrc])
 
   const handleClick = useCallback(() => showCarousel({ src: media.bestResSrc }),
     [showCarousel, media.bestResSrc])