Stabilize drop media

[simo6529]

Summary by CodeRabbit

• Improvements
  • Enhanced YouTube preview loading states with clearer error messaging ("Failed to load YouTube preview" vs "YouTube preview unavailable").
  • Improved visual stability and layout consistency across preview cards with fixed height constraints and better overflow handling.
  • Refined text truncation and aspect ratio handling for embedded content and images.
  • Better rendering of link previews and drop replies with improved spacing and alignment.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors YouTube preview and link preview components to introduce status-driven rendering, stable-frame containers for layout consistency, and enhanced height/overflow constraints. Tests are updated to validate new component structures and async preview resolution interactions.

Changes

Cohort / File(s)	Summary
YouTube Preview __tests__/components/drops/view/part/DropPartMarkdown.test.tsx, components/drops/view/part/dropPartMarkdown/youtubePreview.tsx	Refactored to use status-driven rendering ("loading", "success", "error", "empty") instead of boolean error flags; introduced stable-frame containers and fallback messaging; test updated to validate async preview resolution and new UI structure with encoded thumbnail URLs.
Link Preview Card __tests__/components/waves/LinkPreviewCard.test.tsx, components/waves/LinkPreviewCard.tsx	Introduced stable-frame styling for chat variant via shouldUseStableFrame flag; refactored rendering to compose content blocks instead of early returns; added CHAT_STABLE_FRAME_CLASSES; test includes assertStableFrame helper and home variant coverage.
OpenGraph Preview __tests__/components/waves/OpenGraphPreview.test.tsx, components/waves/OpenGraphPreview.tsx	Enhanced styling with explicit height constraints (tw-h-full, tw-min-h-0), improved overflow handling, and responsive layouts; adjusted skeleton, placeholder, and image containers; test normalized with updated assertions on element classes and content styling.
WaveDropReply __tests__/components/waves/drops/WaveDropReply.test.tsx, components/waves/drops/WaveDropReply.tsx	Added fixed-height wrapper (fixedReplyHeightClasses, data-testid="wave-drop-reply-fixed-container") and expanded flex/width constraints for content overflow stability; test refactored with explicit within queries, ContentDisplay spy, and expectFixedContainer helper.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant YouTubePreview as YouTube<br/>Preview Component
    participant FetchAPI as Fetch API
    participant Renderer as Status<br/>Renderer

    User->>YouTubePreview: Click show preview button
    YouTubePreview->>YouTubePreview: Set status: "loading"
    YouTubePreview->>Renderer: Render loading view
    Renderer-->>User: Display loading frame

    YouTubePreview->>FetchAPI: Fetch preview metadata
    FetchAPI-->>YouTubePreview: Return preview data

    alt Preview data valid
        YouTubePreview->>YouTubePreview: Set status: "success"
        YouTubePreview->>Renderer: Render with thumbnail & title
        Renderer-->>User: Display preview content
    else Invalid or missing data
        YouTubePreview->>YouTubePreview: Set status: "error" or "empty"
        YouTubePreview->>Renderer: Render fallback messaging
        Renderer-->>User: Display fallback link with message
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Boosted cards UI fixes #1829 — Modifies OpenGraphPreview.tsx with overlapping height/line-clamp styling adjustments to achieve consistent vertical constraints.
• Hide link preview for internal links  #1778 — Touches YouTube preview rendering logic; this PR expands that work with status-driven state management and fallback UI improvements.
• Cards width bugfix #1549 — Modifies LinkPreviewCard.tsx and OpenGraphPreview.tsx layout/overflow behavior; aligns with stable-frame and responsive constraint patterns introduced here.

Suggested reviewers

• prxt6529
• ragnep

Poem

🐰 A rabbit's ode to preview frames so fine,
Status flows like carrots in a line,
From loading states to errors caught with grace,
Stable frames now hold their rightful place! 🎬✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Stabilize drop media' directly corresponds to the main objective of this PR, which involves stabilizing media rendering in drop components through layout fixes, height constraints, and improved overflow handling.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch stabilize-drop-media

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

components/waves/LinkPreviewCard.tsx (1)

104-153: Duplicated wrapper markup between fallback and ens branches.

Lines 106-119 (fallback) and 130-143 (ens) share the same outer LinkPreviewCardLayout → variant-conditional div wrapper structure. The only difference is the inner content (fallbackContent vs EnsPreviewCard). Consider extracting the shared card-shell into a small helper to reduce duplication.

♻️ Suggested extraction

+const CardShell = ({
+  href,
+  resolvedVariant,
+  children,
+}: {
+  href: string;
+  resolvedVariant: LinkPreviewVariant | undefined;
+  children: ReactNode;
+}) => (
+  <LinkPreviewCardLayout href={href} variant={resolvedVariant}>
+    <div
+      className={
+        resolvedVariant === "home"
+          ? "tw-relative tw-h-full tw-w-full tw-overflow-hidden tw-rounded-xl tw-border tw-border-solid tw-border-white/10 tw-bg-black/30 tw-p-4"
+          : "tw-h-full tw-min-h-0 tw-w-full tw-overflow-hidden tw-rounded-xl tw-border tw-border-solid tw-border-iron-700 tw-bg-iron-900/40 tw-p-4"
+      }
+    >
+      {children}
+    </div>
+  </LinkPreviewCardLayout>
+);

components/waves/drops/WaveDropReply.tsx (1)

28-28: Move constant outside the component.

fixedReplyHeightClasses is a static string that doesn't depend on props or state. Hoisting it to module scope avoids re-creation on every render and signals it's invariant.

♻️ Suggested change

+const FIXED_REPLY_HEIGHT_CLASSES = "tw-h-[24px] tw-min-h-[24px] tw-max-h-[24px]";
+
 export default function WaveDropReply({
   dropId,
   dropPartId,
   maybeDrop,
   onReplyClick,
 }: WaveDropReplyProps) {
-  const fixedReplyHeightClasses = "tw-h-[24px] tw-min-h-[24px] tw-max-h-[24px]";

Then reference FIXED_REPLY_HEIGHT_CLASSES on line 88.

__tests__/components/waves/drops/WaveDropReply.test.tsx (1)

17-37: Consider resetting hookData in beforeEach to avoid stale state leakage across tests.

Currently, each test manually sets the fields it needs on the shared mutable hookData object, but there's no full reset in beforeEach. If a future test forgets to set a field, it inherits whatever the previous test left. A simple reset alongside the spy clear would make the suite more resilient.

♻️ Suggested addition

   beforeEach(() => {
     mockContentDisplaySpy.mockClear();
+    hookData.drop = null;
+    hookData.content = { segments: [] };
+    hookData.isLoading = false;
   });

__tests__/components/drops/view/part/DropPartMarkdown.test.tsx (1)

385-413: Stable-frame and class-name assertions are tightly coupled to implementation details.

Asserting exact Tailwind class strings (tw-h-[13rem], md:tw-h-[15rem], tw-line-clamp-2, etc.) via className.toContain(…) makes these tests fragile — any CSS refactor (e.g. switching to a design token or renaming the utility) will break them even if the visual behavior is unchanged.

Consider wrapping these layout constraints behind a data-testid or a semantic attribute (e.g. data-variant="stable") if the intent is only to prove the stable-frame container is present, and dropping the class-name checks. If the classes truly need to be pinned (to prevent accidental layout shifts), keep them but document that intent with a comment so future maintainers don't remove them unknowingly.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud