Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughConnection-state gating was added across UI and hooks: disconnected users are prevented from voting, reacting, following, messaging, or accessing My Votes on memes waves. Public-wave shell and its hook were removed; wave layout/sidebar/drop-overlay props were simplified. Numerous tests updated/added to cover disconnected behavior. Changes
Sequence DiagramsequenceDiagram
participant User
participant Component
participant SeizeCtx as useSeizeConnectContext
participant AuthCtx as useAuth / Profile
participant API
User->>Component: attempts action (vote/react/follow/chat)
Component->>SeizeCtx: read isConnected
Component->>AuthCtx: read connectedProfile
alt isConnected = true and connectedProfile exists
SeizeCtx-->>Component: {isConnected: true}
AuthCtx-->>Component: connectedProfile
Component->>API: perform action (POST/DELETE/etc.)
API-->>Component: response
Component-->>User: interactive UI updated
else disconnected or no profile
SeizeCtx-->>Component: {isConnected: false}
AuthCtx-->>Component: null
Component->>Component: render hidden/disabled controls (or copy-link only)
Component-->>User: no-op / read-only UI
end
Note over Component,API: On unmount, cleanup runs only if connectedProfile exists
Component->>AuthCtx: check connectedProfile before cleanup
alt connectedProfile exists
Component->>API: mark wave notifications read / remove delivered
else
Component-->>API: skip cleanup
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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 docstrings
🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baff6caf67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/waves/drops/WaveDropReactions.tsx (1)
413-425:⚠️ Potential issue | 🟡 MinorThe button signals "disabled" via
aria-disabledbut long-press still works when disconnected.At line 415,
aria-disabled={!canReact}announces the button as disabled to assistive technologies, but the element remains focusable and interactive. WhilehandleClick(line 414) has a guard that prevents mutations when!canReact, the touch handlers (line 425) lack this check. This means disconnected users can still triggerhandleLongPressStartand open the detail dialog. Either fully disable the button with thedisabledattribute and conditionally exclude tooltip and touch wiring, or removearia-disabledif the button should remain accessible for long-press interactions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drops/WaveDropReactions.tsx` around lines 413 - 425, Replace the current aria-disabled pattern with a real disabled attribute and prevent touch/tooltip wiring when reactions are not allowed: change aria-disabled={!canReact} to disabled={!canReact} (and remove aria-disabled), only spread {"data-tooltip-id": tooltipId} and {...wrappedTouchHandlers} when canReact is true, and keep the existing guard in handleClick; this ensures handleLongPressStart and tooltips are not reachable when canReact is false while preserving behavior when enabled (references: handleClick, handleLongPressStart, wrappedTouchHandlers, tooltipId, canReact).components/waves/DropPlaceholder.tsx (1)
41-60:⚠️ Potential issue | 🔴 CriticalHandle
SubmissionRestriction.DISABLEDexplicitly to avoid runtime errors.Line 56 routes unhandled submission restrictions to
throwUnhandledRestriction, butSubmissionRestriction.DISABLEDis a valid state and currently falls into this path, which will throw at runtime.🐛 Proposed fix
if (type === "submission" && submissionRestriction) { switch (submissionRestriction) { case SubmissionRestriction.NOT_LOGGED_IN: return "Please log in to make submissions"; case SubmissionRestriction.PROXY_USER: return "Proxy users cannot make submissions"; case SubmissionRestriction.NO_PERMISSION: return "You don't have permission to submit in this wave"; + case SubmissionRestriction.DISABLED: + return "Submissions are currently disabled for this wave"; case SubmissionRestriction.NOT_STARTED: return "Submissions haven't started yet"; case SubmissionRestriction.ENDED: return "Submission period has ended"; case SubmissionRestriction.MAX_DROPS_REACHED: return "You have reached the maximum number of drops allowed";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/DropPlaceholder.tsx` around lines 41 - 60, The switch in DropPlaceholder.tsx handling submissionRestriction treats SubmissionRestriction.DISABLED as unhandled and calls throwUnhandledRestriction; add an explicit case for SubmissionRestriction.DISABLED in the switch (the block that checks type === "submission") and return an appropriate user-facing message (e.g., submissions are disabled) instead of falling through to throwUnhandledRestriction, keeping references to the submissionRestriction variable, SubmissionRestriction enum, and throwUnhandledRestriction for context.
🧹 Nitpick comments (5)
__tests__/components/waves/drops/wave-drops-all/hooks/useWaveDropsNotificationRead.test.tsx (1)
37-45: Consider usingjest.clearAllMocks()for simpler cleanup.The individual
mockClear()calls work correctly, butjest.clearAllMocks()would be more concise and automatically handle any future mocks added to this test suite.♻️ Suggested simplification
beforeEach(() => { - invalidateNotifications.mockClear(); - removeWaveDeliveredNotifications.mockClear(); - ( - commonApiPostWithoutBodyAndResponse as jest.MockedFunction< - typeof commonApiPostWithoutBodyAndResponse - > - ).mockClear(); + jest.clearAllMocks(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/drops/wave-drops-all/hooks/useWaveDropsNotificationRead.test.tsx` around lines 37 - 45, Replace the explicit mockClear calls in the beforeEach block with a single jest.clearAllMocks() call: locate the beforeEach that currently calls invalidateNotifications.mockClear(), removeWaveDeliveredNotifications.mockClear(), and commonApiPostWithoutBodyAndResponse.mockClear() and change it to call jest.clearAllMocks() so all mocks (including future ones) are reset automatically.__tests__/components/waves/WavesLayout.test.tsx (1)
91-100: Consider resetting mocks after each test.Using
jest.resetAllMocks()orjest.clearAllMocks()inafterEachensures mock state doesn't leak between tests, improving test isolation.♻️ Proposed improvement
beforeEach(() => { + jest.clearAllMocks(); mockUseAuthenticatedContent.mockReturnValue({ contentState: "not-authenticated", });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/WavesLayout.test.tsx` around lines 91 - 100, The tests for WavesLayout are missing teardown to reset mock state; add an afterEach hook in the describe("WavesLayout") block that calls jest.clearAllMocks() or jest.resetAllMocks() to prevent mock leakage between tests—apply this to the mocked helpers used in beforeEach (mockUseAuthenticatedContent, mockUseDeviceInfo, mockUsePathname, mockUseSearchParams, mockGetActiveWaveIdFromUrl) so each test starts with clean mock state.components/brain/my-stream/MyStreamWaveDesktopTabs.tsx (1)
140-147: Keep tab eligibility in one place.Line 140 already feeds
isConnectedintoupdateAvailableTabs, but Line 177 re-filtersMY_VOTES/SALES/FAQlocally. That gives two sources of truth for tab availability and makes the fallback logic easy to drift on the next rule change. Consider extracting a shared predicate or trustingavailableTabsas the single input here.Also applies to: 176-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/MyStreamWaveDesktopTabs.tsx` around lines 140 - 147, The code duplicates tab eligibility logic by re-filtering MY_VOTES/SALES/FAQ after calling updateAvailableTabs; instead centralize the predicate by either (A) trusting the availableTabs returned from updateAvailableTabs and using that array for rendering/conditional checks, or (B) extract a shared predicate function (e.g., isTabEligible(tab, {wave, isConnected, isMemesWave, isChatWave, isCurationWave})) and use it both inside updateAvailableTabs and where lines 176-193 currently re-filter; update references to availableTabs, MY_VOTES, SALES, and FAQ to use the single source (availableTabs or the shared predicate) so eligibility logic lives in one place.__tests__/components/brain/ContentTabContext.test.tsx (1)
64-83: Consider asserting active-tab fallback in the disconnected memes case.This test validates
availableTabs, but not the selected-tab recovery path. Add an assertion (or a dedicated test) foractiveContentTabfallback whenMY_VOTESbecomes unavailable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/brain/ContentTabContext.test.tsx` around lines 64 - 83, The test currently checks availableTabs after calling result.current.updateAvailableTabs but doesn't assert that the selected tab recovers when MY_VOTES is removed; update the test (or add a new one) to call result.current.updateAvailableTabs with isConnected: false and isMemesWave: true and then assert result.current.activeContentTab has fallen back from MyStreamWaveTab.MY_VOTES to the expected fallback (e.g., MyStreamWaveTab.LEADERBOARD or another configured default). Use the existing test helpers and reference updateAvailableTabs, availableTabs and activeContentTab to locate where to add the assertion.components/waves/drops/WaveDropMobileMenu.tsx (1)
145-178: Deduplicate the repeated “Copy link” button block.Line 145–178 and Line 243–275 duplicate the same JSX/logic. Extracting a shared renderer will reduce drift risk.
♻️ Proposed refactor
+ const renderCopyLinkButton = () => ( + <button + className={`tw-flex tw-items-center tw-gap-x-4 tw-rounded-xl tw-border-0 tw-bg-iron-950 tw-p-4 ${ + isTemporaryDrop + ? "tw-cursor-default tw-opacity-50" + : "active:tw-bg-iron-800" + } tw-transition-colors tw-duration-200`} + onClick={copyToClipboard} + disabled={isTemporaryDrop} + > + <svg + className="tw-h-5 tw-w-5 tw-flex-shrink-0 tw-text-iron-300" + xmlns="http://www.w3.org/2000/svg" + fill="none" + viewBox="0 0 24 24" + strokeWidth="1.5" + stroke="currentColor" + > + <path + strokeLinecap="round" + strokeLinejoin="round" + d="M13.19 8.688a4.5 4.5 0 011.242 7.244l-4.5 4.5a4.5 4.5 0 01-6.364-6.364l1.757-1.757m13.35-.622l1.757-1.757a4.5 4.5 0 00-6.364-6.364l-4.5 4.5a4.5 4.5 0 001.242 7.244" + /> + </svg> + <span + className={`tw-text-base tw-font-semibold ${ + copied ? "tw-text-primary-400" : "tw-text-iron-300" + }`} + > + {copied ? "Copied!" : "Copy link"} + </span> + </button> + ); ... - {!isConnected ? ( - showCopyOption && ( - <button>...</button> - ) - ) : ( + {!isConnected ? ( + showCopyOption && renderCopyLinkButton() + ) : ( <> ... - {showCopyOption && ( - <button>...</button> - )} + {showCopyOption && renderCopyLinkButton()} ... </> )}Also applies to: 243-275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drops/WaveDropMobileMenu.tsx` around lines 145 - 178, The JSX for the "Copy link" button is duplicated; extract it into a single shared renderer (either a small PureComponent/functional component named e.g. CopyLinkButton or a renderCopyButton helper) and use it in both places in WaveDropMobileMenu.tsx. The extracted component should accept props/closures for copyToClipboard, isTemporaryDrop, copied and showCopyOption (or read them from scope if kept in the same component) and reproduce the current className, disabled logic and label ({copied ? "Copied!" : "Copy link"}); replace the duplicated blocks (the two areas using copyToClipboard/isTemporaryDrop/copy state) with the new shared renderer to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/components/waves/WavesLayout.test.tsx`:
- Around line 102-121: The test is asserting obsolete attributes on the mocked
WavesDesktop; update the "renders the selected wave content for logged-out
users" test in WavesLayout.test.tsx by removing the two expectations that check
data-allow-drop-overlay and data-allow-right-sidebar (or replace them with
assertions that match the new WavesDesktop behavior), keeping the existing
checks for mockGetActiveWaveIdFromUrl, the rendered child ("wave-content"), and
that "wave-screen-message" is not present; ensure the test references the
WavesLayout render and the waves-desktop test id but does not expect the removed
props/attributes.
- Around line 30-46: The WavesDesktop mock in the test only provides data-testid
and data-show-left-sidebar but the test still asserts on removed attributes
data-allow-drop-overlay and data-allow-right-sidebar; either update the mock to
include those attributes or (preferred) remove the invalid assertions around
allowDropOverlay/allowRightSidebar in the test (assertions at the block
referencing lines ~112-119) so the test matches the updated
WavesDesktop/WavesMessagesWrapper behavior.
In `@components/brain/BrainMobile.tsx`:
- Around line 48-49: The mount guard for the mobile quick-vote entry must
include the wallet connection check: update the condition that computes
shouldMountFloatingQuickVoteEntry (used by useBrainMobileActiveView and the
floating entry render) to also require isConnected from useSeizeConnectContext
(i.e., add && isConnected), and/or add a connection gate inside
useMemesWaveFooterStats so its isAvailable returns false unless isConnected is
true; ensure references to shouldMountFloatingQuickVoteEntry,
useMemesWaveFooterStats, useSeizeConnectContext/isConnected and the quickVote
controller remain consistent after the change.
In `@components/waves/drop/SingleWaveDropLog.tsx`:
- Around line 117-119: The className concatenation in SingleWaveDropLog.tsx
produces "tw-inline-blocktw-..." when shouldLimit is true; update the template
to include a separating space—e.g., change className={`tw-inline-block${
shouldLimit ? " tw-max-w-[8rem] tw-truncate" : "" }`} or move the space into the
base string—to ensure a space between "tw-inline-block" and the conditional
classes when shouldLimit is true.
In `@docs/waves/chat/feature-chat-composer-availability.md`:
- Around line 25-29: Update the "limitations" note that currently says
“messaging is generic” so it reflects the two distinct footer states: when the
viewer is logged out and both modes are blocked the footer reads “Connect your
wallet to participate in this wave”, and when both modes are blocked for other
reasons the footer reads “You cannot participate in this wave at the moment”;
locate the sentence that references “messaging is generic” and replace it with a
concise explanation of these two cases and their exact footer copy.
---
Outside diff comments:
In `@components/waves/DropPlaceholder.tsx`:
- Around line 41-60: The switch in DropPlaceholder.tsx handling
submissionRestriction treats SubmissionRestriction.DISABLED as unhandled and
calls throwUnhandledRestriction; add an explicit case for
SubmissionRestriction.DISABLED in the switch (the block that checks type ===
"submission") and return an appropriate user-facing message (e.g., submissions
are disabled) instead of falling through to throwUnhandledRestriction, keeping
references to the submissionRestriction variable, SubmissionRestriction enum,
and throwUnhandledRestriction for context.
In `@components/waves/drops/WaveDropReactions.tsx`:
- Around line 413-425: Replace the current aria-disabled pattern with a real
disabled attribute and prevent touch/tooltip wiring when reactions are not
allowed: change aria-disabled={!canReact} to disabled={!canReact} (and remove
aria-disabled), only spread {"data-tooltip-id": tooltipId} and
{...wrappedTouchHandlers} when canReact is true, and keep the existing guard in
handleClick; this ensures handleLongPressStart and tooltips are not reachable
when canReact is false while preserving behavior when enabled (references:
handleClick, handleLongPressStart, wrappedTouchHandlers, tooltipId, canReact).
---
Nitpick comments:
In `@__tests__/components/brain/ContentTabContext.test.tsx`:
- Around line 64-83: The test currently checks availableTabs after calling
result.current.updateAvailableTabs but doesn't assert that the selected tab
recovers when MY_VOTES is removed; update the test (or add a new one) to call
result.current.updateAvailableTabs with isConnected: false and isMemesWave: true
and then assert result.current.activeContentTab has fallen back from
MyStreamWaveTab.MY_VOTES to the expected fallback (e.g.,
MyStreamWaveTab.LEADERBOARD or another configured default). Use the existing
test helpers and reference updateAvailableTabs, availableTabs and
activeContentTab to locate where to add the assertion.
In
`@__tests__/components/waves/drops/wave-drops-all/hooks/useWaveDropsNotificationRead.test.tsx`:
- Around line 37-45: Replace the explicit mockClear calls in the beforeEach
block with a single jest.clearAllMocks() call: locate the beforeEach that
currently calls invalidateNotifications.mockClear(),
removeWaveDeliveredNotifications.mockClear(), and
commonApiPostWithoutBodyAndResponse.mockClear() and change it to call
jest.clearAllMocks() so all mocks (including future ones) are reset
automatically.
In `@__tests__/components/waves/WavesLayout.test.tsx`:
- Around line 91-100: The tests for WavesLayout are missing teardown to reset
mock state; add an afterEach hook in the describe("WavesLayout") block that
calls jest.clearAllMocks() or jest.resetAllMocks() to prevent mock leakage
between tests—apply this to the mocked helpers used in beforeEach
(mockUseAuthenticatedContent, mockUseDeviceInfo, mockUsePathname,
mockUseSearchParams, mockGetActiveWaveIdFromUrl) so each test starts with clean
mock state.
In `@components/brain/my-stream/MyStreamWaveDesktopTabs.tsx`:
- Around line 140-147: The code duplicates tab eligibility logic by re-filtering
MY_VOTES/SALES/FAQ after calling updateAvailableTabs; instead centralize the
predicate by either (A) trusting the availableTabs returned from
updateAvailableTabs and using that array for rendering/conditional checks, or
(B) extract a shared predicate function (e.g., isTabEligible(tab, {wave,
isConnected, isMemesWave, isChatWave, isCurationWave})) and use it both inside
updateAvailableTabs and where lines 176-193 currently re-filter; update
references to availableTabs, MY_VOTES, SALES, and FAQ to use the single source
(availableTabs or the shared predicate) so eligibility logic lives in one place.
In `@components/waves/drops/WaveDropMobileMenu.tsx`:
- Around line 145-178: The JSX for the "Copy link" button is duplicated; extract
it into a single shared renderer (either a small PureComponent/functional
component named e.g. CopyLinkButton or a renderCopyButton helper) and use it in
both places in WaveDropMobileMenu.tsx. The extracted component should accept
props/closures for copyToClipboard, isTemporaryDrop, copied and showCopyOption
(or read them from scope if kept in the same component) and reproduce the
current className, disabled logic and label ({copied ? "Copied!" : "Copy
link"}); replace the duplicated blocks (the two areas using
copyToClipboard/isTemporaryDrop/copy state) with the new shared renderer to
avoid drift.
🪄 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: 36f77526-6066-4a88-b679-498e17b22cb9
📒 Files selected for processing (35)
__tests__/components/brain/BrainMobile.test.tsx__tests__/components/brain/ContentTabContext.test.tsx__tests__/components/brain/mobile/BrainMobileTabs.test.tsx__tests__/components/brain/my-stream/MyStreamWaveChat.test.tsx__tests__/components/brain/my-stream/MyStreamWaveDesktopTabs.test.tsx__tests__/components/user/utils/profile/UserProfileTooltip.test.tsx__tests__/components/waves/DropPlaceholder.test.tsx__tests__/components/waves/WavesLayout.test.tsx__tests__/components/waves/drops/WaveDropActions.test.tsx__tests__/components/waves/drops/WaveDropMobileMenu.test.tsx__tests__/components/waves/drops/WaveDropReactions.test.tsx__tests__/components/waves/drops/wave-drops-all/hooks/useWaveDropsNotificationRead.test.tsxcomponents/brain/BrainMobile.tsxcomponents/brain/ContentTabContext.tsxcomponents/brain/mobile/BrainMobileTabs.tsxcomponents/brain/mobile/useBrainMobileActiveView.tscomponents/brain/my-stream/MyStreamWaveChat.tsxcomponents/brain/my-stream/MyStreamWaveDesktopTabs.tsxcomponents/shared/WavesMessagesWrapper.tsxcomponents/user/utils/profile/UserProfileTooltip.tsxcomponents/waves/DropPlaceholder.tsxcomponents/waves/WavesDesktop.tsxcomponents/waves/drop/SingleWaveDropLog.tsxcomponents/waves/drops/WaveDropActions.tsxcomponents/waves/drops/WaveDropMobileMenu.tsxcomponents/waves/drops/WaveDropReactions.tsxcomponents/waves/drops/wave-drops-all/hooks/useWaveDropsNotificationRead.tscomponents/waves/drops/wave-drops-all/index.tsxcomponents/waves/layout/WavesLayout.tsxcomponents/waves/public/LoggedOutSkeleton.tsxcomponents/waves/public/PublicWaveShell.tsxcomponents/waves/public/usePublicWaveShellState.tsdocs/waves/chat/feature-chat-composer-availability.mddocs/waves/flow-wave-participation.mddocs/waves/troubleshooting-wave-navigation-and-posting.md
💤 Files with no reviewable changes (4)
- components/waves/public/LoggedOutSkeleton.tsx
- components/waves/public/usePublicWaveShellState.ts
- components/waves/public/PublicWaveShell.tsx
- components/waves/WavesDesktop.tsx
|

Summary by CodeRabbit
New Features
Documentation