Conversation
📝 WalkthroughWalkthroughAdds a per-wave view mode (chat/gallery) with persistent cross-tab storage, a new gallery UI with infinite-scroll gallery items, and propagates view-mode and drop-click handlers through my-stream wave components; updates tests and mocks accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Tabs as MyStreamWaveTabs
participant Wave as MyStreamWave
participant Chat as MyStreamWaveChat
participant Gallery as WaveGallery
participant Store as useWaveViewMode
User->>Tabs: Click view toggle
Tabs->>Wave: onToggleViewMode()
Wave->>Store: toggleViewMode(waveId)
Store->>Store: update localStorage & notify subscribers
Store-->>Wave: new viewMode
Wave->>Chat: render with viewMode
alt viewMode == "gallery"
Chat->>Gallery: render WaveGallery (fetch via useWaveGalleryDrops)
Gallery-->>User: display grid & infinite scroll
else
Chat->>Chat: render WaveDropsAll (chat view)
Chat-->>User: display threaded drops
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/worktree/wt-common.sh (1)
28-103: Replacesed -i ''with a portable variant.
sed -i ''is BSD-specific and fails on GNU sed (Linux). Usesed -i.bakfollowed by cleanup to ensure portability across both macOS and Linux:- sed -i '' "s/__COLOR__/$color/g" "$settings_file" + sed -i.bak "s/__COLOR__/$color/g" "$settings_file" && rm -f "$settings_file.bak"
🤖 Fix all issues with AI agents
In `@__tests__/components/brain/my-stream/MyStreamWaveChat.test.tsx`:
- Around line 8-15: The test mock for useSearchParams is missing a toString
implementation causing a runtime error when MyStreamWaveChat calls
searchParams.toString(); update the mock object searchParamsMock used in the
jest.mock block so it includes a toString method (e.g., toString: jest.fn(() =>
"")) alongside the existing get mock; this ensures calls in MyStreamWaveChat
(searchParams.toString()) return a string and prevent the runtime failure.
In `@components/waves/gallery/WaveGalleryItem.tsx`:
- Around line 50-87: The button in WaveGalleryItem lacks visible keyboard focus;
update the button's className (the <button> in WaveGalleryItem) to include
focus-visible classes such as focus-visible:tw-ring-2 and
focus-visible:tw-ring-primary-500 (and optionally focus-visible:tw-outline-none)
to show a clear focus ring, and update the overlay elements that currently use
group-hover: (the two divs with group-hover:tw-opacity-100 and
group-hover:tw-translate-y-0) to also include group-focus-visible: variants
(e.g. group-focus-visible:tw-opacity-100 and
group-focus-visible:tw-translate-y-0) so keyboard focus on the button triggers
the same visual overlays.
In `@hooks/useWaveGalleryDrops.ts`:
- Around line 100-137: The debounce logic in onRefetch can drop a scheduled
refetch because when timeSinceLastRefetch < minDebounceTime and
pendingRefetchRef.current is already true you clear the existing timeout and
don't schedule a new one; change it to always (re)schedule the timeout: ensure
pendingRefetchRef.current = true, clear any existing timeoutRef.current, then
assign timeoutRef.current = setTimeout(...) to reschedule the deferred call
(keeping the inner guard that checks isFetchingRef.current /
isFetchingNextPageRef.current before calling executeRefetch); leave
executeRefetch and the fetch guards intact.
🧹 Nitpick comments (3)
components/waves/gallery/WaveGalleryItem.tsx (1)
36-38: Consider memoizing the click handler.
handleImageClickcreates a new function on every render, which could cause unnecessary re-renders of child components if passed as a prop. Since this component is memoized, consider usinguseCallback.♻️ Suggested improvement
+import { memo, useMemo, useCallback } from "react"; -import { memo, useMemo } from "react"; - const handleImageClick = () => { - onDropClick(drop); - }; + const handleImageClick = useCallback(() => { + onDropClick(drop); + }, [onDropClick, drop]);hooks/useWaveViewMode.ts (1)
14-19: Module-level mutable state may cause test isolation issues.These module-level variables (
memoryModes,useMemoryStore,cachedModes,cachedRaw) persist across test runs, potentially causing flaky tests. Consider exposing a reset function for testing or using a more testable pattern.♻️ Suggested improvement for test isolation
+// Export for testing purposes only +export const __resetForTesting = () => { + memoryModes = {}; + useMemoryStore = false; + cachedModes = EMPTY_MODES; + cachedRaw = null; +};__tests__/components/brain/my-stream/tabs/MyStreamWaveTabsDefault.test.tsx (1)
43-78: Test covers prop passing but doesn't verify toggle behavior.The test passes the new props (
viewMode,onToggleViewMode,showGalleryToggle) but doesn't verify that clicking the toggle button callsmockToggleViewMode. Consider adding a test case to verify the toggle functionality.💚 Suggested test for toggle behavior
it("calls onToggleViewMode when toggle button is clicked", () => { const setActiveContentTab = jest.fn(); useContentTab.mockReturnValue({ activeContentTab: "CHAT", setActiveContentTab, }); const wave = { id: "w1", name: "Wave", picture: null, contributors_overview: [], } as any; render( <SidebarProvider> <MyStreamWaveTabsDefault wave={wave} viewMode="chat" onToggleViewMode={mockToggleViewMode} showGalleryToggle={true} /> </SidebarProvider> ); const toggleButton = screen.getByRole("button", { name: /switch to gallery view/i }); fireEvent.click(toggleButton); expect(mockToggleViewMode).toHaveBeenCalledTimes(1); });
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@components/home/next-mint-leading/NextMintLeadingSection.tsx`:
- Around line 41-47: The comparison logic for isNextMintSameAsNowMinting can
erroneously treat two undefined values as equal and hide the NextMintCard;
update the check in NextMintLeadingSection to only compare when both values are
present (e.g., ensure nowMinting?.name and nextMintTitle are non-null/defined
before doing toLowerCase().trim()), otherwise treat them as not equal, and keep
showNextMint = nextMint && !isNextMintSameAsNowMinting so the card is shown
during loading or missing titles.
In `@components/home/now-minting/NowMintingHeader.tsx`:
- Around line 88-91: The map over artistHandles uses the raw handle string as
the React key which can duplicate if the same handle appears multiple times;
update the mapping in the NowMintingHeader where artistHandles.map(...) renders
NowMintingArtistHandlePill to use a stable unique key per item (e.g., combine
handle with the map index or use the index as a fallback: `${handle}-${index}`
or index alone) so keys are guaranteed unique and React warnings are avoided.
🧹 Nitpick comments (1)
components/home/now-minting/NowMintingHeader.tsx (1)
41-43: Consider using a safer type forArtistProfileHandle.The
as BaseNFTcast on a partial object is fragile. IfArtistProfileHandleaccesses otherBaseNFTproperties beyondartist_seize_handle, this could cause runtime issues or undefined behavior.Consider either:
- Creating a minimal interface/type that
ArtistProfileHandleactually requires- Updating
ArtistProfileHandleto accept just the handle string directly if that's all it needsExample approach
// If ArtistProfileHandle only needs the handle, consider updating its props // or create a Pick type: type ArtistHandleNFT = Pick<BaseNFT, 'artist_seize_handle'>;
|



Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.