Conversation
Signed-off-by: ragnep <ragneinfo@gmail.com>
📝 WalkthroughWalkthroughThis PR introduces a community-curated drops discovery feature with media-type filtering and infinite pagination, refactors TypeScript imports across 20+ files to use dedicated generated model modules instead of ObjectSerializer, updates the wave discovery UI to display community curations when no wave is selected (replacing WaveScreenMessage), and applies layout sizing adjustments (tw-min-h-0) for proper flex container behavior. Changes
Sequence DiagramsequenceDiagram
participant User
participant CommunityCurations as CommunityCurations<br/>Component
participant Hook as useCommunityCurations<br/>Drops Hook
participant API as API Server
participant Masonry as CommunityCurations<br/>Masonry
participant Observer as Intersection<br/>Observer
User->>CommunityCurations: Mount with mediaFilter
CommunityCurations->>Hook: Call with limit & mediaFilter
Hook->>API: Fetch drops (limit=12, curation_name=ART)
API-->>Hook: Return page 1 drops
Hook-->>CommunityCurations: allDrops, drops[], hasNextPage
CommunityCurations->>Masonry: Render with drops & scrollContainer
Masonry->>Masonry: Measure grid width (ResizeObserver)
Masonry->>Masonry: Track scroll (requestAnimationFrame)
Masonry->>Observer: Set up intersection trigger at bottom
User->>Masonry: Scroll down
Observer->>Masonry: Intersection detected
Masonry->>Hook: Call fetchNextPage()
Hook->>API: Fetch drops (serial_no_less_than=X)
API-->>Hook: Return page 2 drops
Hook-->>Masonry: Update drops with deduped results
Masonry->>Masonry: Reposition items
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 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)
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: 3
🤖 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/art-feed/artFeed.helpers.ts`:
- Around line 168-179: The current getArtFeedMediaType only uses MIME type and
then defaults to "other", which misses URL-derivable types; update
getArtFeedMediaType so after locating media (via drop.parts.flatMap(...).at(0))
you first try getMediaTypeFromMimeType(media.mime_type), then fall back to
getMediaTypeFromUrl(media.url) before calling getNftLinkMediaType(drop) and
finally defaulting to "other"; keep the existing early
getDropPreviewImageUrl(metadata) check intact.
In `@components/art-feed/ArtFeed.tsx`:
- Around line 120-149: The media and audience option buttons are missing
accessible pressed state; update the button elements generated from
MEDIA_FILTER_OPTIONS and AUDIENCE_OPTIONS to expose active state by adding
aria-pressed={mediaFilter === option.key} for the media buttons and
aria-pressed={audience === option.key} for the audience buttons (keep existing
onClick handlers setMediaFilter/setAudience and className logic via
getTabButtonClassName); ensure the buttons remain type="button" and include an
appropriate aria-label or use the existing option.label as visible text so
screen readers can announce them.
In `@hooks/useArtFeedDrops.ts`:
- Around line 97-105: The current useMemo filter (drops) only filters
already-fetched pages (allDrops) using matchesMediaFilter, mediaFilter, audience
and isDropFromFollowedAuthor, which can incorrectly show an empty ArtFeed when
later pages would match; update the data flow so filtering happens in the query
layer or by auto-fetching: either push mediaFilter and audience into the query
key/params used by the API so the server returns only matching drops (make the
query include mediaFilter and audience and remove client-only filtering in the
useMemo), or implement an auto-fetch/loadMore loop in the hook that, after each
page load, continues fetching additional pages until at least one matching drop
is found or pagination is exhausted (use the same matchesMediaFilter and
isDropFromFollowedAuthor checks to decide whether to continue). Ensure you
update references to drops/useMemo, allDrops, matchesMediaFilter, mediaFilter,
audience and isDropFromFollowedAuthor and adjust ArtFeed.tsx to rely on the
query result rather than client-side-only filtered pages.
🪄 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: f7b11d35-d961-4547-97aa-cddaef06ca12
📒 Files selected for processing (8)
app/art-feed/page.client.tsxapp/art-feed/page.tsxcomponents/art-feed/ArtFeed.tsxcomponents/art-feed/ArtFeedTrigger.tsxcomponents/art-feed/artFeed.constants.tscomponents/art-feed/artFeed.helpers.tscomponents/brain/left-sidebar/waves/MemesWaveFooter.tsxhooks/useArtFeedDrops.ts
Signed-off-by: ragnep <ragneinfo@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/waves/layout/WavesLayout.tsx (1)
82-90:⚠️ Potential issue | 🟠 MajorAuthenticated path doesn't enable main-content-without-wave — inconsistent with the unauthenticated branch.
The unauthenticated branch sets
allowMainContentWithoutWave={activeWaveId === null}, but thecontentState === "ready"branch rendersWavesDesktopwithout this prop. As a result, an authenticated user on a mobile-width viewport (isApp === false, breakpoint"S") with no wave selected will fall intoWavesMessagesWrapper'scanShowMainContent === falsebranch and won't seeCommunityCurations(rendered byWavesView), while a logged-out user in the same scenario will. If the goal is for Community Curations to be the new default landing for everyone without a wave, this branch needs the same prop.🔧 Proposed fix
if (contentState === "ready") { - const Component = isApp ? WavesMobile : WavesDesktop; - content = ( - <div className="tw-flex-1" id="waves-content"> - <Component> - <div className={containerClassName}>{children}</div> - </Component> - </div> - ); + content = ( + <div className="tw-flex-1" id="waves-content"> + {isApp ? ( + <WavesMobile> + <div className={containerClassName}>{children}</div> + </WavesMobile> + ) : ( + <WavesDesktop allowMainContentWithoutWave={activeWaveId === null}> + <div className={containerClassName}>{children}</div> + </WavesDesktop> + )} + </div> + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/layout/WavesLayout.tsx` around lines 82 - 90, In the contentState === "ready" branch add the same allowMainContentWithoutWave prop used in the unauthenticated branch: pass allowMainContentWithoutWave={activeWaveId === null} into the rendered Waves component (both WavesMobile and WavesDesktop) so that the Component = isApp ? WavesMobile : WavesDesktop render receives allowMainContentWithoutWave and preserves the same main-content behavior for authenticated users; update the JSX in the ready block where Component is used to include this prop.
🧹 Nitpick comments (6)
components/brain/left-sidebar/waves/MemesWaveFooter.tsx (1)
92-135: Single-child flex wrapper appears redundant.The new
<div className="tw-flex tw-items-stretch tw-gap-2">wraps only one child (<button>). With a single child,tw-flex tw-items-stretch tw-gap-2has no visible effect, andtw-flex-1on the button is unnecessary since the outertw-w-fullalready makes it span the full width. If this scaffolding is intentional in preparation for adding sibling controls in a follow-up, consider adding a brief comment to signal intent; otherwise, it can be flattened.♻️ Optional flattening if no siblings are planned
- <div className="tw-mt-auto tw-w-full tw-flex-shrink-0 tw-border-0 tw-border-t tw-border-solid tw-border-iron-800/60 tw-bg-black tw-p-4"> - <div className="tw-flex tw-items-stretch tw-gap-2"> - <button - type="button" - aria-label={buttonAriaLabel} - onClick={handleOpenQuickVote} - onFocus={handlePrefetchQuickVote} - onMouseEnter={handlePrefetchQuickVote} - className="tw-group tw-min-w-0 tw-flex-1 tw-cursor-pointer tw-border-0 tw-bg-transparent tw-p-0 tw-text-left" - > + <div className="tw-mt-auto tw-w-full tw-flex-shrink-0 tw-border-0 tw-border-t tw-border-solid tw-border-iron-800/60 tw-bg-black tw-p-4"> + <button + type="button" + aria-label={buttonAriaLabel} + onClick={handleOpenQuickVote} + onFocus={handlePrefetchQuickVote} + onMouseEnter={handlePrefetchQuickVote} + className="tw-group tw-block tw-w-full tw-min-w-0 tw-cursor-pointer tw-border-0 tw-bg-transparent tw-p-0 tw-text-left" + > ... - </button> - </div> + </button> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/left-sidebar/waves/MemesWaveFooter.tsx` around lines 92 - 135, The wrapper div with className "tw-flex tw-items-stretch tw-gap-2" in MemesWaveFooter is redundant because it only contains a single <button> (which also has tw-flex-1 and the parent already has tw-w-full); either remove that wrapper and drop tw-flex-1 from the button, or if you intend to add siblings later, keep the wrapper but add a short comment stating it's reserved for future controls and keep tw-flex-1 only if you want the button to grow; update the JSX around the wrapper div and the button element (remove the extra <div> and the button's tw-flex-1) accordingly to flatten the structure.components/community-curations/CommunityCurations.tsx (1)
81-84: Skeletons disappear when switching filters mid-session.
isInitialLoading = isLoading && allDrops.length === 0only shows skeletons on the very first load. When the user changesmediaFilterand the hook refetches with a different cache key,allDrops.lengthis likely > 0 from the previous filter, so the user sees stale results (or the empty state) flicker before new data arrives. IfuseCommunityCurationsDropsresetsallDropson filter change you may also want to surface a refetch indicator; otherwise consider keying skeletons offisLoading || isFetchingfor the active filter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/community-curations/CommunityCurations.tsx` around lines 81 - 84, The skeleton visibility logic uses isInitialLoading = isLoading && allDrops.length === 0 which fails when switching mediaFilter because old allDrops may be non-empty; update the logic in CommunityCurations.tsx to account for ongoing refetches for the active filter by including isFetching (or the hook’s refetching state) or by comparing the current mediaFilter key: e.g. compute isInitialLoading = (isLoading || isFetching) && allDrops.length === 0 or track the active filter key from useCommunityCurationsDrops and show skeletons when that key is being refetched; change references to isInitialLoading, isLoading, isFetching, allDrops and mediaFilter accordingly.__tests__/components/waves/WavesLayout.test.tsx (1)
81-114: Consider adding a test for the authenticated (ready) branch with no active wave.The current suite only exercises the
not-authenticatedcontent state. Adding a case whereuseAuthenticatedContentreturns{ contentState: "ready" }andgetActiveWaveIdFromUrlreturnsnullwould lock in the desiredallowMainContentWithoutWavebehavior on the authenticated path and prevent regressions of the inconsistency flagged inWavesLayout.tsx.🤖 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 81 - 114, Add a test case for WavesLayout that mocks useAuthenticatedContent to return { contentState: "ready" } and mockGetActiveWaveIdFromUrl to return null, then render <WavesLayout> with a child element and assert that the rendered DOM shows allowMainContentWithoutWave (data-allow-main-content-without-wave) is "true" and that connect-wallet is not present; locate mocks in the test file (mockGetActiveWaveIdFromUrl and the mocked useAuthenticatedContent) and add a new it(...) block mirroring the existing "logged-out web users when no wave is selected" test but with contentState set to "ready" to cover the authenticated ready branch of WavesLayout.components/shared/WavesMessagesWrapper.tsx (1)
115-119: Simplify the sidebar-visibility predicate.
!canShowMainContentalready impliesisMobile && !hasWave && !allowMainContentWithoutWave, so the extra!hasWaveterm is redundant. The predicate boils down to "show sidebar on desktop, or on mobile when main content can't render":♻️ Proposed simplification
- const hasWave = waveId !== undefined; - const canShowMainContent = !isMobile || hasWave || allowMainContentWithoutWave; - const shouldShowLeftSidebar = - showLeftSidebar && (!isMobile || (!hasWave && !canShowMainContent)); - const shouldShowMainContent = canShowMainContent; + const hasWave = waveId !== undefined; + const shouldShowMainContent = + !isMobile || hasWave || allowMainContentWithoutWave; + const shouldShowLeftSidebar = + showLeftSidebar && (!isMobile || !shouldShowMainContent);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/shared/WavesMessagesWrapper.tsx` around lines 115 - 119, The shouldShowLeftSidebar predicate includes a redundant !hasWave term; simplify by using canShowMainContent instead: update the expression for shouldShowLeftSidebar (which currently uses showLeftSidebar, isMobile, hasWave, and canShowMainContent) to use showLeftSidebar && (!isMobile || !canShowMainContent) so the logic reads "show sidebar on desktop, or on mobile when main content can't render", referencing the existing hasWave and allowMainContentWithoutWave logic captured in canShowMainContent.hooks/useCommunityCurationsDrops.ts (1)
17-18: Note: temporary curation source constant.The
TEMPORARY_prefix and hardcoded"ART"value indicate this is placeholder wiring. Consider tracking it with a TODO/issue link so it isn't forgotten when the real source becomes available, or surface it as a prop.Want me to open a follow-up issue to track replacing this temporary constant with a configurable curation source?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useCommunityCurationsDrops.ts` around lines 17 - 18, The constant TEMPORARY_COMMUNITY_CURATIONS_SOURCE_CURATION_NAME is a hardcoded placeholder ("ART") and risks being forgotten; update hooks/useCommunityCurationsDrops.ts to either (a) add a clear TODO comment including a tracking issue/URL next to TEMPORARY_COMMUNITY_CURATIONS_SOURCE_CURATION_NAME or (b) refactor the hook to accept the curation source as a prop/parameter (expose it where COMMUNITY_CURATIONS_DROPS_QUERY_KEY is used), and remove the TEMPORARY_ prefix once wired; include the chosen approach in a one-line comment so the placeholder is easily discoverable.components/community-curations/CommunityCurationsMasonry.tsx (1)
74-94: Two competing writers for the grid width.
useElementWidthalready maintainswidthvia its internalResizeObserver, but it also exposessetWidthsosetContainerRefcan writeelement.offsetWidthsynchronously when the ref attaches. That mirrors what RO will produce moments later, so the two writers will normally converge, but it's fragile: the ref-callback path usesoffsetWidth(border-box, integer) while RO usesMath.floor(contentRect.width)(content-box) — these can disagree (padding/scrollbar/box-sizing) and produce a transient layout the first time the grid mounts.Consider treating the RO as the single source of truth and removing the
setWidthwrite fromsetContainerRef, or unifying both paths to read from the same measurement (getBoundingClientRect/contentRect).Also applies to: 245-251
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/community-curations/CommunityCurationsMasonry.tsx` around lines 74 - 94, The hook useElementWidth should treat the ResizeObserver as the single source of truth to avoid conflicting measurements: remove the direct setWidth call from the ref callback (setContainerRef) so only the ResizeObserver updates width, or if you need a synchronous initial measurement keep a single measurement method—replace the ref-path write with a synchronous read that mirrors the RO calculation (e.g. use element.getBoundingClientRect().width and Math.floor) before calling setWidth so both paths compute width identically; apply the same change to the other ref callback at the second occurrence (around lines 245-251) so setContainerRef and the duplicate ref callback no longer write different box-model values.
🤖 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/community-curations/communityCurations.helpers.ts`:
- Around line 90-103: In getCommunityCurationsMediaType, when a media object
exists but getMediaTypeFromMimeType(media.mime_type) returns null (mime_type
missing/invalid), first attempt URL-based inference on the media URL (e.g., call
getMediaTypeFromUrl(media.url) or equivalent) before falling back to
getNftLinkMediaType(drop) and then "other"; specifically, inside
getCommunityCurationsMediaType after obtaining const media =
drop.parts.flatMap(...).at(0), if media is present try
getMediaTypeFromMimeType(media.mime_type) || getMediaTypeFromUrl(media.url) ||
getNftLinkMediaType(drop) || "other" so drops with .mp4/.mp3 URLs are correctly
classified.
In `@components/community-curations/CommunityCurations.tsx`:
- Around line 135-140: The empty-state copy is misleading when a media filter is
active: update the render logic around the existing conditional (where
isInitialLoading, isError, and drops.length === 0 are checked) to branch on
mediaFilter (the mediaFilter prop/state) and pass a different title/description
to CommunityCurationsEmptyState when mediaFilter !== "all" (e.g., "No
{mediaFilter} drops match this filter — try 'All'."), otherwise keep the current
copy; modify CommunityCurations.tsx where CommunityCurationsEmptyState is
rendered to select the appropriate strings and pass them as props.
In `@components/community-curations/CommunityCurationsMasonry.tsx`:
- Around line 275-284: The current handleIntersection in
CommunityCurationsMasonry.tsx swallows fetchNextPage errors; update
handleIntersection (the callback using fetchNextPage) to catch errors and route
them to the app's error reporter (e.g., captureException/reportError) instead of
ignoring them, and set a local sticky error state (e.g., nextPageError) when a
failure occurs so you can hide/disable the auto-trigger UI and show an inline
retry control; implement a retry handler that calls fetchNextPage again and
clears nextPageError on success, and ensure the dependency array still includes
fetchNextPage, hasNextPage, isFetchingNextPage, and the new error state/update
function.
In `@hooks/useCommunityCurationsDrops.ts`:
- Around line 78-112: The current client-side filtering causes sparse pages
after pagination; update useCommunityCurationsDrops to either (preferred)
include mediaFilter in the useInfiniteQuery key and pass mediaFilter into
fetchTemporaryCommunityCurationsDrops so server returns already-filtered pages
(update queryKey from [COMMUNITY_CURATIONS_DROPS_QUERY_KEY, { limit }] to
include mediaFilter and change the queryFn to call
fetchTemporaryCommunityCurationsDrops({ cursor: pageParam, limit, mediaFilter
})), or (alternative) implement an auto-fetch loop inside
useCommunityCurationsDrops that while query.hasNextPage && !query.isFetching &&
drops.length < Math.max(1, Math.floor((limit ?? default)/2)) calls
query.fetchNextPage() until the threshold is reached; reference the
useInfiniteQuery call, fetchTemporaryCommunityCurationsDrops, getNextPageParam,
and the drops/allDrops memos when making the change.
---
Outside diff comments:
In `@components/waves/layout/WavesLayout.tsx`:
- Around line 82-90: In the contentState === "ready" branch add the same
allowMainContentWithoutWave prop used in the unauthenticated branch: pass
allowMainContentWithoutWave={activeWaveId === null} into the rendered Waves
component (both WavesMobile and WavesDesktop) so that the Component = isApp ?
WavesMobile : WavesDesktop render receives allowMainContentWithoutWave and
preserves the same main-content behavior for authenticated users; update the JSX
in the ready block where Component is used to include this prop.
---
Nitpick comments:
In `@__tests__/components/waves/WavesLayout.test.tsx`:
- Around line 81-114: Add a test case for WavesLayout that mocks
useAuthenticatedContent to return { contentState: "ready" } and
mockGetActiveWaveIdFromUrl to return null, then render <WavesLayout> with a
child element and assert that the rendered DOM shows allowMainContentWithoutWave
(data-allow-main-content-without-wave) is "true" and that connect-wallet is not
present; locate mocks in the test file (mockGetActiveWaveIdFromUrl and the
mocked useAuthenticatedContent) and add a new it(...) block mirroring the
existing "logged-out web users when no wave is selected" test but with
contentState set to "ready" to cover the authenticated ready branch of
WavesLayout.
In `@components/brain/left-sidebar/waves/MemesWaveFooter.tsx`:
- Around line 92-135: The wrapper div with className "tw-flex tw-items-stretch
tw-gap-2" in MemesWaveFooter is redundant because it only contains a single
<button> (which also has tw-flex-1 and the parent already has tw-w-full); either
remove that wrapper and drop tw-flex-1 from the button, or if you intend to add
siblings later, keep the wrapper but add a short comment stating it's reserved
for future controls and keep tw-flex-1 only if you want the button to grow;
update the JSX around the wrapper div and the button element (remove the extra
<div> and the button's tw-flex-1) accordingly to flatten the structure.
In `@components/community-curations/CommunityCurations.tsx`:
- Around line 81-84: The skeleton visibility logic uses isInitialLoading =
isLoading && allDrops.length === 0 which fails when switching mediaFilter
because old allDrops may be non-empty; update the logic in
CommunityCurations.tsx to account for ongoing refetches for the active filter by
including isFetching (or the hook’s refetching state) or by comparing the
current mediaFilter key: e.g. compute isInitialLoading = (isLoading ||
isFetching) && allDrops.length === 0 or track the active filter key from
useCommunityCurationsDrops and show skeletons when that key is being refetched;
change references to isInitialLoading, isLoading, isFetching, allDrops and
mediaFilter accordingly.
In `@components/community-curations/CommunityCurationsMasonry.tsx`:
- Around line 74-94: The hook useElementWidth should treat the ResizeObserver as
the single source of truth to avoid conflicting measurements: remove the direct
setWidth call from the ref callback (setContainerRef) so only the ResizeObserver
updates width, or if you need a synchronous initial measurement keep a single
measurement method—replace the ref-path write with a synchronous read that
mirrors the RO calculation (e.g. use element.getBoundingClientRect().width and
Math.floor) before calling setWidth so both paths compute width identically;
apply the same change to the other ref callback at the second occurrence (around
lines 245-251) so setContainerRef and the duplicate ref callback no longer write
different box-model values.
In `@components/shared/WavesMessagesWrapper.tsx`:
- Around line 115-119: The shouldShowLeftSidebar predicate includes a redundant
!hasWave term; simplify by using canShowMainContent instead: update the
expression for shouldShowLeftSidebar (which currently uses showLeftSidebar,
isMobile, hasWave, and canShowMainContent) to use showLeftSidebar && (!isMobile
|| !canShowMainContent) so the logic reads "show sidebar on desktop, or on
mobile when main content can't render", referencing the existing hasWave and
allowMainContentWithoutWave logic captured in canShowMainContent.
In `@hooks/useCommunityCurationsDrops.ts`:
- Around line 17-18: The constant
TEMPORARY_COMMUNITY_CURATIONS_SOURCE_CURATION_NAME is a hardcoded placeholder
("ART") and risks being forgotten; update hooks/useCommunityCurationsDrops.ts to
either (a) add a clear TODO comment including a tracking issue/URL next to
TEMPORARY_COMMUNITY_CURATIONS_SOURCE_CURATION_NAME or (b) refactor the hook to
accept the curation source as a prop/parameter (expose it where
COMMUNITY_CURATIONS_DROPS_QUERY_KEY is used), and remove the TEMPORARY_ prefix
once wired; include the chosen approach in a one-line comment so the placeholder
is easily discoverable.
🪄 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: f6fe1432-852a-46f0-a4e8-72eda62e68fe
📒 Files selected for processing (13)
__tests__/components/waves/WavesLayout.test.tsxcomponents/brain/content/BrainContent.tsxcomponents/brain/left-sidebar/waves/MemesWaveFooter.tsxcomponents/brain/my-stream/MyStreamWaveContent.tsxcomponents/community-curations/CommunityCurations.tsxcomponents/community-curations/CommunityCurationsMasonry.tsxcomponents/community-curations/communityCurations.constants.tscomponents/community-curations/communityCurations.helpers.tscomponents/shared/WavesMessagesWrapper.tsxcomponents/waves/WavesDesktop.tsxcomponents/waves/WavesView.tsxcomponents/waves/layout/WavesLayout.tsxhooks/useCommunityCurationsDrops.ts
✅ Files skipped from review due to trivial changes (2)
- components/community-curations/communityCurations.constants.ts
- components/brain/my-stream/MyStreamWaveContent.tsx
Signed-off-by: ragnep <ragneinfo@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
components/community-curations/CommunityCurationsMasonry.tsx (1)
320-327:⚠️ Potential issue | 🟡 MinorDo not swallow pagination fetch errors.
Line 326 silently discards failures from
fetchNextPage, which hides persistent API/network issues and makes debugging harder.Suggested minimal fix
- fetchNextPage().catch(() => undefined); + fetchNextPage().catch((error) => { + // route to app-level error reporter if available + console.error("CommunityCurationsMasonry fetchNextPage failed", error); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/community-curations/CommunityCurationsMasonry.tsx` around lines 320 - 327, The handleIntersection callback currently swallows errors from fetchNextPage(); update handleIntersection so it does not silently ignore failures: call fetchNextPage() and handle rejections by logging the error (or rethrowing/propagating to a parent error handler) instead of .catch(() => undefined). Specifically modify the fetchNextPage() invocation inside handleIntersection (references: handleIntersection, fetchNextPage, hasNextPage, isFetchingNextPage) to surface or record the error so pagination/network failures are visible for debugging.
🧹 Nitpick comments (1)
components/waves/leaderboard/sidebar/WaveLeaderboardRightSidebarTimeWindowSelect.tsx (1)
22-40: Implement keyboard navigation for the tabs pattern.With
role="tab"androle="tablist", this component should implement roving focus and arrow key navigation per the WAI-ARIA tabs pattern. AddtabIndex={isActive ? 0 : -1}for roving focus, and handle arrow keys (Left/Right) plus Home/End for keyboard navigation.Proposed update
- {TIME_WINDOW_OPTIONS.map((option) => { + {TIME_WINDOW_OPTIONS.map((option, index) => { const isActive = value === option; return ( <button key={option} type="button" role="tab" aria-selected={isActive} + tabIndex={isActive ? 0 : -1} + onKeyDown={(e) => { + const moveTo = (nextIndex: number) => { + const len = TIME_WINDOW_OPTIONS.length; + const normalized = (nextIndex + len) % len; + onChange(TIME_WINDOW_OPTIONS[normalized]); + }; + if (e.key === "ArrowRight") { + e.preventDefault(); + moveTo(index + 1); + } else if (e.key === "ArrowLeft") { + e.preventDefault(); + moveTo(index - 1); + } else if (e.key === "Home") { + e.preventDefault(); + onChange(TIME_WINDOW_OPTIONS[0]); + } else if (e.key === "End") { + e.preventDefault(); + onChange(TIME_WINDOW_OPTIONS[TIME_WINDOW_OPTIONS.length - 1]); + } + }} onClick={() => onChange(option)} className={`tw-rounded-full tw-border-0 tw-px-2 tw-py-0.5 tw-text-xs tw-font-medium tw-leading-4 tw-transition-colors tw-duration-150 ${ isActive ? "tw-bg-iron-700 tw-text-iron-50" : "tw-bg-transparent tw-text-iron-500 hover:tw-bg-iron-800/60 hover:tw-text-iron-200" }`} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/sidebar/WaveLeaderboardRightSidebarTimeWindowSelect.tsx` around lines 22 - 40, Add roving focus and arrow/Home/End keyboard handling for the tabs: set tabIndex={isActive ? 0 : -1} on each button rendered from TIME_WINDOW_OPTIONS and implement an onKeyDown handler (e.g., handleKeyDown) to respond to ArrowLeft/ArrowRight/Home/End to compute the next option index and call onChange(nextOption) (use TIME_WINDOW_OPTIONS to find current index and wrap around). Attach the onKeyDown to each tab button (or the tablist container) so keyboard navigation updates focus/selection consistent with the WAI-ARIA tabs pattern while keeping the existing role="tab" and aria-selected usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@components/community-curations/CommunityCurationsMasonry.tsx`:
- Around line 320-327: The handleIntersection callback currently swallows errors
from fetchNextPage(); update handleIntersection so it does not silently ignore
failures: call fetchNextPage() and handle rejections by logging the error (or
rethrowing/propagating to a parent error handler) instead of .catch(() =>
undefined). Specifically modify the fetchNextPage() invocation inside
handleIntersection (references: handleIntersection, fetchNextPage, hasNextPage,
isFetchingNextPage) to surface or record the error so pagination/network
failures are visible for debugging.
---
Nitpick comments:
In
`@components/waves/leaderboard/sidebar/WaveLeaderboardRightSidebarTimeWindowSelect.tsx`:
- Around line 22-40: Add roving focus and arrow/Home/End keyboard handling for
the tabs: set tabIndex={isActive ? 0 : -1} on each button rendered from
TIME_WINDOW_OPTIONS and implement an onKeyDown handler (e.g., handleKeyDown) to
respond to ArrowLeft/ArrowRight/Home/End to compute the next option index and
call onChange(nextOption) (use TIME_WINDOW_OPTIONS to find current index and
wrap around). Attach the onKeyDown to each tab button (or the tablist container)
so keyboard navigation updates focus/selection consistent with the WAI-ARIA tabs
pattern while keeping the existing role="tab" and aria-selected usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f44cfd45-0b45-4af6-ab53-78c96fb8a216
⛔ Files ignored due to path filters (3)
generated/models/ApiCreateMediaUploadUrlRequest.tsis excluded by!**/generated/**generated/models/ApiCreateNewWave.tsis excluded by!**/generated/**generated/models/ApiCreateNewWaveVotingConfig.tsis excluded by!**/generated/**
📒 Files selected for processing (31)
__tests__/components/shared/WavesMessagesWrapper.test.tsx__tests__/components/waves/WavesLayout.test.tsx__tests__/hooks/useCommunityCurationsDrops.test.tsapp/[user]/_lib/userTabPageFactory.tsxcomponents/brain/right-sidebar/WaveContent.tsxcomponents/community-curations/CommunityCurations.tsxcomponents/community-curations/CommunityCurationsMasonry.tsxcomponents/drops/view/BoostedDropCard.tsxcomponents/react-query-wrapper/ReactQueryWrapper.tsxcomponents/shared/WavesMessagesWrapper.tsxcomponents/user/collected/UserPageCollected.tsxcomponents/user/collected/UserPageCollectedStats.tsxcomponents/user/collected/stats/helpers.tscomponents/user/collected/stats/subcomponents/CollectedStatsDetailsPanel.tsxcomponents/user/collected/stats/types.tscomponents/waves/WaveScreenMessage.tsxcomponents/waves/drop/SingleWaveDropAuthor.tsxcomponents/waves/drop/SingleWaveDropChat.tsxcomponents/waves/drop/SingleWaveDropVote.tsxcomponents/waves/drop/SingleWaveDropVotes.tsxcomponents/waves/drops/ContentDisplay.tsxcomponents/waves/drops/ContentSegmentComponent.tsxcomponents/waves/drops/reaction-utils.tscomponents/waves/header/WaveHeader.tsxcomponents/waves/layout/WavesLayout.tsxcomponents/waves/leaderboard/sidebar/WaveLeaderboardRightSidebarBoostedDrops.tsxcomponents/waves/leaderboard/sidebar/WaveLeaderboardRightSidebarTimeWindowSelect.tsxcomponents/waves/leaderboard/sidebar/WaveLeaderboardRightSidebarVoters.tsxcomponents/waves/memes/submission/MobileMemesArtSubmissionBtn.tsxcomponents/waves/utils/getOptimisticDrop.tscontexts/wave/utils/wave-messages-utils.ts
💤 Files with no reviewable changes (1)
- components/waves/WaveScreenMessage.tsx
✅ Files skipped from review due to trivial changes (17)
- components/user/collected/stats/subcomponents/CollectedStatsDetailsPanel.tsx
- components/user/collected/stats/helpers.ts
- components/waves/header/WaveHeader.tsx
- components/waves/drop/SingleWaveDropAuthor.tsx
- components/user/collected/UserPageCollectedStats.tsx
- app/[user]/_lib/userTabPageFactory.tsx
- components/waves/drops/reaction-utils.ts
- components/waves/leaderboard/sidebar/WaveLeaderboardRightSidebarBoostedDrops.tsx
- components/user/collected/stats/types.ts
- components/waves/drop/SingleWaveDropVote.tsx
- components/waves/utils/getOptimisticDrop.ts
- components/react-query-wrapper/ReactQueryWrapper.tsx
- components/brain/right-sidebar/WaveContent.tsx
- components/waves/drop/SingleWaveDropChat.tsx
- contexts/wave/utils/wave-messages-utils.ts
- components/user/collected/UserPageCollected.tsx
- components/waves/drop/SingleWaveDropVotes.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/components/waves/WavesLayout.test.tsx
- components/shared/WavesMessagesWrapper.tsx
- components/community-curations/CommunityCurations.tsx
|
simo6529
approved these changes
Apr 28, 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
Bug Fixes
Accessibility
Style