Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThreads post image metadata and render options into the renderer, injects 3Speak thumbnails and supports direct iframe embeds with orientation handling, adds a HiveSigner callback URL builder and client callback page, and refactors 3Speak thumbnail extraction/upload to use AbortController and timeouts. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as PostContentRenderer
participant Enhancer as ThreeSpeak Enhancer
participant ThumbUtil as injectThreeSpeakThumbnail
participant Iframe as 3Speak iframe
participant User as User
Renderer->>Enhancer: render placeholders (passes images, renderOptions)
Enhancer->>ThumbUtil: request thumbnail (images, embed element)
ThumbUtil-->>Enhancer: inject <img> thumbnail (if found)
User->>Enhancer: click play
Enhancer->>Iframe: create/append iframe (direct DOM or React mount)
Iframe->>Enhancer: postMessage "3speak-player-ready" (isVertical/aspectRatio)
Enhancer->>Enhancer: update container classes (speak-portrait / speak-square)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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 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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/web/src/features/post-renderer/components/ecency-renderer.tsx (2)
78-88:⚠️ Potential issue | 🟠 MajorDon't remove author/tag enhancement until callers are migrated.
This renderer no longer applies
AuthorLinkExtension/TagLinkExtension, butapps/web/src/features/post-renderer/components/extensions/wave-like-post-extension.tsx:15-119still renders<EcencyRenderer value={post.body} />withoutrenderOptions. That regresses author/tag enhancement inside wave previews. Please keep a fallback here whenrenderOptionsis absent, or update that caller before removing these extensions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/post-renderer/components/ecency-renderer.tsx` around lines 78 - 88, The recent removal of AuthorLinkExtension and TagLinkExtension from EcencyRenderer breaks wave-like previews because WaveLikePostExtension renders <EcencyRenderer value={post.body} /> without passing renderOptions; restore a fallback so author/tag enhancement remains until callers are migrated by detecting when renderOptions is undefined in EcencyRenderer and adding AuthorLinkExtension and TagLinkExtension to the extensions list (or treat undefined as the default that enables those extensions), or alternatively update WaveLikePostExtension to pass the proper renderOptions; locate EcencyRenderer, WaveLikePostExtension, and the extension components AuthorLinkExtension/TagLinkExtension to implement the fallback or caller update accordingly.
40-63:⚠️ Potential issue | 🟠 MajorCover the new 3Speak
postMessagepath with a spec.This effect adds new user-visible behavior, but there is still no renderer test covering accepted vs ignored messages and the resulting
speak-portrait/speak-squareclasses.As per coding guidelines, "All new features in
@ecency/webrequire tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/post-renderer/components/ecency-renderer.tsx` around lines 40 - 63, Add a renderer test for the new 3Speak postMessage handling in ecency-renderer.tsx: mount the renderer with renderOptions.embedVideosDirectly=true and create a mock iframe element with class "speak-iframe" inside a container with class "markdown-video-link-speak" (set iframe.contentWindow to window or a fake object), then dispatch MessageEvent("message") with origin "https://play.3speak.tv" and data {type: "3speak-player-ready", isVertical: true} and assert the container gains "speak-portrait"; add a second test dispatching data with {type: "3speak-player-ready", aspectRatio: 1.02} and assert "speak-square"; add negative tests sending messages with wrong origin, wrong type, or event.source not matching iframe.contentWindow and assert no classes are added; use your existing test utilities (React Testing Library / jsdom) and target the useEffect/handleMessage logic and the ".speak-iframe" / "markdown-video-link-speak" selectors.
🧹 Nitpick comments (1)
apps/web/src/features/post-renderer/components/extensions/wave-like-post-extension.scss (1)
1-96: Clean up legacy wave renderer CSS to avoid divergence.The old
.ecency-renderer-wave-like-post-extension-renderer*rules inapps/web/src/styles/renderer.css:215-270appear obsolete after this rename. Removing or migrating that block would prevent duplicated/dead styling paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/post-renderer/components/extensions/wave-like-post-extension.scss` around lines 1 - 96, Remove the obsolete legacy rules matching .ecency-renderer-wave-like-post-extension-renderer* from the old stylesheet and consolidate any needed declarations into the new .er-wave-renderer block (and its modifiers .er-wave-renderer--logo, --author, --body, and nested elements like &__overlay, &--author-avatar, &--author-content) so there is no duplicated or dead styling; delete the old block or migrate its relevant properties into the new SCSS selectors and run a quick visual check to confirm nothing regressed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/src/features/post-renderer/components/extensions/wave-like-post-extension.scss`:
- Around line 58-62: The avatar exclusion selector still targets the old class
name; update the selector in the waves common styles to use the renamed class
.er-wave-renderer--author-avatar (the new avatar class defined by the component)
so the avatar is excluded from the generic image rules—locate the selector
referencing the old avatar class in the waves common stylesheet and replace it
with .er-wave-renderer--author-avatar, keeping the same specificity and
combinators as the original rule.
In `@apps/web/src/features/waves/styles/thread-render.scss`:
- Around line 3-4: The selectors currently target .thread-render and
.waves-list-item broadly, causing markdown descendant and img rules (the blocks
around lines 14–16 and 99–107) to leak onto non-markdown UI; restrict those
rules to the rendered-markdown area only by scoping descendant and image
selectors to a dedicated markdown container (e.g., change selectors using
.thread-render or .waves-list-item to target .thread-render .rendered-markdown
and .waves-list-item .rendered-markdown, and update the img/aspect-ratio rules
likewise), updating the CSS rules that apply to descendants and images so only
elements inside the rendered-markdown container receive markdown typography and
aspect-ratio styling.
---
Duplicate comments:
In `@apps/web/src/features/post-renderer/components/ecency-renderer.tsx`:
- Around line 78-88: The recent removal of AuthorLinkExtension and
TagLinkExtension from EcencyRenderer breaks wave-like previews because
WaveLikePostExtension renders <EcencyRenderer value={post.body} /> without
passing renderOptions; restore a fallback so author/tag enhancement remains
until callers are migrated by detecting when renderOptions is undefined in
EcencyRenderer and adding AuthorLinkExtension and TagLinkExtension to the
extensions list (or treat undefined as the default that enables those
extensions), or alternatively update WaveLikePostExtension to pass the proper
renderOptions; locate EcencyRenderer, WaveLikePostExtension, and the extension
components AuthorLinkExtension/TagLinkExtension to implement the fallback or
caller update accordingly.
- Around line 40-63: Add a renderer test for the new 3Speak postMessage handling
in ecency-renderer.tsx: mount the renderer with
renderOptions.embedVideosDirectly=true and create a mock iframe element with
class "speak-iframe" inside a container with class "markdown-video-link-speak"
(set iframe.contentWindow to window or a fake object), then dispatch
MessageEvent("message") with origin "https://play.3speak.tv" and data {type:
"3speak-player-ready", isVertical: true} and assert the container gains
"speak-portrait"; add a second test dispatching data with {type:
"3speak-player-ready", aspectRatio: 1.02} and assert "speak-square"; add
negative tests sending messages with wrong origin, wrong type, or event.source
not matching iframe.contentWindow and assert no classes are added; use your
existing test utilities (React Testing Library / jsdom) and target the
useEffect/handleMessage logic and the ".speak-iframe" /
"markdown-video-link-speak" selectors.
---
Nitpick comments:
In
`@apps/web/src/features/post-renderer/components/extensions/wave-like-post-extension.scss`:
- Around line 1-96: Remove the obsolete legacy rules matching
.ecency-renderer-wave-like-post-extension-renderer* from the old stylesheet and
consolidate any needed declarations into the new .er-wave-renderer block (and
its modifiers .er-wave-renderer--logo, --author, --body, and nested elements
like &__overlay, &--author-avatar, &--author-content) so there is no duplicated
or dead styling; delete the old block or migrate its relevant properties into
the new SCSS selectors and run a quick visual check to confirm nothing
regressed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72631b84-15d3-4373-8f69-1eeb7f7a2c30
⛔ Files ignored due to path filters (6)
packages/render-helper/dist/browser/index.jsis excluded by!**/dist/**packages/render-helper/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/render-helper/dist/node/index.cjsis excluded by!**/dist/**packages/render-helper/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/render-helper/dist/node/index.mjsis excluded by!**/dist/**packages/render-helper/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (6)
apps/web/public/sw.jsapps/web/src/features/post-renderer/components/ecency-renderer.tsxapps/web/src/features/post-renderer/components/extensions/wave-like-post-extension.scssapps/web/src/features/post-renderer/components/utils/setupPostEnhancements.tsapps/web/src/features/waves/styles/thread-render.scsspackages/render-helper/src/methods/linkify.method.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/features/post-renderer/components/utils/setupPostEnhancements.ts
- packages/render-helper/src/methods/linkify.method.ts
apps/web/src/features/post-renderer/components/extensions/wave-like-post-extension.scss
Show resolved
Hide resolved
Comment on lines
+3
to
+4
| .thread-render, | ||
| .waves-list-item { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/features/waves/styles/thread-render.scss | head -120Repository: ecency/vision-next
Length of output: 3359
🏁 Script executed:
cat -n apps/web/src/app/waves/_components/waves-list-item.tsx | head -320Repository: ecency/vision-next
Length of output: 11495
🏁 Script executed:
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "PostContentRenderer" | head -10Repository: ecency/vision-next
Length of output: 598
🏁 Script executed:
grep -r "markdown-view" --include="*.tsx" --include="*.ts" --include="*.scss" | head -20Repository: ecency/vision-next
Length of output: 3055
🏁 Script executed:
cat -n apps/web/src/features/shared/post-content-renderer.tsxRepository: ecency/vision-next
Length of output: 3562
🏁 Script executed:
cat -n apps/web/src/features/post-renderer/components/ecency-renderer.tsx | head -100Repository: ecency/vision-next
Length of output: 3965
Scope leak: .waves-list-item and .thread-render now receive unintended descendant/image rules
At lines 3–4, the style block now targets the entire .waves-list-item container. Because lines 14–16 and 99–107 apply broad descendant and image selectors, non-markdown UI inside the wave card (from WavesListItemHeader, avatars, action buttons) will unintentionally inherit markdown typography and image aspect-ratio treatment.
Both .thread-render and .waves-list-item should scope their descendant rules to the rendered markdown content only:
Proposed scoping fix
-.thread-render,
-.waves-list-item {
+.thread-render .markdown-view:not(.markdown-view-pure),
+.waves-list-item .markdown-view:not(.markdown-view-pure) {
display: grid;
overflow: hidden;Also applies to: lines 14–16, 99–107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/waves/styles/thread-render.scss` around lines 3 - 4,
The selectors currently target .thread-render and .waves-list-item broadly,
causing markdown descendant and img rules (the blocks around lines 14–16 and
99–107) to leak onto non-markdown UI; restrict those rules to the
rendered-markdown area only by scoping descendant and image selectors to a
dedicated markdown container (e.g., change selectors using .thread-render or
.waves-list-item to target .thread-render .rendered-markdown and
.waves-list-item .rendered-markdown, and update the img/aspect-ratio rules
likewise), updating the CSS rules that apply to descendants and images so only
elements inside the rendered-markdown container receive markdown typography and
aspect-ratio styling.
feruzm
added
the
patch
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/features/post-renderer/components/ecency-renderer.tsx`:
- Around line 53-57: The component adds "speak-portrait" and "speak-square" to a
container in the event handler but never removes them on unmount or when
embedVideosDirectly changes; update the effect that registers this handler (in
the EcencyRenderer component / the useEffect handling event.data) to track the
modified container(s) (e.g., store the last container reference or a Set of
containers) and in the effect cleanup remove both "speak-portrait" and
"speak-square" from those containers, and also ensure you remove classes when
embedVideosDirectly becomes falsy by running the same cleanup before toggling
behavior.
In `@apps/web/src/features/waves/styles/thread-render.scss`:
- Line 10: The stylesheet contains a deprecated declaration "word-break:
break-word"; remove or replace it: either delete that line and rely on the
existing "overflow-wrap: break-word" (preferred), or if you need aggressive
breaking change it to "word-break: break-all". Update the occurrence of
"word-break: break-word" in the thread-render.scss rule accordingly and ensure
only supported values remain.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 557ff4c3-19a7-4609-a074-5c6f61ee0688
⛔ Files ignored due to path filters (6)
packages/render-helper/dist/browser/index.jsis excluded by!**/dist/**packages/render-helper/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/render-helper/dist/node/index.cjsis excluded by!**/dist/**packages/render-helper/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/render-helper/dist/node/index.mjsis excluded by!**/dist/**packages/render-helper/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (6)
apps/web/public/sw.jsapps/web/src/features/post-renderer/components/ecency-renderer.tsxapps/web/src/features/post-renderer/components/extensions/wave-like-post-extension.scssapps/web/src/features/post-renderer/components/utils/setupPostEnhancements.tsapps/web/src/features/waves/styles/thread-render.scsspackages/render-helper/src/methods/linkify.method.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/features/post-renderer/components/utils/setupPostEnhancements.ts
- packages/render-helper/src/methods/linkify.method.ts
- apps/web/src/features/post-renderer/components/extensions/wave-like-post-extension.scss
Comment on lines
+53
to
+57
| if (event.data.isVertical) { | ||
| container.classList.add("speak-portrait"); | ||
| } else if (event.data.aspectRatio && Math.abs(event.data.aspectRatio - 1) < 0.1) { | ||
| container.classList.add("speak-square"); | ||
| } |
Contributor
There was a problem hiding this comment.
Classes added but never cleaned up on component unmount or re-render.
When embedVideosDirectly becomes falsy or the component unmounts, the speak-portrait and speak-square classes remain on DOM elements. Consider tracking modified containers and removing classes in the cleanup function.
🧹 Suggested fix
useEffect(() => {
if (!renderOptions?.embedVideosDirectly) return;
+ const modifiedContainers: Element[] = [];
const handleMessage = (event: MessageEvent) => {
if (event.origin !== "https://play.3speak.tv" || event.data?.type !== "3speak-player-ready") return;
const iframes = ref.current?.querySelectorAll<HTMLIFrameElement>(".speak-iframe");
iframes?.forEach((iframe) => {
if (iframe.contentWindow !== event.source) return;
const container = iframe.closest(".markdown-video-link-speak");
if (!container) return;
if (event.data.isVertical) {
container.classList.add("speak-portrait");
+ modifiedContainers.push(container);
} else if (event.data.aspectRatio && Math.abs(event.data.aspectRatio - 1) < 0.1) {
container.classList.add("speak-square");
+ modifiedContainers.push(container);
}
});
};
window.addEventListener("message", handleMessage);
- return () => window.removeEventListener("message", handleMessage);
+ return () => {
+ window.removeEventListener("message", handleMessage);
+ modifiedContainers.forEach(c => c.classList.remove("speak-portrait", "speak-square"));
+ };
}, [renderOptions?.embedVideosDirectly]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (event.data.isVertical) { | |
| container.classList.add("speak-portrait"); | |
| } else if (event.data.aspectRatio && Math.abs(event.data.aspectRatio - 1) < 0.1) { | |
| container.classList.add("speak-square"); | |
| } | |
| useEffect(() => { | |
| if (!renderOptions?.embedVideosDirectly) return; | |
| const modifiedContainers: Element[] = []; | |
| const handleMessage = (event: MessageEvent) => { | |
| if (event.origin !== "https://play.3speak.tv" || event.data?.type !== "3speak-player-ready") return; | |
| const iframes = ref.current?.querySelectorAll<HTMLIFrameElement>(".speak-iframe"); | |
| iframes?.forEach((iframe) => { | |
| if (iframe.contentWindow !== event.source) return; | |
| const container = iframe.closest(".markdown-video-link-speak"); | |
| if (!container) return; | |
| if (event.data.isVertical) { | |
| container.classList.add("speak-portrait"); | |
| modifiedContainers.push(container); | |
| } else if (event.data.aspectRatio && Math.abs(event.data.aspectRatio - 1) < 0.1) { | |
| container.classList.add("speak-square"); | |
| modifiedContainers.push(container); | |
| } | |
| }); | |
| }; | |
| window.addEventListener("message", handleMessage); | |
| return () => { | |
| window.removeEventListener("message", handleMessage); | |
| modifiedContainers.forEach(c => c.classList.remove("speak-portrait", "speak-square")); | |
| }; | |
| }, [renderOptions?.embedVideosDirectly]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/post-renderer/components/ecency-renderer.tsx` around
lines 53 - 57, The component adds "speak-portrait" and "speak-square" to a
container in the event handler but never removes them on unmount or when
embedVideosDirectly changes; update the effect that registers this handler (in
the EcencyRenderer component / the useEffect handling event.data) to track the
modified container(s) (e.g., store the last container reference or a Set of
containers) and in the effect cleanup remove both "speak-portrait" and
"speak-square" from those containers, and also ensure you remove classes when
embedVideosDirectly becomes falsy by running the same cleanup before toggling
behavior.
|
|
||
| line-height: 1.5; | ||
| overflow-wrap: break-word; | ||
| word-break: break-word; |
Contributor
There was a problem hiding this comment.
Replace deprecated break-word with break-all or overflow-wrap: break-word.
The break-word value for word-break is deprecated. Use word-break: break-all if you want aggressive breaking, or rely solely on overflow-wrap: break-word (already present on line 9) which handles most cases.
🧹 Suggested fix
overflow-wrap: break-word;
- word-break: break-word;
+ word-break: normal;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| word-break: break-word; | |
| overflow-wrap: break-word; | |
| word-break: normal; |
🧰 Tools
🪛 Stylelint (17.5.0)
[error] 10-10: Unexpected deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/features/waves/styles/thread-render.scss` at line 10, The
stylesheet contains a deprecated declaration "word-break: break-word"; remove or
replace it: either delete that line and rely on the existing "overflow-wrap:
break-word" (preferred), or if you need aggressive breaking change it to
"word-break: break-all". Update the occurrence of "word-break: break-word" in
the thread-render.scss rule accordingly and ensure only supported values remain.
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 by CodeRabbit
New Features
New Features / Enhancements
Improvements
Tests