fix(embed): restore youtube thumbnails and file-row labels#1148
Merged
Conversation
Exclude the flash tag dropdown from themed post-form select styling so it renders with the browser's default select appearance, matching the flag selector.
Extract embedded IPv6 addresses from dns6 multiaddrs so geo lookup and country flags work for relay peers that publish IPv6 via DNS hostnames.
Country-only boards auto-publish geographic location flags without showing a flag dropdown, matching existing /bant/ behavior.
Restore thumbnail-first previews for YouTube embeds in post media and markdown hover. Desktop posts show File with the thumbnail image URL and a youtube video type label.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (40)
✅ Files skipped from review due to trivial changes (15)
🚧 Files skipped from review as they are similar to previous changes (20)
📝 WalkthroughWalkthroughThis PR centralizes YouTube video ID parsing, adds YouTube thumbnail/link helpers and floating-preview thumbnail behavior, expands auto-geographic-flag boards (bant → bant,int,sp), adds DNS6 embedded-IPv6 extraction for peer geolocation, refactors comment reason rendering, updates tests/mocks, and adds ChangesYouTube Media & Comment Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (4)
src/components/comment-content/comment-content.tsx (1)
230-236: 💤 Low valueOptional: Remove explicit
nullreturn.The explicit
nullon line 236 can be omitted since the ternary expression without an else clause returnsundefinedimplicitly, or you can remove the else branch entirely. Both are equivalent for React rendering.♻️ Proposed simplification
- {pendingApprovalReasonMessage ? ( + {pendingApprovalReasonMessage && ( <> <br /> <br /> {renderContent(pendingApprovalReasonMessage)} </> - ) : null} + )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/comment-content/comment-content.tsx` around lines 230 - 236, The JSX fragment renders pendingApprovalReasonMessage with a ternary that returns null in the else branch; simplify by removing the explicit null and either use a short-circuit expression (pendingApprovalReasonMessage && (<><br /><br />{renderContent(pendingApprovalReasonMessage)}</>)) or drop the else branch so the ternary returns undefined implicitly—update the JSX around pendingApprovalReasonMessage in the CommentContent component (where renderContent is called) to remove the explicit null return.src/components/comment-content/__tests__/comment-content.test.tsx (1)
69-71: 💤 Low valueRemove unused mock translation key.
The
'reason_reason'translation key mock is not used in any test case and can be removed for cleaner test setup.♻️ Proposed cleanup
useTranslation: () => ({ t: (key: string, options?: Record<string, unknown>) => { - if (key === 'reason_reason') { - return `reason:${options?.reason}`; - } if (key === 'pending_mod_approval_reason') { return `pending-reason:${options?.reason}`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/comment-content/__tests__/comment-content.test.tsx` around lines 69 - 71, Remove the unused mock translation key by deleting the conditional branch that checks for "reason_reason" (the if (key === 'reason_reason') { return `reason:${options?.reason}`; } block) from the test i18n/mock translator in comment-content.test.tsx, and run the tests to confirm nothing else depends on 'reason_reason'; if similar unused branches exist, remove them as well to keep the mock translation function minimal.src/components/markdown/markdown.tsx (1)
126-127: 💤 Low valueFloating embed behavior now forces thumbnails for all media types.
The change to
showThumbnail={true}andsetShowThumbnail={() => {}}prevents autoplay in floating hover previews, which makes sense for YouTube. However, this affects all embeddable media types (Streamable, etc.), not just YouTube. If this broader impact is intentional to improve hover preview UX across all media types, consider documenting this behavior change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/markdown/markdown.tsx` around lines 126 - 127, The change forcing thumbnails by setting showThumbnail={true} and replacing setShowThumbnail with a no-op causes all embeds to use static thumbnails (preventing autoplay) rather than only YouTube; update the floating/embed logic to conditionally enable thumbnail-only behavior for specific providers instead of globally: locate the props passed (showThumbnail and setShowThumbnail) in the Markdown embed/floating preview component (e.g., the component rendering embeds or the FloatingEmbed wrapper) and change the boolean/no-op to be provider-aware (check embed provider/type like "youtube" vs others) so only YouTube (or configured providers) gets showThumbnail={true} and a disabled setShowThumbnail, while other providers retain their original behavior or are configurable; add a brief comment documenting this provider-specific decision.src/components/post-form/post-form.tsx (1)
394-394: 💤 Low valueConsider moving styling change to separate commit.
The addition of
className={styles.flagSelector}to the flash tag selector creates visual consistency with the flag selector, but this styling change is not mentioned in the layer description ("Auto-publish geographic location for additional boards") or PR objectives. While the change itself is fine, it appears to be outside the scope of this layer's stated purpose.Consider whether this should be:
- Documented in the PR description as an additional improvement
- Split into a separate commit for clarity
- Left as-is if it's needed for some non-obvious reason
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/post-form/post-form.tsx` at line 394, The change adds className={styles.flagSelector} to the flash tag <select> in the PostForm component (select name='flashTag', ref={flashTagRef}) which is unrelated to the layer's stated purpose; either move this styling change into its own commit or explicitly document it in the PR description as an additional UI improvement—revert the className addition in this commit if you split it, or add a note to the PR and keep the change if you choose to include it here.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@public/translations/ar/default.json`:
- Line 403: The translation key "youtube_video" is left as the English
placeholder in all non-English locale JSONs (ar, bn, cs, da, id, it, ja, ko);
update each locale's value for "youtube_video" to the correct target-language
string (e.g., Arabic: "فيديو يوتيوب", Bengali: "ইউটিউব ভিডিও", Czech: "video
youtube" or leave as-is, Danish: "youtube-video", Indonesian: "video youtube",
Italian: "video youtube", Japanese: "YouTubeビデオ" or "YouTube動画", Korean: "유튜브
동영상"), preserving JSON quoting and key names exactly and ensuring no trailing
commas or formatting errors.
In `@public/translations/de/default.json`:
- Line 403: The translation key "youtube_video" currently has an English value;
update its value in public/translations/de/default.json to a German localized
string such as "YouTube‑Video" (or "youtube‑video" if you prefer
lowercase/hyphenation) so the key youtube_video is properly translated into
German.
In `@public/translations/el/default.json`:
- Line 403: The translation for the key "youtube_video" is still in English;
update its value to a Greek translation such as "βίντεο YouTube" (or "βίντεο
youtube") by replacing "youtube video" with the Greek text for the
"youtube_video" key, and verify the phrasing with a native Greek speaker if
needed.
In `@public/translations/es/default.json`:
- Line 403: Update the translation value for the "youtube_video" key in the
Spanish translations JSON so it's localized (currently "youtube video"); replace
it with the Spanish string (e.g., "vídeo de youtube" or "video de youtube")
while keeping the JSON key intact and preserving proper quotes and punctuation.
In `@public/translations/fa/default.json`:
- Line 403: The translation key "youtube_video" in
public/translations/fa/default.json is left in English and should be localized;
replace the value "youtube video" with the Persian translation (e.g., "ویدئوی
یوتیوب" or "ویدیوی یوتیوب") so the UI label for youtube_video is displayed in
Persian; update the JSON value for the "youtube_video" key accordingly while
preserving JSON formatting.
In `@public/translations/mr/default.json`:
- Line 403: The translation key "youtube_video" currently contains the English
string "youtube video"; update its value in the Marathi translations file so it
is localized (for example set "youtube_video" to "youtube व्हिडिओ" or an
equivalent Marathi phrase), ensuring the JSON string is properly quoted and
escaped if needed and matches the style of other entries in the file.
In `@public/translations/nl/default.json`:
- Line 403: The translation for the key "youtube_video" is still in English;
update its value in the JSON (key "youtube_video") to a Dutch translation such
as "youtube-video" (or "youtube video" if you prefer spacing) so the
nl/default.json entry is localized to Dutch.
In `@public/translations/no/default.json`:
- Line 403: The translation for the key "youtube_video" is currently in English;
update the value for the "youtube_video" key in the Norwegian translations file
to an appropriate Norwegian string (e.g., "youtube-video" or another
Norwegian-localized equivalent) so the key is properly localized in the file.
In `@public/translations/pl/default.json`:
- Line 403: The key "youtube_video" in the Polish translations is still English;
update its value under "youtube_video" in public/translations/pl/default.json to
a proper Polish translation such as "wideo YouTube" or "film YouTube" (matching
project capitalization/term conventions) so Polish users see a localized label.
In `@public/translations/pt/default.json`:
- Line 403: The translation key "youtube_video" currently has an English value
("youtube video"); update its value in the Portuguese locale to a proper
Portuguese phrase such as "vídeo do YouTube" (or "vídeo YouTube"/"vídeo do
youtube" if you prefer lowercase branding) by replacing the value for the
"youtube_video" key in public/translations/pt/default.json so Portuguese users
see the translated string.
In `@public/translations/ro/default.json`:
- Line 403: The translation for the key "youtube_video" is still in English;
update the value for the "youtube_video" entry in
public/translations/ro/default.json to a Romanian translation such as "video
YouTube" (or "video youtube") so Romanian users see the localized text for the
youtube_video key.
In `@public/translations/ru/default.json`:
- Line 403: The translation for the key "youtube_video" is still in English;
update the value for "youtube_video" in the locale JSON to an appropriate
Russian translation (e.g., "видео YouTube" or "видео youtube") so Russian users
see localized text, preserving the JSON string format and surrounding quotes.
In `@public/translations/sq/default.json`:
- Line 403: The translation key "youtube_video" in
public/translations/sq/default.json currently has an English value ("youtube
video"); update its value to the correct Albanian translation (for example
"video YouTube" or "video në YouTube") so the Albanian locale displays the
proper localized string — modify the JSON entry for "youtube_video" accordingly.
In `@public/translations/sv/default.json`:
- Line 403: The translation key "youtube_video" currently has an English value
("youtube video") in the Swedish locale; update the value to a proper Swedish
translation (e.g., "youtube-video" or another preferred Swedish phrasing) by
replacing the string for the "youtube_video" key in the JSON (key:
"youtube_video") so Swedish users see the correct localized text.
In `@public/translations/te/default.json`:
- Line 403: The translation for the key "youtube_video" in
public/translations/te/default.json is currently in English ("youtube video");
update the value to Telugu by replacing the string for the "youtube_video" key
with the appropriate Telugu translation (e.g., "యూట్యూబ్ వీడియో") so the Telugu
locale shows localized text.
In `@public/translations/th/default.json`:
- Line 403: The "youtube_video" localization entry currently contains English
text; update the value for the "youtube_video" key to the Thai translation (for
example "วิดีโอ YouTube") in the JSON so the Thai locale shows proper localized
text, preserving the JSON string quoting and UTF-8 encoding and keeping the key
name "youtube_video" unchanged.
In `@public/translations/tr/default.json`:
- Line 403: The "youtube_video" translation key currently contains English text;
update its value to the Turkish translation by replacing "youtube video" with
"youtube videosu" in the translations JSON so the "youtube_video" key returns
the correct localized string.
In `@public/translations/uk/default.json`:
- Line 403: Replace the English value for the i18n key "youtube_video" in the
translations JSON so the Ukrainian localization is used; update the entry for
"youtube_video" from "youtube video" to the correct Ukrainian string (e.g.,
"youtube відео") in the translations object to maintain i18n consistency.
In `@public/translations/ur/default.json`:
- Line 403: The translation for the key "youtube_video" in
public/translations/ur/default.json is still in English; replace the value
"youtube video" with the correct Urdu localization (for example "یوٹیوب ویڈیو")
so the "youtube_video" key returns Urdu text for Urdu users.
---
Nitpick comments:
In `@src/components/comment-content/__tests__/comment-content.test.tsx`:
- Around line 69-71: Remove the unused mock translation key by deleting the
conditional branch that checks for "reason_reason" (the if (key ===
'reason_reason') { return `reason:${options?.reason}`; } block) from the test
i18n/mock translator in comment-content.test.tsx, and run the tests to confirm
nothing else depends on 'reason_reason'; if similar unused branches exist,
remove them as well to keep the mock translation function minimal.
In `@src/components/comment-content/comment-content.tsx`:
- Around line 230-236: The JSX fragment renders pendingApprovalReasonMessage
with a ternary that returns null in the else branch; simplify by removing the
explicit null and either use a short-circuit expression
(pendingApprovalReasonMessage && (<><br /><br
/>{renderContent(pendingApprovalReasonMessage)}</>)) or drop the else branch so
the ternary returns undefined implicitly—update the JSX around
pendingApprovalReasonMessage in the CommentContent component (where
renderContent is called) to remove the explicit null return.
In `@src/components/markdown/markdown.tsx`:
- Around line 126-127: The change forcing thumbnails by setting
showThumbnail={true} and replacing setShowThumbnail with a no-op causes all
embeds to use static thumbnails (preventing autoplay) rather than only YouTube;
update the floating/embed logic to conditionally enable thumbnail-only behavior
for specific providers instead of globally: locate the props passed
(showThumbnail and setShowThumbnail) in the Markdown embed/floating preview
component (e.g., the component rendering embeds or the FloatingEmbed wrapper)
and change the boolean/no-op to be provider-aware (check embed provider/type
like "youtube" vs others) so only YouTube (or configured providers) gets
showThumbnail={true} and a disabled setShowThumbnail, while other providers
retain their original behavior or are configurable; add a brief comment
documenting this provider-specific decision.
In `@src/components/post-form/post-form.tsx`:
- Line 394: The change adds className={styles.flagSelector} to the flash tag
<select> in the PostForm component (select name='flashTag', ref={flashTagRef})
which is unrelated to the layer's stated purpose; either move this styling
change into its own commit or explicitly document it in the PR description as an
additional UI improvement—revert the className addition in this commit if you
split it, or add a note to the PR and keep the change if you choose to include
it here.
🪄 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: 101f575b-0d18-4180-bd15-a420fe79a509
📒 Files selected for processing (55)
public/translations/ar/default.jsonpublic/translations/bn/default.jsonpublic/translations/cs/default.jsonpublic/translations/da/default.jsonpublic/translations/de/default.jsonpublic/translations/el/default.jsonpublic/translations/en/default.jsonpublic/translations/es/default.jsonpublic/translations/fa/default.jsonpublic/translations/fi/default.jsonpublic/translations/fil/default.jsonpublic/translations/fr/default.jsonpublic/translations/he/default.jsonpublic/translations/hi/default.jsonpublic/translations/hu/default.jsonpublic/translations/id/default.jsonpublic/translations/it/default.jsonpublic/translations/ja/default.jsonpublic/translations/ko/default.jsonpublic/translations/mr/default.jsonpublic/translations/nl/default.jsonpublic/translations/no/default.jsonpublic/translations/pl/default.jsonpublic/translations/pt/default.jsonpublic/translations/ro/default.jsonpublic/translations/ru/default.jsonpublic/translations/sq/default.jsonpublic/translations/sv/default.jsonpublic/translations/te/default.jsonpublic/translations/th/default.jsonpublic/translations/tr/default.jsonpublic/translations/uk/default.jsonpublic/translations/ur/default.jsonpublic/translations/vi/default.jsonpublic/translations/zh/default.jsonsrc/components/comment-content/__tests__/comment-content.test.tsxsrc/components/comment-content/comment-content.tsxsrc/components/comment-media/__tests__/comment-media.test.tsxsrc/components/embed/__tests__/embed.test.tsxsrc/components/embed/embed-utils.tssrc/components/embed/embed.tsxsrc/components/embed/index.tssrc/components/markdown/__tests__/markdown.test.tsxsrc/components/markdown/markdown.tsxsrc/components/post-desktop/post-desktop.tsxsrc/components/post-form/__tests__/post-form.test.tsxsrc/components/post-form/post-form.tsxsrc/components/reply-modal/__tests__/reply-modal.test.tsxsrc/components/settings-modal/p2p-stats-settings/__tests__/p2p-stats-settings.test.tsxsrc/lib/__tests__/comment-flag-selection.test.tssrc/lib/__tests__/peer-geo.test.tssrc/lib/comment-flag-selection.tssrc/lib/peer-geo.tssrc/lib/utils/__tests__/media-utils.test.tssrc/lib/utils/media-utils.ts
Translate the youtube video label, handle mobile/music YouTube hosts as standard YouTube URLs, keep affected mocks current, and cap default Vitest workers to reduce local CPU spikes.
Member
Author
|
Addressed the valid review feedback in effe16f. Fixed:
Triaged as non-blocking:
Local verification run: |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit effe16f. Configure here.
| const mediaDimensions = getMediaDimensions(commentMediaInfo); | ||
| const directoryEntry = findDirectoryByAddress(directories, communityAddress); | ||
| const requirePostLinkIsMedia = directoryEntry?.features?.requirePostLinkIsMedia === true; | ||
| const fileLabel = youtubeFileLink || requirePostLinkIsMedia ? t('file') : t('link'); |
There was a problem hiding this comment.
Wrong file row label
Medium Severity
For YouTube OP links, the file row prefix is the thumbnail URL string instead of the translated “File” label, because fileLabel is parsed as youtubeFileLink || (requirePostLinkIsMedia ? … : …) rather than grouping the two conditions before the ternary.
Reviewed by Cursor Bugbot for commit effe16f. Configure here.
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
1 participant
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
img.youtube.comthumb again, not an autoplaying iframe).embed-utilsso thumbnails work for standard and Invidious-style URLs.PostMediashows File with the thumbnail image URL and type youtube video instead of Link + iframe.Test plan
youtube video, thumbnail visible, click expands embed.yarn test,yarn build,yarn lint,yarn type-check(run in CI)Note
Low Risk
Mostly UI, media parsing, and i18n with broad test coverage; no auth or data-model changes.
Overview
This PR restores thumbnail-first YouTube handling in posts and link hovers, and tightens related parsing and UI copy.
YouTube embeds and media —
getYouTubeVideoIdis centralized inembed-utils(watch, youtu.be, shorts, mobile/music hosts, Invidious). Embeds andmedia-utilsuse it forimg.youtube.comthumbnails. Desktop PostMedia shows File, the thumbnail URL, and label youtube video instead of Link/iframe. Markdown floating link previews keep thumbnails (showThumbnail: true) instead of loading an iframe on hover.Moderation / display — Removal, delete, and pending-mod reason text is rendered through markdown (no extra quotes); tests updated.
Board flags —
/int/and/sp/join/bant/for auto geographic location without a visible flag selector; post/reply tests expanded.P2P geo — IPv6 can be parsed from DNS6 hostnames with dash-encoded addresses (peer map flags in settings).
Minor:
vitest --maxWorkers=4,youtube_videoi18n, flash tag select styling.Reviewed by Cursor Bugbot for commit effe16f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor