Conversation
WalkthroughAdds data-wave-drop-id and data-serial-no DOM attributes, implements a new client hook useWaveDropsClipboard to build plain/Markdown clipboard payloads (embeds, attachments, quotes, selection ranges), wires the hook into the WaveDrops renderer, updates DropsList/HighlightDropWrapper to pass IDs, and adjusts a timezone test helper. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as WaveDrops UI
participant Hook as useWaveDropsClipboard
participant DOM as Document/Elements
participant Clipboard as Clipboard API
User->>UI: Select messages (click/drag or keyboard)
UI->>Hook: containerRef active & drops supplied
Hook->>DOM: attach copy listener / inspect selection on copy
DOM-->>Hook: copy event (Ctrl/Cmd+C) with modifiers
Note over Hook,DOM: Map DOM nodes -> data-wave-drop-id -> drop objects
Hook->>Hook: Build per-drop payloads (plain + markdown), resolve embeds/quotes, format timestamps
Hook->>Clipboard: write text/plain and optionally text/markdown
Clipboard->>User: clipboard populated
alt Fallback path
Hook->>Clipboard: navigator.clipboard.writeText() fallback
Clipboard->>User: fallback success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
🧬 Code graph analysis (1)components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (9)
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: 1
🧹 Nitpick comments (6)
components/waves/drops/wave-drops-all/index.tsx (1)
3-3: Clipboard wiring looks solid; minor perf/type nits.
- Consider avoiding a freshly allocated [] in the memo to reduce needless re-runs:
const dropsForClipboard = waveMessages?.drops ?? EMPTY_DROPS; with EMPTY_DROPS as a module constant.- Narrow the ref type in options to HTMLDivElement to match usage and catch mismatches earlier.
-const containerRef = useRef<HTMLDivElement | null>(null); +const containerRef = useRef<HTMLDivElement | null>(null); -const dropsForClipboard = useMemo( - () => waveMessages?.drops ?? [], - [waveMessages?.drops] -); +const EMPTY_DROPS: readonly ExtendedDrop[] = []; +const dropsForClipboard = useMemo( + () => waveMessages?.drops ?? EMPTY_DROPS, + [waveMessages?.drops] +);Also applies to: 18-18, 55-55, 72-76, 77-81, 148-151
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (5)
590-596: Also write text/markdown when exporting Markdown.In Markdown mode, set both text/plain and text/markdown to maximize compatibility.
- if (event.clipboardData) { - event.clipboardData.setData("text/plain", payload); - } + if (event.clipboardData) { + event.clipboardData.setData("text/plain", payload); + if (formatRef.current === "markdown") { + event.clipboardData.setData("text/markdown", payload); + } + }
72-79: Harden CSS escaping fallback.The fallback only escapes quotes/backslashes; IDs with ] or control chars could still break attribute selectors. Prefer a small local escape that covers CSS specials or ship a tiny CSS.escape polyfill.
-const escapeAttributeValue = (value: string): string => { +const escapeAttributeValue = (value: string): string => { if (typeof CSS !== "undefined" && typeof CSS.escape === "function") { return CSS.escape(value); } - return value.replace(/["\\]/g, "\\$&"); + return value.replace(/[\0-\x1F\x7F"\\\[\]]/g, "\\$&"); };
34-70: Simplify editable-node detection.Minor redundancy and an unnecessary ternary; tighten without behavior change.
-const nodeIsEditable = (node: Node | null): boolean => { +const nodeIsEditable = (node: Node | null): boolean => { if (!node) { return false; } if (isHTMLElement(node)) { if (node.isContentEditable) { return true; } if ( node instanceof HTMLInputElement || node instanceof HTMLTextAreaElement ) { return true; } if (node.dataset?.waveClipboardAllowDefault === "true") { return true; } } - const parent = isHTMLElement(node) ? node.parentElement : node.parentElement; + const parent = (node as Node).parentElement; if (!parent) { return false; } if (parent.closest('[contenteditable="true"]')) { return true; } if (parent instanceof HTMLInputElement || parent instanceof HTMLTextAreaElement) { return true; } return ( parent.closest('[data-wave-clipboard-allow-default="true"]') !== null ); };
264-274: Improve Markdown for attachments (optional).In Markdown mode, consider formatting attachments as links to distinguish them from plain text.
- attachmentMarkdownLines: uniqueAttachments, + attachmentMarkdownLines: uniqueAttachments.map((u) => `[attachment](${u})`),
598-599: Remove inline comment to match “no comments in TS/TSX” guideline.Delete the in-catch comment.
- .catch(() => { - // Silently ignore clipboard promise failures – the event clipboard fallback already ran. - }); + .catch(() => {});As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
codex/STATE.md(1 hunks)codex/tickets/TKT-0010.md(1 hunks)components/waves/drops/WaveDrop.tsx(1 hunks)components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts(1 hunks)components/waves/drops/wave-drops-all/index.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript across the codebase
Files:
components/waves/drops/WaveDrop.tsxcomponents/waves/drops/wave-drops-all/index.tsxcomponents/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks for UI components
Files:
components/waves/drops/WaveDrop.tsxcomponents/waves/drops/wave-drops-all/index.tsx
🧬 Code graph analysis (2)
components/waves/drops/wave-drops-all/index.tsx (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (1)
useWaveDropsClipboard(398-613)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
generated/models/ApiDropMetadata.ts (1)
ApiDropMetadata(15-41)helpers/waves/drop.helpers.ts (1)
ExtendedDrop(16-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
codex/STATE.md (1)
16-16: LGTM — metadata date updated correctly.components/waves/drops/WaveDrop.tsx (1)
325-327: Attributes align with clipboard selector usage.The data attributes match the hook’s query logic (dataset.waveDropId). No issues.
codex/tickets/TKT-0010.md (1)
45-45: Log entry reads clear and auditable.Entry captures scope and verification status. No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (1)
132-142: Fix Markdown-to-plain conversion; current bold regex is wrong.The pattern uses the captured text as the closing delimiter, so bold markers aren't stripped reliably; the generic character cull then over-removes legitimate characters like hyphens and underscores.
🧹 Nitpick comments (2)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
54-54: Simplify redundant ternary.Both branches of the ternary return
node.parentElement, making the conditional check unnecessary.Apply this diff:
- const parent = isHTMLElement(node) ? node.parentElement : node.parentElement; + const parent = node.parentElement;
177-177: Remove unnecessary type assertion.TypeScript can infer the type from the EmbedInfo interface definition.
Apply this diff:
- const group = groups.get(groupKey) ?? { extras: [] as string[] }; + const group = groups.get(groupKey) ?? { extras: [] };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
codex/STATE.md(1 hunks)components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- codex/STATE.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript across the codebase
Files:
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts
🧬 Code graph analysis (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
generated/models/ApiDropMetadata.ts (1)
ApiDropMetadata(15-41)helpers/waves/drop.helpers.ts (1)
ExtendedDrop(16-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
607-609: Remove comment to comply with coding guidelines.The inline comment violates the coding guideline that specifies no comments in the code.
As per coding guidelines
Apply this diff:
if (navigator?.clipboard?.writeText) { void navigator.clipboard .writeText(payload) - .catch(() => { - // Silently ignore clipboard promise failures – the event clipboard fallback already ran. - }); + .catch(() => {}); }
132-144: Fix the broken bold/strong regex on line 138.The regex
/(?:\*\*|__)(.*?)\1/gis incorrect. The\1backreferences the first capture group(.*?)(the content), not the delimiter. This means it tries to match**contentfollowed by the same content repeated (e.g.,**hello**hello**), which will never match standard markdown bold syntax.Apply this diff to fix the bold pattern:
const toPlainText = (markdown: string): string => markdown .replace(/```([\s\S]*?)```/g, (_, code) => code.trim()) .replace(/`([^`]+)`/g, "$1") .replace(/!\[.*?]\((.*?)\)/g, (_, url: string) => url) .replace(/\[(.*?)]\((.*?)\)/g, "$1 ($2)") - .replace(/(?:\*\*|__)(.*?)\1/g, "$1") + .replace(/(\*\*|__)(.*?)\1/g, "$2") .replace(/(^|[^\w])\*([^*\s][^*]*?)\*(?=$|[^\w])/g, (_, prefix: string, content: string) => `${prefix}${content}`) .replace(/(^|[^\w])_([^_\s][^_]*?)_(?=$|[^\w])/g, (_, prefix: string, content: string) => `${prefix}${content}`) .replace(/~~([^~]+)~~/g, "$1") .replace(/^\s{0,3}>\s?/gm, "") .replace(/\n{3,}/g, "\n\n") .trim();Note: A past review comment marked this as addressed, but the fix is not present in the current code.
🧹 Nitpick comments (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (1)
105-105: Remove deprecateddetach()calls.
Range.detach()has been deprecated and is now a no-op in modern browsers. The optional chaining indicates awareness it might not exist, but these calls are unnecessary and can be removed.Apply this diff:
const coversStart = range.compareBoundaryPoints(Range.START_TO_START, elementRange) <= 0; const coversEnd = range.compareBoundaryPoints(Range.END_TO_END, elementRange) >= 0; - elementRange.detach?.(); - return coversStart && coversEnd;const text = clipped.toString(); - clipped.detach?.(); - elementRange.detach?.(); - return text;Also applies to: 127-128
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
__tests__/components/meme-calendar/meme-calendar.helpers.timezone.test.ts(1 hunks)components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript across the codebase
Files:
__tests__/components/meme-calendar/meme-calendar.helpers.timezone.test.tscomponents/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/components/meme-calendar/meme-calendar.helpers.timezone.test.ts
__tests__/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use
@testing-library/reactand@testing-library/user-eventfor React component tests
Files:
__tests__/components/meme-calendar/meme-calendar.helpers.timezone.test.ts
**/__tests__/**
📄 CodeRabbit inference engine (AGENTS.md)
Place tests in
__tests__directories when organizing test suites
Files:
__tests__/components/meme-calendar/meme-calendar.helpers.timezone.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Mock external dependencies and APIs in tests
Files:
__tests__/components/meme-calendar/meme-calendar.helpers.timezone.test.ts
🧬 Code graph analysis (2)
__tests__/components/meme-calendar/meme-calendar.helpers.timezone.test.ts (1)
components/meme-calendar/meme-calendar.helpers.tsx (1)
wallTimeToUtcInstantInZone(103-135)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
generated/models/ApiDropMetadata.ts (1)
ApiDropMetadata(15-41)helpers/waves/drop.helpers.ts (1)
ExtendedDrop(16-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
__tests__/components/meme-calendar/meme-calendar.helpers.timezone.test.ts (1)
18-27: The original review comment is incorrect. The function is internal to the module and cannot be imported.Verification shows that
mintEndInstantUtcForMintDayexists incomponents/meme-calendar/meme-calendar.helpers.tsxat line 176, but it is not exported—it's an internal helper function. The test file correctly defines a local copy because the actual implementation is not part of the module's public API and therefore cannot be imported.This is not code duplication or a test isolation issue—it's the expected pattern when working with internal module helpers in tests. The local implementation in the test allows the test suite to function independently without exposing internal details.
No changes are needed.
Likely an incorrect or invalid review comment.
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (1)
427-444: Verify the safety of the formatRef pattern for keyboard-to-clipboard coordination.Using
useRefto coordinate state betweenkeydownandcopyevents works, but creates a potential race condition if:
- Multiple key events occur before copy
- Copy event is delayed or queued
- User presses Cmd+Shift+C then quickly presses Cmd+C
The last keydown always wins, which may not reflect the actual key combo that triggered the copy.
Consider whether this edge case matters for your use case. If the timing window is acceptable, the current approach is fine. Otherwise, you might need to:
- Store the format in the event itself (if possible)
- Read modifier keys directly from the copy event
- Use a timestamp-based approach to match keydown to copy events
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (1)
445-457: Fix inconsistent indentation.Lines 448-457 have inconsistent indentation compared to lines 446-447. All properties in the return object should align.
Apply this diff:
return { id: drop.stableHash ?? drop.id, author: drop.author?.handle ?? "Unknown", - timestamp: drop.created_at, - markdownContent, - plainContent, + timestamp: drop.created_at, + markdownContent, + plainContent, embedPlainLines, embedMarkdownLines, attachmentPlainLines: uniqueAttachments, attachmentMarkdownLines: uniqueAttachments.map( (url) => `[attachment](${url})` ), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/drops/view/DropsList.tsx(1 hunks)components/drops/view/HighlightDropWrapper.tsx(3 hunks)components/waves/drops/WaveDrop.tsx(1 hunks)components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts(1 hunks)components/waves/drops/wave-drops-all/index.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/waves/drops/WaveDrop.tsx
- components/waves/drops/wave-drops-all/index.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript across the codebase
Files:
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.tscomponents/drops/view/DropsList.tsxcomponents/drops/view/HighlightDropWrapper.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks for UI components
Files:
components/drops/view/DropsList.tsxcomponents/drops/view/HighlightDropWrapper.tsx
🧬 Code graph analysis (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
generated/models/ApiDropMetadata.ts (1)
ApiDropMetadata(15-41)helpers/waves/drop.helpers.ts (1)
ExtendedDrop(16-20)
🪛 Biome (2.1.2)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts
[error] 79-79: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
components/drops/view/HighlightDropWrapper.tsx (1)
24-24: LGTM! Clean integration of waveDropId prop.The optional
waveDropIdprop is properly typed withreadonly, correctly destructured, and applied as a data attribute. The implementation follows React conventions whereundefinedvalues don't render attributes.Also applies to: 43-43, 247-253
components/drops/view/DropsList.tsx (1)
127-131: LGTM! Correct fallback chain for wave drop IDs.The logic correctly computes
waveDropIdonly for FULL drops using a sensible fallback chain (stableHash ?? drop.id ?? drop.stableKey), matching the pattern used inbuildClipboardMessage(line 446 of useWaveDropsClipboard.ts).components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (5)
74-80: Static analysis warning is a false positive.The control character range
\0-\x1Fin the regex is intentional for escaping CSS selector special characters whenCSS.escapeis unavailable. The implementation correctly falls back to manual escaping for older environments.However, given modern browser support, consider simplifying to use
CSS.escapeexclusively if the target environment guarantees support, or document the fallback necessity.
158-224: LGTM! Robust metadata parsing with proper fallbacks.The
extractEmbedsfunction handles hierarchical metadata keys correctly using separators, and the quote source registry pattern ensures referenced drops are tracked and merged properly.
460-577: LGTM! Clean formatting with proper locale support.The formatting functions correctly handle both plain and markdown formats, use
Intl.DateTimeFormatfor locale-aware timestamps, and adjust heading style based on message count for better readability.
579-868: LGTM! Comprehensive selection handling with partial-range support.The selection logic correctly handles complex scenarios: detects editable contexts to avoid interference, safely escapes selectors, uses Range API for boundary detection, and extracts partial text when drops are only partially selected.
870-1019: LGTM! Well-structured hook with proper React patterns.The hook correctly:
- Memoizes expensive computations (drop filtering, quote lookup, message building)
- Tracks mutable format state with
useRef- Handles keyboard (Shift for markdown) and copy events
- Uses both
event.clipboardDataandnavigator.clipboardfor broad compatibility- Properly cleans up event listeners on unmount
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (3)
984-1073: Re-run effect when the container node changes; avoid stale listeners.The effect depends on
containerRef(stable object), so ifcontainerRef.currentswaps (remount), listeners won’t re-bind. Depend on the actual node.- }, [clipboardMessages, containerRef]); + }, [clipboardMessages, containerRef.current]);To verify potential leaks across remounts, search for places where the container is unmounted/remounted and confirm listeners are reattached.
728-736: Don’t bail on unknown ids; skip them to make selection handling resilient.Returning
nullif any selected id isn’t inclipboardMessagesdisables copy needlessly (e.g., mixed content). Skip missing ids and proceed if at least one remains.const messages: ClipboardMessage[] = []; for (const id of selectedIds) { const message = clipboardMessages.get(id); - if (!message) { - return null; - } - messages.push(message); + if (message) { + messages.push(message); + } } + if (messages.length === 0) { + return null; + }
134-147: toPlainText improvements are solid; consider ordered lists too.Current replacements look good. Optionally also strip ordered list prefixes like “1. ” or “1) ”.
markdown .replaceAll(/```([\s\S]*?)```/g, (_, code) => code.trim()) .replaceAll(/`([^`]+)`/g, "$1") .replaceAll(/!\[[^\]]*]\(([^)]+)\)/g, "$1") .replaceAll(/\[([^\]]+)\]\(([^)]+)\)/g, "$1 ($2)") .replaceAll(/(\*\*|__)(.*?)\1/g, "$2") .replaceAll(/([*_])(.*?)\1/g, "$2") .replaceAll(/~~(.*?)~~/g, "$1") .replaceAll(/(^|\n)#{1,6}\s+/g, "$1") .replaceAll(/(^|\n)\s*[-*+]\s+/g, "$1") + .replaceAll(/(^|\n)\s*\d+[.)]\s+/g, "$1") .replaceAll(/(^|\n)>\s?/g, "$1") .replaceAll(/\n{3,}/g, "\n\n") .trim();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
codex/STATE.md(1 hunks)components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- codex/STATE.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript across the codebase
Files:
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts
🪛 Biome (2.1.2)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts
[error] 79-79: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 79-79: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
81-82: Consider uppercase hex literals for consistency.While the lowercase hex notation (
0x1f,0x7f) works correctly, uppercase (0x1F,0x7F) is more conventional in JavaScript/TypeScript codebases and improves readability.Apply this diff:
const code = ch.codePointAt(0)!; - if (code <= 0x1f || code === 0x7f || ch === "\"" || ch === "\\" || ch === "[" || ch === "]") { + if (code <= 0x1F || code === 0x7F || ch === "\"" || ch === "\\" || ch === "[" || ch === "]") { out += "\\" + ch;
943-948: Consider addingreadonlyto properties for consistency.While not strictly required since this is an internal type, adding
readonlyto the properties ofBuildSegmentsOptionswould improve immutability guarantees and align with the pattern used in other types likeUseWaveDropsClipboardOptions.Apply this diff:
type BuildSegmentsOptions = { - selectedIds: string[]; - partialSegments: Map<string, string>; - messagesById: Map<string, ClipboardMessage>; - format: ClipboardFormat; + readonly selectedIds: string[]; + readonly partialSegments: Map<string, string>; + readonly messagesById: Map<string, ClipboardMessage>; + readonly format: ClipboardFormat; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript across the codebase
Files:
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts
🧬 Code graph analysis (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
generated/models/ApiDropMetadata.ts (1)
ApiDropMetadata(15-41)helpers/waves/drop.helpers.ts (1)
ExtendedDrop(16-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (10)
1-29: LGTM: Type definitions follow best practices.The type definitions are well-structured with appropriate
readonlymodifiers on interface properties, and the imports are organized correctly.
31-72: LGTM: Node checking logic is thorough.The utility functions correctly identify editable nodes and handle various edge cases including contentEditable elements, form inputs, and custom data attributes.
91-141: LGTM: Element finding and range utilities are well-implemented.The DOM traversal and range manipulation logic correctly handles element lookups, range coverage checks, and text extraction from selections.
143-157: LGTM: Markdown-to-plain-text conversion is well-structured.The function correctly strips various Markdown constructs in the proper order, with appropriate use of backreferences to match paired delimiters. The fix from previous reviews addressed the earlier regex issues.
159-260: LGTM: Embed extraction logic is well-designed.The metadata parsing and grouping logic correctly handles various embed patterns using immutable data structures. The fallback mechanisms for title, URL, and description fields are appropriate.
262-403: LGTM: Quote and reference handling is comprehensive.The code correctly merges quote drop sources, maintains a registry, and formats references with appropriate fallbacks for deleted or incomplete data. The blockquote formatting properly handles multi-line content.
405-563: LGTM: Message building logic is robust.The clipboard message construction correctly assembles content from multiple sources (replies, quotes, parts, metadata, media) with appropriate handling of different drop types and graceful fallbacks for missing data.
566-683: LGTM: Formatting utilities handle edge cases well.The timestamp formatting includes proper error handling, and the message formatting logic correctly differentiates between single and multiple message contexts, with appropriate format-specific styling.
685-942: LGTM: Selection and range handling is sophisticated and correct.The code properly handles complex selection scenarios including:
- Filtering editable elements
- Detecting range intersections with error handling
- Sorting messages chronologically
- Managing partial selections at boundaries
- Clipping selection ranges to element bounds
The logic correctly distinguishes between fully-selected and partially-selected drops.
984-1134: LGTM: Hook implementation is well-architected.The main hook correctly:
- Uses
useMemoto optimize expensive computations (drop filtering, lookup maps, message building)- Manages format state with a ref to avoid re-renders
- Properly coordinates keydown and copy events for format selection
- Handles partial and full selections
- Supports both plain and markdown output
- Includes proper event listener cleanup
The clipboard API usage with fallback handling is appropriate, and the dependencies array is correct.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript across the codebase
Files:
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts
🧬 Code graph analysis (1)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (2)
generated/models/ApiDropMetadata.ts (1)
ApiDropMetadata(15-41)helpers/waves/drop.helpers.ts (1)
ExtendedDrop(16-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
components/waves/drops/wave-drops-all/hooks/useWaveDropsClipboard.ts (8)
1-29: LGTM!The imports and type definitions are well-structured. All properties correctly use
readonlyas per coding guidelines.
31-141: LGTM!The utility functions are well-implemented with appropriate defensive checks. Past review issues (redundant ternary, control character regex) have been properly addressed.
143-157: LGTM!The markdown-to-plain text conversion has been properly fixed with correct regex patterns for bold, italic, and other markdown elements. The past review issue has been addressed.
159-260: LGTM!The embed extraction logic is well-structured with appropriate null handling and immutable updates.
262-403: LGTM!The quote and reference handling logic is well-organized with clear separation of concerns and proper null handling.
405-667: LGTM!The message building logic is comprehensive and handles edge cases well with appropriate fallbacks. The timestamp formatting includes proper error handling.
669-982: LGTM!The selection handling logic is robust with proper error handling and well-organized helper functions for complex range operations.
1045-1134: LGTM!The effect correctly sets up and cleans up event listeners. The global
keydownhandler is acceptable since it verifies the event is relevant to this container before acting. Dependency array is correct and cleanup is properly implemented.
|
2 participants
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation
Tests