Conversation
|
📝 WalkthroughWalkthroughThis PR implements media preload behavior for the Memes Quick Vote dialog by introducing a Changes
Sequence DiagramsequenceDiagram
participant User
participant Dialog as MemesQuickVoteDialog
participant Queue as useMemesQuickVoteQueue
participant PreviewStack as MemesQuickVotePreviewStack
participant ActivePreview as Preview<br/>(renderMode: active)
participant PreloadedPreview as Preview<br/>(renderMode: preloaded)
participant Media as Media<br/>Components
User->>Dialog: Open dialog with activeDrop
Dialog->>Queue: Request nextDrop from lookahead
Queue-->>Dialog: Return nextDrop (if voteable & not GLB)
Dialog->>PreviewStack: Render with activeDrop & nextDrop
PreviewStack->>ActivePreview: Render active item<br/>(interactive, loadStrategy: in-view)
ActivePreview->>Media: Load media on viewport
Media-->>ActivePreview: Display interactive preview
PreviewStack->>PreloadedPreview: Render next item hidden<br/>(non-interactive, loadStrategy: eager)
PreloadedPreview->>Media: Load media eagerly
Media-->>PreloadedPreview: Preload next preview
User->>Dialog: Vote or skip activeDrop
Dialog->>Dialog: Update to next drop
PreviewStack->>ActivePreview: Swap to preloaded preview<br/>(now interactive)
PreloadedPreview->>PreloadedPreview: Unmount/hidden
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
components/drops/view/item/content/media/MediaDisplay.tsx (1)
177-186: Avoid returningnullfor eager HTML when no preview image exists.If
loadStrategy === "eager"andpreviewImageUrlis absent, this path currently blanks media entirely. Prefer falling through to existing HTML handling so content still renders.Suggested patch
if (mediaType === MediaType.HTML) { - if (loadStrategy === "eager") { - return previewImageUrl ? ( - <MediaDisplayImage - src={previewImageUrl} - imageScale={imageScale} - loadStrategy={loadStrategy} - /> - ) : null; + if (loadStrategy === "eager" && previewImageUrl) { + return ( + <MediaDisplayImage + src={previewImageUrl} + imageScale={imageScale} + loadStrategy={loadStrategy} + /> + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/item/content/media/MediaDisplay.tsx` around lines 177 - 186, The eager-HTML branch in MediaDisplay currently returns null when previewImageUrl is missing, causing HTML media to be blank; change the logic in the MediaDisplay component so that when mediaType === MediaType.HTML and loadStrategy === "eager" you only render <MediaDisplayImage> if previewImageUrl exists, but do not return null when it doesn't—instead let execution fall through to the existing HTML rendering path (the same path used for non-eager HTML) so the HTML content still renders; locate the conditional handling around mediaType === MediaType.HTML, loadStrategy, previewImageUrl and MediaDisplayImage and remove or refactor the early `return null` so fall-through occurs.components/brain/left-sidebar/waves/memes-quick-vote/MemesQuickVoteDialog.tsx (1)
47-65: Reuse the shared GLB classifier for this preload guard.This re-implements the same GLB/GLTF detection already used in
components/drops/view/item/content/media/DropListItemContentMedia.tsxLines 42-70. If those rules drift, quick vote can start eagerly mounting a model that the renderer itself still classifies as GLB, which is exactly the path this guard is meant to block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/left-sidebar/waves/memes-quick-vote/MemesQuickVoteDialog.tsx` around lines 47 - 65, The getQuickVotePreloadedNextDrop function reimplements GLB/GLTF detection; instead import and call the shared GLB classifier used by DropListItemContentMedia (the classifier exported from components/drops/view/item/content/media/DropListItemContentMedia.tsx) rather than duplicating mime/url checks—update getQuickVotePreloadedNextDrop to return null when that shared classifier reports the media is a GLB/GLTF and return nextDrop otherwise, referencing getQuickVotePreloadedNextDrop and the shared classifier name when you change the import and conditional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@__tests__/components/brain/left-sidebar/waves/memes-quick-vote/MemesQuickVoteDialog.test.tsx`:
- Around line 116-135: Tests still instantiate MemesQuickVoteDialog only via
isOpen/sessionId/onClose or by toggling useIsMobileScreenMock, but the component
now reads queue state from props and derives mobile mode via useMediaQuery;
update tests to use createDialogProps(...) to supply full prop-driven state
(e.g., activeDrop, sessionId, uncastPower, submitVote, etc.) and stop using
useIsMobileScreenMock, instead mock matchMedia/useMediaQuery to control mobile
vs desktop behavior; locate uses of MemesQuickVoteDialog in tests and replace
direct prop sets with createDialogProps(overrides) and adjust media mocks to
reflect mobile or not so assertions exercise the proper code paths.
- Line 130: The test mock for skipDrop is synchronous but the component
(MemesQuickVoteDialog.tsx) calls skipDrop(activeDrop).then(...), so replace the
bare mock skipDrop: jest.fn() with an async-resolving mock (e.g., use
jest.fn().mockResolvedValue(...) or a function returning Promise.resolve()) so
calls to skipDrop(activeDrop).then(...) work without throwing; update the mock
declaration named skipDrop in MemesQuickVoteDialog.test.tsx accordingly.
---
Nitpick comments:
In
`@components/brain/left-sidebar/waves/memes-quick-vote/MemesQuickVoteDialog.tsx`:
- Around line 47-65: The getQuickVotePreloadedNextDrop function reimplements
GLB/GLTF detection; instead import and call the shared GLB classifier used by
DropListItemContentMedia (the classifier exported from
components/drops/view/item/content/media/DropListItemContentMedia.tsx) rather
than duplicating mime/url checks—update getQuickVotePreloadedNextDrop to return
null when that shared classifier reports the media is a GLB/GLTF and return
nextDrop otherwise, referencing getQuickVotePreloadedNextDrop and the shared
classifier name when you change the import and conditional.
In `@components/drops/view/item/content/media/MediaDisplay.tsx`:
- Around line 177-186: The eager-HTML branch in MediaDisplay currently returns
null when previewImageUrl is missing, causing HTML media to be blank; change the
logic in the MediaDisplay component so that when mediaType === MediaType.HTML
and loadStrategy === "eager" you only render <MediaDisplayImage> if
previewImageUrl exists, but do not return null when it doesn't—instead let
execution fall through to the existing HTML rendering path (the same path used
for non-eager HTML) so the HTML content still renders; locate the conditional
handling around mediaType === MediaType.HTML, loadStrategy, previewImageUrl and
MediaDisplayImage and remove or refactor the early `return null` so fall-through
occurs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3fe8cbfb-7910-4b50-97d3-3a3a85598052
📒 Files selected for processing (11)
__tests__/components/brain/left-sidebar/waves/memes-quick-vote/MemesQuickVoteDialog.test.tsxcomponents/brain/left-sidebar/waves/memes-quick-vote/MemesQuickVoteDescription.tsxcomponents/brain/left-sidebar/waves/memes-quick-vote/MemesQuickVoteDialog.tsxcomponents/brain/left-sidebar/waves/memes-quick-vote/MemesQuickVotePreview.tsxcomponents/drops/view/item/content/media/DropListItemContentMedia.tsxcomponents/drops/view/item/content/media/DropListItemContentMediaImage.tsxcomponents/drops/view/item/content/media/MediaDisplay.tsxcomponents/drops/view/item/content/media/MediaDisplayImage.tsxcomponents/drops/view/item/content/media/mediaLoadStrategy.tshooks/useMemesQuickVoteDialogController.tshooks/useMemesQuickVoteQueue.ts
ragnep
approved these changes
Mar 30, 2026
This was referenced Apr 2, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Performance