Skip to content

Conversation

ameer2468
Copy link
Collaborator

@ameer2468 ameer2468 commented Oct 8, 2025

Summary by CodeRabbit

  • New Features

    • Unified “Copy link” behavior that automatically uses the correct domain and environment.
    • Improved copy interaction feedback for a clearer, more consistent experience across the card.
  • Bug Fixes

    • You can now delete comments without a parent (root-level comments) reliably.
    • Deletion confirmation is shown consistently, ensuring clearer decision prompts.
    • Reduced link-copy inconsistencies across different environments and domain setups.

Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Caution

Review failed

The head commit changed during the review from 0725d77 to c833dfa.

Walkthrough

Refactors CapCard to derive domain info from context and consolidate copy-link logic into a new internal handler. Updates Comment component’s delete flow by requiring a concrete parentId (nullable), making confirmation unconditional, and invoking onDelete with explicit null when no parent exists.

Changes

Cohort / File(s) Summary of Changes
CapCard copy/link refactor
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
Removed CapCardProps fields customDomain? and domainVerified?. Now derives domain data from useDashboardContext. Introduced copyLinkHandler to compute/copy URLs. Replaced inline copy logic and updated UI to use copyPressed state with centralized handler.
Comment deletion flow/signature
apps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsx
Changed onDelete prop signature to `(commentId, parentId: Comment.CommentId

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CapCard UI
  participant CapCard Logic as CapCard copyLinkHandler
  participant Clipboard

  User->>CapCard UI: Click "Copy link"
  CapCard UI->>CapCard Logic: Request URL for copy
  CapCard Logic->>CapCard Logic: Read activeOrganization from context
  CapCard Logic->>CapCard Logic: Derive customDomain/domainVerified
  CapCard Logic->>CapCard Logic: Construct URL (env/custom domain)
  CapCard Logic->>Clipboard: WriteText(URL)
  Clipboard-->>CapCard Logic: Success/Failure
  CapCard Logic-->>CapCard UI: Update copyPressed state
  Note over CapCard Logic,CapCard UI: Centralized link construction replaces inline logic
Loading
sequenceDiagram
  autonumber
  actor User
  participant Comment UI
  participant Confirm Dialog
  participant Parent Handler as onDelete(commentId, parentId|null)

  User->>Comment UI: Click "Delete"
  Comment UI->>Confirm Dialog: Prompt delete confirmation (unconditional)
  alt User confirms
    Comment UI->>Parent Handler: onDelete(comment.id, comment.parentCommentId or null)
  else User cancels
    Comment UI-->>User: No action
  end
  Note over Comment UI: parentId is now explicit null when absent
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

I twitch my ears at links that gleam,
One hop, one copy—centralized dream.
Comments whisper, “farewell” with grace,
Parent or none, we mark the place.
In tidy trails through fields of flow,
A rabbit nods: refactors grow. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely describes the two main fixes implemented in this pull request—correcting the comment deletion flow and addressing custom domain link generation (including handling spaces)—without extraneous detail or generic wording.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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

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

@ameer2468 ameer2468 merged commit 4b9eb28 into main Oct 8, 2025
14 checks passed
@ameer2468 ameer2468 deleted the fix-copy-link-spaces branch October 8, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant