Merged
Conversation
📝 WalkthroughWalkthroughExtracts mobile copy-link UI/clipboard logic into a new WaveDropMobileMenuCopyLink component, integrates it into multiple mobile touch menus (leaderboard, grid, winners, memes), removes inline clipboard handling from WaveDropMobileMenu, updates mobile/desktop action gating, and adds tests for clipboard success/failure, temporary-drop disabling, event bubbling, and mobile menu presence. ChangesMobile copy-link extraction & integration
Approve-wave / small-leaderboard behavior and approval-status flow
Miscellaneous small fixes
Sequence Diagram(s)sequenceDiagram
participant User
participant MobileMenu as MobileMenu (portal)
participant CopyButton as WaveDropMobileMenuCopyLink
participant Clipboard as navigator.clipboard
participant MenuState as Mobile menu state
User->>MobileMenu: long-press -> open touch action sheet
MobileMenu->>CopyButton: render copy action item
User->>CopyButton: tap "Copy link"
CopyButton->>Clipboard: writeText(dropLink) (if supported)
Clipboard-->>CopyButton: resolves/rejects
CopyButton-->>MenuState: call onCopy() -> setIsActive(false)
CopyButton-->>User: show "Copied!" feedback (2s)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 (1)
__tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsx (1)
32-107: ⚡ Quick winAdd a test case covering clipboard write failure.
navigator.clipboard.writeTextcan be denied by browser permissions in production. The current suite only mocks a resolved write. An error path test would ensure the component handles rejection gracefully (e.g., no uncaught promise,onCopynot invoked, error state shown).➕ Suggested additional test case
+ it("handles clipboard write failure gracefully", async () => { + writeText.mockRejectedValueOnce(new DOMException("NotAllowedError")); + const onCopy = jest.fn(); + const drop: any = { + id: "d1", + wave: { id: "w1" }, + serial_no: 5, + drop_type: ApiDropType.Chat, + }; + + render(<WaveDropMobileMenuCopyLink drop={drop} onCopy={onCopy} />); + + await userEvent.click(screen.getByRole("button", { name: "Copy link" })); + + // onCopy should NOT be called on write failure + expect(onCopy).not.toHaveBeenCalled(); + // No uncaught error thrown from the component + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsx` around lines 32 - 107, Add a test that simulates navigator.clipboard.writeText rejecting and verify the component handles it: in __tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsx add a case where writeText.mockRejectedValueOnce(new Error(...)) before rendering WaveDropMobileMenuCopyLink, click the "Copy link" button, then assert that onCopy was NOT called, no uncaught promise is thrown (test completes), and any UI error indicator produced by WaveDropMobileMenuCopyLink (e.g., an error message or aria-live region) is rendered; reference the WaveDropMobileMenuCopyLink component, navigator.clipboard.writeText mock, and the onCopy mock to locate and update the tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/waves/drops/WaveDropMobileMenuCopyLink.tsx`:
- Around line 81-99: The onCopy() call is executed synchronously before the
clipboard.writeText Promise resolves, causing the component to unmount before
setCopied(true) runs; move the onCopy() invocation into the
clipboard.writeText().then() callback after setCopied(true) (and keep it in the
.catch() path for failures) so the "Copied!" state is set and visible before the
menu is dismissed; update references in the block around clipboard.writeText,
setCopied, copiedResetTimeoutRef, isMountedRef, and onCopy accordingly.
---
Nitpick comments:
In `@__tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsx`:
- Around line 32-107: Add a test that simulates navigator.clipboard.writeText
rejecting and verify the component handles it: in
__tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsx add a case
where writeText.mockRejectedValueOnce(new Error(...)) before rendering
WaveDropMobileMenuCopyLink, click the "Copy link" button, then assert that
onCopy was NOT called, no uncaught promise is thrown (test completes), and any
UI error indicator produced by WaveDropMobileMenuCopyLink (e.g., an error
message or aria-live region) is rendered; reference the
WaveDropMobileMenuCopyLink component, navigator.clipboard.writeText mock, and
the onCopy mock to locate and update the tests.
🪄 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: 942fb577-1617-42ca-ac16-1c750182c9df
📒 Files selected for processing (11)
__tests__/components/memes/drops/MemesLeaderboardDrop.test.tsx__tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsx__tests__/components/waves/leaderboard/drops/DefaultWaveLeaderboardDrop.interaction.test.tsx__tests__/components/waves/leaderboard/grid/WaveLeaderboardGridItem.test.tsxcomponents/memes/drops/MemesLeaderboardDrop.tsxcomponents/waves/drops/WaveDropMobileMenu.tsxcomponents/waves/drops/WaveDropMobileMenuCopyLink.tsxcomponents/waves/leaderboard/drops/DefaultWaveLeaderboardDrop.tsxcomponents/waves/leaderboard/grid/WaveLeaderboardGridItem.tsxcomponents/waves/winners/drops/DefaultWaveWinnerDrop.tsxcomponents/waves/winners/drops/MemesWaveWinnerDrop.tsx
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
__tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsx (1)
45-71: ⚡ Quick winTest doesn't exercise the real unmount scenario — "Copied!" visibility is unverifiable with a plain mock.
onCopyisjest.fn()and doesn't unmount the component, so afterclipboardWrite.resolve()the child remains mounted andfindByText("Copied!")succeeds. In production,onCopycallssetIsActive(false)in the parent; with React 18/19 automatic batching,setCopied(true)and the parent'ssetIsActive(false)are processed in the same batch, unmounting the child before "Copied!" ever renders.The test passes today only because it silently ignores this teardown, and the title "closing the menu" is therefore misleading — the menu is never actually closed in the test.
Consider adding a variant that wraps the component in a stateful parent and passes
() => setMounted(false)asonCopy, then asserts:
- "Copied!" is visible before the component is torn down (requires moving
onCopy()into thesetTimeout).- The component is subsequently removed from the DOM after 2 s.
This also acts as a regression guard for the component-level fix above.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsx` around lines 45 - 71, Add a new test variant that actually unmounts the menu to mirror production teardown: render WaveDropMobileMenuCopyLink inside a small stateful parent (e.g., mounted state + conditional render), pass an onCopy handler that setsMounted(false) (callable synchronously or via setTimeout to emulate the component change if you also move the component's onCopy call into a setTimeout), trigger the copy (using createDeferredClipboardWrite as in the existing test), resolve the deferred clipboard promise, assert that "Copied!" appears before the parent unmounts, then assert the component is removed from the DOM after the expected delay (~2s) to guard regressing the component-level timing bug involving setCopied and parent setIsActive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/waves/drops/WaveDropMobileMenuCopyLink.tsx`:
- Around line 81-101: The current clipboard.writeText().then() calls
setCopied(true) and onCopy() together causing React to batch and unmount the
component before the "Copied!" state can render; update the logic inside the
.then() so that you still setCopied(true), clear any existing
copiedResetTimeoutRef, and then set copiedResetTimeoutRef to a
globalThis.setTimeout that after 2000ms first setsCopied(false) (guarded by
isMountedRef.current) and only then calls onCopy() to dismiss the menu; keep the
.catch() behavior unchanged and preserve the isMountedRef checks and
clearTimeout call to avoid leaks.
---
Nitpick comments:
In `@__tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsx`:
- Around line 45-71: Add a new test variant that actually unmounts the menu to
mirror production teardown: render WaveDropMobileMenuCopyLink inside a small
stateful parent (e.g., mounted state + conditional render), pass an onCopy
handler that setsMounted(false) (callable synchronously or via setTimeout to
emulate the component change if you also move the component's onCopy call into a
setTimeout), trigger the copy (using createDeferredClipboardWrite as in the
existing test), resolve the deferred clipboard promise, assert that "Copied!"
appears before the parent unmounts, then assert the component is removed from
the DOM after the expected delay (~2s) to guard regressing the component-level
timing bug involving setCopied and parent setIsActive.
🪄 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: 87d9b582-9583-4e87-8503-5523d4a54b3a
📒 Files selected for processing (2)
__tests__/components/waves/drops/WaveDropMobileMenuCopyLink.test.tsxcomponents/waves/drops/WaveDropMobileMenuCopyLink.tsx
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
components/waves/winners/drops/DefaultWaveWinnerDrop.tsx (1)
216-219: ⚡ Quick winUse the shared close handler for
onCopyto avoid close-path drift.At Line 218, use
handleMobileMenuCloseinstead of directsetIsActive(false)so all menu close paths stay centralized.Suggested diff
<WaveDropMobileMenuCopyLink drop={extendedDrop} - onCopy={() => setIsActive(false)} + onCopy={handleMobileMenuClose} />🤖 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 `@components/waves/winners/drops/DefaultWaveWinnerDrop.tsx` around lines 216 - 219, Replace the direct state call in the WaveDropMobileMenuCopyLink onCopy handler with the shared close handler to centralize menu shutdown logic: change the onCopy prop from using setIsActive(false) to calling the existing handleMobileMenuClose function so WaveDropMobileMenuCopyLink uses handleMobileMenuClose (not setIsActive) to close the mobile menu.components/waves/small-leaderboard/DefaultWaveSmallLeaderboardDrop.tsx (1)
28-28: ⚡ Quick winHarden the top-three rank predicate to positive bounds.
Line 28 currently accepts
0/negative ranks as “top three”. Consider constraining to1..3to avoid invalid ranking UI states (and apply the same predicate inMemesWaveSmallLeaderboardDrop.tsx, Line 25).♻️ Proposed fix
- {!isApproveWave && typeof drop.rank === "number" && drop.rank <= 3 ? ( + {!isApproveWave && + typeof drop.rank === "number" && + drop.rank >= 1 && + drop.rank <= 3 ? (🤖 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 `@components/waves/small-leaderboard/DefaultWaveSmallLeaderboardDrop.tsx` at line 28, The top-three check currently treats zero/negative ranks as valid; update the predicate in DefaultWaveSmallLeaderboardDrop (where the JSX uses {!isApproveWave && typeof drop.rank === "number" && drop.rank <= 3}) to require drop.rank >= 1 && drop.rank <= 3 instead, and make the identical change in MemesWaveSmallLeaderboardDrop (the analogous conditional at Line 25) so only ranks 1 through 3 render the top-three UI; keep the typeof drop.rank === "number" and isApproveWave guard intact.
🤖 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.
Nitpick comments:
In `@components/waves/small-leaderboard/DefaultWaveSmallLeaderboardDrop.tsx`:
- Line 28: The top-three check currently treats zero/negative ranks as valid;
update the predicate in DefaultWaveSmallLeaderboardDrop (where the JSX uses
{!isApproveWave && typeof drop.rank === "number" && drop.rank <= 3}) to require
drop.rank >= 1 && drop.rank <= 3 instead, and make the identical change in
MemesWaveSmallLeaderboardDrop (the analogous conditional at Line 25) so only
ranks 1 through 3 render the top-three UI; keep the typeof drop.rank ===
"number" and isApproveWave guard intact.
In `@components/waves/winners/drops/DefaultWaveWinnerDrop.tsx`:
- Around line 216-219: Replace the direct state call in the
WaveDropMobileMenuCopyLink onCopy handler with the shared close handler to
centralize menu shutdown logic: change the onCopy prop from using
setIsActive(false) to calling the existing handleMobileMenuClose function so
WaveDropMobileMenuCopyLink uses handleMobileMenuClose (not setIsActive) to close
the mobile menu.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d528e938-ed1f-4365-ac7c-96bbb2454185
📒 Files selected for processing (22)
__tests__/components/brain/my-stream/MyStreamWaveLeaderboard.test.tsx__tests__/components/brain/my-stream/votes/MyStreamWaveMyVotes.test.tsx__tests__/components/waves/leaderboard/drops/DefaultWaveLeaderboardDrop.interaction.test.tsx__tests__/components/waves/leaderboard/grid/WaveLeaderboardGridItem.test.tsx__tests__/components/waves/small-leaderboard/DefaultWaveSmallLeaderboardDrop.test.tsx__tests__/components/waves/small-leaderboard/MemesWaveSmallLeaderboardDrop.test.tsx__tests__/components/waves/small-leaderboard/WaveSmallLeaderboardDefaultDrop.test.tsx__tests__/hooks/waves/useApprovalWaveStatus.test.tscomponents/brain/my-stream/MyStreamWaveLeaderboard.tsxcomponents/brain/my-stream/votes/MyStreamWaveMyVotes.tsxcomponents/waves/create-wave/services/createWaveDropRequest.tscomponents/waves/drops/WaveDropMobileMenu.tsxcomponents/waves/leaderboard/drops/DefaultWaveLeaderboardDrop.tsxcomponents/waves/leaderboard/grid/WaveLeaderboardGridItem.tsxcomponents/waves/leaderboard/leaderboardRendererRegistry.tsxcomponents/waves/small-leaderboard/DefaultWaveSmallLeaderboardDrop.tsxcomponents/waves/small-leaderboard/MemesWaveSmallLeaderboardDrop.tsxcomponents/waves/small-leaderboard/QuorumWaveSmallLeaderboardDrop.tsxcomponents/waves/small-leaderboard/WaveSmallLeaderboardDefaultDrop.tsxcomponents/waves/small-leaderboard/WaveSmallLeaderboardDrop.tsxcomponents/waves/winners/drops/DefaultWaveWinnerDrop.tsxcomponents/waves/winners/drops/MemesWaveWinnerDrop.tsx
💤 Files with no reviewable changes (1)
- components/brain/my-stream/MyStreamWaveLeaderboard.tsx
✅ Files skipped from review due to trivial changes (1)
- components/waves/leaderboard/drops/DefaultWaveLeaderboardDrop.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/components/waves/leaderboard/drops/DefaultWaveLeaderboardDrop.interaction.test.tsx
- components/waves/winners/drops/MemesWaveWinnerDrop.tsx
- tests/components/waves/leaderboard/grid/WaveLeaderboardGridItem.test.tsx
- components/waves/drops/WaveDropMobileMenu.tsx
- components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx
ragnep
approved these changes
May 5, 2026
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
2 participants
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
Tests