Conversation
- Align attachment list to left instead of center - Position prev/next buttons over the image with full height click area - Use circular button styling matching ImageViewer design - Center dot indicators relative to image width Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add useImagePreloader hook and buildImageUrl utility to preload adjacent images in AttachmentList, ImageViewer, and StoryViewer components. This improves perceived performance when navigating through carousels and stories. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…utton for other people's profile page
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds pull-to-refresh and an internal infinite-scroll hook, optimistic UI for reactions/bookmarks/comments, image URL builder and preloader, touch/swipe image viewer improvements, ActionButton API changes, and new feed reload methods surfaced in OwnFeedsContext; several viewer/composer props updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PullToRefresh as PullToRefresh (rgba(0,128,0,0.5))
participant Page as Page (rgba(0,0,255,0.5))
participant OwnFeeds as OwnFeedsContext (rgba(128,0,128,0.5))
participant StreamAPI as StreamAPI (rgba(255,165,0,0.5))
User->>PullToRefresh: touch & pull down
PullToRefresh->>PullToRefresh: measure distance & threshold
User->>PullToRefresh: release
PullToRefresh->>Page: onRefresh()
Page->>OwnFeeds: reloadForyouFeed()/reloadOwnFeed()
OwnFeeds->>OwnFeeds: clear errors & call getOrCreate()
OwnFeeds->>StreamAPI: getOrCreate() request
StreamAPI-->>OwnFeeds: feed response
OwnFeeds-->>Page: updated feed state
Page-->>PullToRefresh: signal refresh complete
PullToRefresh-->>User: hide indicator
sequenceDiagram
participant User
participant UI as UI (rgba(70,130,180,0.5))
participant Optimistic as OptimisticState (rgba(75,0,130,0.5))
participant ActionBtn as ActionButton (rgba(0,128,128,0.5))
participant StreamAPI as StreamAPI (rgba(255,165,0,0.5))
User->>ActionBtn: click toggle
ActionBtn->>Optimistic: apply optimistic update (isLiked/count)
Optimistic-->>UI: immediate UI update
UI-->>User: show updated state
ActionBtn->>ActionBtn: startTransition()
ActionBtn->>StreamAPI: add/delete reaction request
alt success
StreamAPI-->>ActionBtn: success
ActionBtn->>Optimistic: confirm state
else error
StreamAPI-->>ActionBtn: error
ActionBtn->>Optimistic: revert state
ActionBtn->>UI: surface error prop / disable control
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
sample-apps/react-demo/app/components/utility/ActionButton.tsx (2)
23-34:disabledprop is ignored whenhrefis provided.When
hrefis set, the component renders aNavLinkinstead of abutton, but thedisabledprop is not applied. This means the link remains interactive even whendisabled={true}, creating inconsistent behavior.Consider either:
- Adding visual disabled styling and preventing navigation for the link case
- Documenting that
disabledonly applies to button modeSuggested fix to handle disabled state for links
- {href ? <div className="btn btn-sm btn-soft"><NavLinkButton href={href}>{content}</NavLinkButton></div> : <button + {href ? <div className={`btn btn-sm btn-soft ${disabled ? 'btn-disabled pointer-events-none' : ''}`}><NavLinkButton href={href}>{content}</NavLinkButton></div> : <button
57-69: Samedisabledhandling gap exists inSecondaryActionButton.The
disabledprop is similarly ignored whenhrefis provided. Apply the same fix as suggested forActionButton.Suggested fix
- {href ? <div className={`btn btn-md btn-ghost p-2 ${className}`}><NavLinkButton href={href}>{content}</NavLinkButton></div> : <button + {href ? <div className={`btn btn-md btn-ghost p-2 ${className} ${disabled ? 'btn-disabled pointer-events-none' : ''}`}><NavLinkButton href={href}>{content}</NavLinkButton></div> : <buttonsample-apps/react-demo/app/profile/[id]/page.tsx (1)
55-62: Unhandled promise rejection for timelinegetOrCreate().
_feed.getOrCreate()has error handling via.catch(), but_timeline.getOrCreate()on line 59 does not. If this call fails, it will result in an unhandled promise rejection.🛡️ Proposed fix
_feed.getOrCreate().catch((e) => { setError(e); throw e; }); - _timeline.getOrCreate(); + _timeline.getOrCreate().catch((e) => { + // Timeline errors are non-critical for profile display + console.error('Failed to load timeline:', e); + }); setFeed(_feed); setTimeline(_timeline);
🤖 Fix all issues with AI agents
In
`@sample-apps/react-demo/app/components/activity/activity-interactions/ToggleReaction.tsx`:
- Around line 15-16: Update ToggleReaction.tsx to safely access reaction_groups:
change the likeCount computation to use optional chaining on
activity.reaction_groups (e.g., replace activity.reaction_groups.like?.count
with activity.reaction_groups?.like?.count) so that likeCount and any subsequent
usage won't throw when reaction_groups is undefined; ensure isLiked and other
references to activity.reaction_groups follow the same pattern.
In `@sample-apps/react-demo/app/utility/useImagePreloader.ts`:
- Around line 7-9: The URL builder in useImagePreloader currently appends
"&w=...&h=..." blindly and breaks when baseUrl has no query string; update the
function that returns `${baseUrl}&w=${width}&h=${height}` to determine the
correct separator by checking baseUrl for an existing '?' (e.g., separator =
baseUrl.includes('?') ? '&' : '?') and then return
`${baseUrl}${separator}w=${width}&h=${height}`; also consider using
URL/URLSearchParams if you want proper encoding of width/height query params,
and keep the existing baseUrl falsy check.
🧹 Nitpick comments (6)
sample-apps/react-demo/app/components/activity/ActivityList.tsx (1)
51-53: Consider adding a comment explaining the intentional omission of the dependency array.This
useEffectruns on every render to detect scroll capability changes after content updates. While this is a valid pattern, it may appear unintentional to other developers. Adding a brief comment would improve clarity.📝 Suggested clarification
useEffect(() => { checkCanScroll(); - }); + }); // Intentionally runs on every render to detect scroll capability after content changessample-apps/react-demo/app/components/stories/StoryViewer.tsx (1)
13-17: Consider SSR safety forscreenglobal.Using
screen.widthandscreen.heightdirectly will fail during server-side rendering sincescreenis a browser-only global. In Next.js with'use client', this should be safe at runtime, but the function could be called during hydration beforescreenis available, or in tests without a DOM environment.💡 Suggestion: Add a fallback or guard
const getStoryImageUrl = (activity: ActivityResponse | undefined) => { const attachment = activity?.attachments?.[0]; if (!attachment || attachment.type === 'video') return null; + const width = typeof screen !== 'undefined' ? screen.width * 2 : 1920; + const height = typeof screen !== 'undefined' ? screen.height * 2 : 1080; - return buildImageUrl(attachment.image_url, screen.width * 2, screen.height * 2); + return buildImageUrl(attachment.image_url, width, height); };sample-apps/react-demo/app/components/utility/PullToRefresh.tsx (2)
78-100: Potential stale closure forpullDistancein async handler.
handleTouchEndcapturespullDistancein its closure. Since it's an async function and state updates are batched, if the user triggers multiple rapid touch interactions,pullDistancecould be stale. However, given theisPullingRefguard and typical touch interaction timing, this is unlikely to cause issues in practice.💡 Optional: Use a ref for pullDistance if issues arise
If you encounter edge cases with rapid interactions, consider tracking
pullDistancein a ref alongside the state to ensure the async handler always reads the latest value.
128-139: No visual feedback during refresh.When
isRefreshingis true, the arrow icon is hidden (Line 130:{!isRefreshing && <span ...), but there's no spinner or loading indicator shown in its place. Users may not have clear feedback that a refresh is in progress.💡 Add a loading spinner during refresh
<div className="flex items-center justify-center py-2"> - {!isRefreshing && <span + {isRefreshing ? ( + <span className="loading loading-spinner loading-sm"></span> + ) : ( + <span className="material-symbols-outlined transition-transform duration-200" style={{ transform: `rotate(${progress >= 1 ? 180 : 0}deg)`, opacity: progress, }} > arrow_downward - </span>} + </span> + )} </div>Based on learnings: "Ensure React Demo app source code is clean, well-structured, and follows React best practices; design must be modern and polished using daisyUI components effectively as this is a reference implementation."
sample-apps/react-demo/app/components/common/attachments/ImageViewer.tsx (1)
26-40: MemoizeimageAttachmentsto prevent unnecessary recomputations.
imageAttachmentsis created viafilter()on every render (line 26), producing a new array reference each time. SinceurlsToPreloaddepends onimageAttachments, this defeats the memoization—useMemowill recalculate on every render.♻️ Proposed fix
+ const imageAttachments = useMemo( + () => attachments.filter((a) => a.type !== 'video'), + [attachments], + ); - const imageAttachments = attachments.filter((a) => a.type !== 'video'); const hasMultiple = imageAttachments.length > 1; const currentAttachment = imageAttachments[currentIndex]; const urlsToPreload = useMemo(() => { if (imageAttachments.length <= 1) return []; const prevIdx = currentIndex === 0 ? imageAttachments.length - 1 : currentIndex - 1; const nextIdx = currentIndex === imageAttachments.length - 1 ? 0 : currentIndex + 1; return [prevIdx, nextIdx] .map((idx) => imageAttachments[idx]) .map((a) => buildImageUrl(a.image_url, VIEWER_SIZE.width, VIEWER_SIZE.height)); }, [imageAttachments, currentIndex]);sample-apps/react-demo/app/profile/[id]/page.tsx (1)
81-83: Simplify redundant boolean variables.
shouldShowBookmarksandshouldShowToggleFollowduplicate theisOwnProfilelogic already computed on line 39. Consider usingisOwnProfiledirectly for clarity.♻️ Proposed simplification
- const shouldShowBookmarks = currentUser?.id === userId; - - const shouldShowToggleFollow = currentUser?.id !== userId;Then in the JSX:
- {shouldShowBookmarks && ( + {isOwnProfile && ( <NavLink ...>- {shouldShowToggleFollow && <ToggleFollowButton userId={userId} />} + {!isOwnProfile && <ToggleFollowButton userId={userId} />}
sample-apps/react-demo/app/components/activity/activity-interactions/ToggleReaction.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 97-108: Clarify that the example commands under the "Typical
use-case" heading must be run from the test-data-generator working directory and
avoid duplication by replacing the full command list with a short pointer to the
existing "Usage" section; update the "Typical use-case" paragraph to say
something like "Run from the test-data-generator/ directory — see Usage for full
command list" and keep only any brief, high-level order notes (referencing yarn
create-users, yarn create-follows, yarn create-posts, yarn create-stories)
rather than repeating the full command block so the canonical command examples
live in the "Usage" section.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@sample-apps/react-demo/app/components/stories/StoryViewer.tsx`:
- Around line 13-17: getStoryImageUrl uses browser-only globals
screen.width/height which break SSR; modify getStoryImageUrl (and callers) to
avoid reading screen on the server by guarding access with a runtime check
(e.g., typeof window !== 'undefined' or typeof screen !== 'undefined') and
provide safe fallback dimensions (or return null) when running server-side.
Update the logic around buildImageUrl(attachment.image_url, ...) inside
getStoryImageUrl to use guardedWidth/guardedHeight variables or accept
dimensions as parameters so server rendering never directly uses
screen.width/screen.height.
- Around line 172-176: In StoryViewer.tsx update the <img> element that uses
mediaUrl to include an appropriate alt attribute for accessibility: if the image
conveys meaningful content use a descriptive alt string (e.g., based on the
story title or media metadata available in the StoryViewer component), otherwise
set alt="" for decorative images; ensure the alt value is derived from
props/state in the StoryViewer component (not hard-coded) so screen readers
receive relevant context.
- Around line 130-140: The call to formatDistanceToNow(currentStory?.created_at,
...) can throw if currentStory is undefined; update the StoryViewer render to
guard before calling formatDistanceToNow (e.g., only invoke formatDistanceToNow
when currentStory and currentStory.created_at are defined) or provide a safe
fallback string. Locate the UI block using currentStory and formatDistanceToNow
in StoryViewer.tsx and add a conditional or ternary around the date line (the
JSX referencing currentStory?.created_at and formatDistanceToNow) so it renders
a fallback like an empty string or "recently" when the date is missing.
🧹 Nitpick comments (1)
sample-apps/react-demo/app/components/stories/StoryTimeline.tsx (1)
43-50: RedundantisOpenprop value.Since
StoryVieweris only rendered whenselectedStoryGroupis truthy (line 43),isOpen={!!selectedStoryGroup}will always evaluate totrue. This differs from the pattern inOwnStories.tsxwhereStoryViewerstays mounted andisOpencontrols the dialog visibility.Consider either:
- Simplifying to
isOpen={true}to make the intent clear, or- Keeping
StoryViewermounted (likeOwnStories.tsx) and usingisOpento control the dialog stateOption 1: Simplify the prop
{selectedStoryGroup && ( <StoryViewer key={selectedStoryGroup.group} activities={selectedStoryGroup.activities} - isOpen={!!selectedStoryGroup} + isOpen={true} onClose={closeStoryViewer} /> )}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sample-apps/react-demo/app/components/utility/ActionButton.tsx (1)
4-34:⚠️ Potential issue | 🟡 MinorDisabled state is ignored for
hrefbuttons.When
hrefis set, the button remains navigable even ifdisabledis true. This breaks parity with the non-link branch and can trigger actions while the UI shows a disabled state.🛠️ Proposed fix to honor `disabled` for link buttons
- return <> - {href ? <div className="btn btn-sm btn-soft"><NavLinkButton href={href}>{content}</NavLinkButton></div> : <button + return <> + {href ? ( + <div className={`btn btn-sm btn-soft ${disabled ? 'pointer-events-none opacity-50' : ''}`}> + <NavLinkButton href={href} disabled={disabled}>{content}</NavLinkButton> + </div> + ) : <button type="button" className="btn btn-sm btn-soft" disabled={disabled} onClick={onClick} > {content} </button> } <ErrorToast error={error} /> </> }; @@ - return <> - - {href ? <div className={`btn btn-md btn-ghost p-2 ${className}`}><NavLinkButton href={href}>{content}</NavLinkButton></div> : <button + return <> + {href ? ( + <div className={`btn btn-md btn-ghost p-2 ${className} ${disabled ? 'pointer-events-none opacity-50' : ''}`}> + <NavLinkButton href={href} disabled={disabled}>{content}</NavLinkButton> + </div> + ) : <button type="button" className={`btn btn-md btn-ghost p-2 ${className}`} disabled={disabled} onClick={onClick} > {content} </button> } <ErrorToast error={error} /> </> }; @@ -const NavLinkButton = ({ - children, - href, -}: { - children: React.ReactNode; - href: string; -}) => { - return <NavLink className="w-full h-full flex flex-row items-center justify-stretch gap-2 min-w-0" href={href}>{children}</NavLink>; -}; +const NavLinkButton = ({ + children, + href, + disabled, +}: { + children: React.ReactNode; + href: string; + disabled?: boolean; +}) => { + return ( + <NavLink + className={`w-full h-full flex flex-row items-center justify-stretch gap-2 min-w-0 ${disabled ? 'cursor-not-allowed' : ''}`} + href={href} + aria-disabled={disabled} + tabIndex={disabled ? -1 : 0} + onClick={disabled ? (e) => e.preventDefault() : undefined} + > + {children} + </NavLink> + ); +};Also applies to: 37-69, 72-79
sample-apps/react-demo/app/components/stories/StoryViewer.tsx (1)
70-74:⚠️ Potential issue | 🟠 MajorGuard against empty activities before marking watched.
If
activitiesis empty (orcurrentIndexis out of range),activityis undefined andactivity.is_watchedthrows.🛠️ Proposed fix
useEffect(() => { const activity = activities[currentIndex]; - if (!activity.is_watched) { + if (!activity) return; + if (!activity.is_watched) { void feed?.markActivity({ mark_watched: [activity.id] }); } }, [currentIndex, feed, activities]);
🤖 Fix all issues with AI agents
In `@sample-apps/react-demo/app/components/stories/StoryViewer.tsx`:
- Around line 43-53: The useEffect currently calls
dialogRef.current?.showModal() unconditionally when isOpen is true, which can
throw InvalidStateError if the dialog is already open; update the effect to
guard calls against dialogRef.current.open (or equivalent) so you only call
dialogRef.current.showModal() when dialogRef.current exists and
!dialogRef.current.open, and optionally only call dialogRef.current.close() when
dialogRef.current.open for symmetry; adjust the block around dialogRef,
setCurrentIndex, activities and isOpen so the showModal/close calls are safely
guarded.
🧹 Nitpick comments (2)
sample-apps/react-demo/app/utility/userIdToName.ts (1)
1-3: The rename looks good, but there's a pre-existing bug in the logic.The function name simplification from
userIdToUserNametouserIdToNameis appropriate. However, since you're touching this file and this is a reference implementation, consider fixing the bug in line 2:str.slice(1)operates on the original string instead of the hyphen-replaced string.Example:
"john-doe"produces"John-doe"instead of"John doe".🔧 Proposed fix
export const userIdToName = (str: string) => { - return str.replaceAll('-', ' ').charAt(0).toUpperCase() + str.slice(1); + const replaced = str.replaceAll('-', ' '); + return replaced.charAt(0).toUpperCase() + replaced.slice(1); };Based on learnings: "Ensure React Demo app source code is clean, well-structured, and follows React best practices... as this is a reference implementation."
sample-apps/react-demo/app/bookmarks/page.tsx (1)
69-69: Simplify the Tailwind utility classes—flex-row,items-stretch, andjustify-stretchare redundant defaults.While
justify-stretchis a valid Tailwind utility, it appliesjustify-content: stretch, which is ineffective on flexbox containers (primarily relevant for CSS Grid). Combined withflex-rowanditems-stretchbeing the default flex behaviors, these can be safely removed.Since
ActivityPreviewalready wraps its content withw-full, the list item only needs the basew-full list-rowclasses. A similar pattern insearch/page.tsxsuccessfully uses justlist-row w-fullwithout the extra flex utilities.♻️ Suggested simplification
- <li className="list-row w-full flex flex-row justify-stretch items-stretch" key={bookmark.activity.id}> + <li className="list-row w-full" key={bookmark.activity.id}>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@sample-apps/react-demo/app/components/common/attachments/ImageViewer.tsx`:
- Around line 29-43: The filtered array imageAttachments is recreated each
render (from attachments.filter(...)) which invalidates the useMemo for
urlsToPreload; fix by memoizing the filter result (e.g., wrap the filter in a
useMemo) so imageAttachments only changes when attachments changes, then keep
urlsToPreload's useMemo dependent on the memoized imageAttachments and
currentIndex; reference imageAttachments, urlsToPreload, useMemo, attachments,
currentIndex, buildImageUrl, and VIEWER_SIZE when making the change.
- Around line 145-152: The img src fallback uses an empty string when
buildImageUrl(...) returns null which can display a broken image; update the
ImageViewer component to avoid setting src="" by conditionally rendering the
<img> only when buildImageUrl(currentAttachment?.image_url, VIEWER_SIZE.width,
VIEWER_SIZE.height) returns a non-null/undefined URL (or use a real placeholder
URL), e.g. compute const imageUrl = buildImageUrl(...) and then render the <img>
only if imageUrl is truthy (or render an alternative placeholder element) so
that currentAttachment, buildImageUrl, and VIEWER_SIZE are handled defensively.
🧹 Nitpick comments (4)
sample-apps/react-demo/app/components/common/attachments/AttachmentList.tsx (2)
54-88: Consider extracting swipe handlers to a shared hook.The swipe logic (touch refs, threshold check, direction handling) is duplicated in
ImageViewer.tsx. Extracting this into a reusable hook likeuseSwipeNavigationwould reduce duplication and ensure consistent behavior.♻️ Example hook extraction
// app/utility/useSwipeNavigation.ts export const useSwipeNavigation = ( onNext: () => void, onPrevious: () => void, enabled: boolean, threshold = 50 ) => { const touchStartX = useRef<number | null>(null); const touchEndX = useRef<number | null>(null); const handleTouchStart = useCallback((e: React.TouchEvent) => { if (!enabled) return; touchStartX.current = e.touches[0].clientX; touchEndX.current = null; }, [enabled]); const handleTouchMove = useCallback((e: React.TouchEvent) => { if (!enabled) return; touchEndX.current = e.touches[0].clientX; }, [enabled]); const handleTouchEnd = useCallback(() => { if (!enabled || touchStartX.current === null || touchEndX.current === null) return; const diff = touchStartX.current - touchEndX.current; if (Math.abs(diff) > threshold) { diff > 0 ? onNext() : onPrevious(); } touchStartX.current = null; touchEndX.current = null; }, [enabled, onNext, onPrevious, threshold]); return { handleTouchStart, handleTouchMove, handleTouchEnd }; };
114-122: Navigation buttons missing cursor style for better UX.The navigation buttons have
cursor-pointeron the outer button but the inner<span>withbtn btn-circlemay override interaction styles. This looks fine, but consider addingtype="button"to prevent accidental form submissions if this component is ever used inside a form.♻️ Suggested improvement
<button + type="button" className="absolute left-0 top-0 h-full z-10 flex items-center justify-center px-2 cursor-pointer" onClick={goToPrevious} aria-label="Previous attachment" >Also applies to: 131-141
sample-apps/react-demo/app/components/common/attachments/ImageViewer.tsx (2)
67-101: Duplicate swipe handlers - same pattern as AttachmentList.tsx.As noted in AttachmentList.tsx, these touch handlers follow the exact same pattern. Extracting to a shared
useSwipeNavigationhook would improve maintainability.
118-124: Close button usesfixedpositioning which may overlay other UI.The close button with
fixed right-0 top-0will position relative to the viewport, not the modal. While this works when the modal is fullscreen, it could cause issues if the modal styling changes. Consider usingabsolutewithin the modal-box container instead.♻️ Alternative positioning
- <button - className="btn btn-circle btn-sm fixed right-0 top-0 z-50 bg-base-100/80 hover:bg-base-100" - onClick={onClose} - aria-label="Close" - > + <button + className="btn btn-circle btn-sm absolute right-4 top-4 z-50 bg-base-100/80 hover:bg-base-100" + onClick={onClose} + aria-label="Close" + >
sample-apps/react-demo/app/components/common/attachments/ImageViewer.tsx
Outdated
Show resolved
Hide resolved
sample-apps/react-demo/app/components/common/attachments/ImageViewer.tsx
Show resolved
Hide resolved
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)
sample-apps/react-demo/app/components/stories/StoryViewer.tsx (1)
75-80:⚠️ Potential issue | 🟡 MinorPotential runtime error when
activitiesarray is empty.If
activitiesis empty,activities[currentIndex]returnsundefined, and accessingactivity.is_watchedon Line 77 will throw aTypeError.Proposed fix with early return guard
useEffect(() => { const activity = activities[currentIndex]; - if (!activity.is_watched) { + if (!activity || activity.is_watched) return; - void feed?.markActivity({ mark_watched: [activity.id] }); - } + void feed?.markActivity({ mark_watched: [activity.id] }); }, [currentIndex, feed, activities]);
🤖 Fix all issues with AI agents
In `@sample-apps/react-demo/app/components/common/attachments/ImageViewer.tsx`:
- Around line 25-32: When attachments change the currentIndex can become out of
bounds causing currentAttachment to be undefined; clamp currentIndex whenever
imageAttachments length changes and also ensure the initial state is clamped.
Add a useEffect that watches imageAttachments.length and calls
setCurrentIndex(prev => Math.min(prev, Math.max(0, imageAttachments.length -
1))). Also clamp the initial useState(initialIndex) value to the valid range and
apply the same bounds-checking in navigation handlers (e.g., next/previous
functions that update currentIndex) to prevent cycling from an invalid index.
- Line 2: The code is using the React namespace (React.TouchEvent) without
importing React under the modern JSX transform; import the TouchEvent type
directly from 'react' and replace React.TouchEvent<HTMLDivElement> (or other
element) with TouchEvent<HTMLDivElement> where used (e.g., in ImageViewer's
touch handlers and in AttachmentList at the noted handler locations). Update the
top-level import line to include TouchEvent (import { useEffect, useRef,
useState, useCallback, useMemo, TouchEvent } from 'react') and adjust the event
handler signatures to use the direct TouchEvent generic type.
In `@sample-apps/react-demo/app/components/stories/StoryViewer.tsx`:
- Around line 153-158: The close button in StoryViewer.tsx lacks an accessible
name; update the button element that uses onClose (the element with className
and the inner <span className="material-symbols-outlined">close</span>) to
include an aria-label (e.g., aria-label="Close" or "Close story") so screen
readers can identify its purpose while keeping the existing onClick and visual
icon intact.
sample-apps/react-demo/app/components/common/attachments/ImageViewer.tsx
Outdated
Show resolved
Hide resolved
sample-apps/react-demo/app/components/common/attachments/ImageViewer.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sample-apps/react-demo/app/AppSkeleton.tsx (2)
67-67:⚠️ Potential issue | 🟡 MinorInvalid Tailwind class:
position-absoluteshould beabsolute.
position-absoluteis not a valid Tailwind CSS class. The correct class isabsolute. Additionally,left-23is not a standard Tailwind spacing value—consider usingleft-6or an arbitrary value likeleft-[5.75rem]if precise positioning is needed.🐛 Proposed fix
- <div className="badge badge-primary badge-xs position-absolute left-23"> + <div className="badge badge-primary badge-xs absolute left-6">
91-113:⚠️ Potential issue | 🟠 MajorInvalid HTML:
<button>elements wrapping anchor links.Nesting
<a>elements (fromMenuNavLink→NavLink→ Next.jsLink) inside<button>elements produces invalid HTML per the spec, as both are interactive content. This causes accessibility issues and unpredictable browser behavior.Remove the
<button>wrappers and let the anchor links be direct children of the dock component:🛠️ Proposed fix
<div className="dock md:hidden w-full"> - <button> - <HomeLink /> - </button> + <HomeLink /> - <button> - <ExploreLink /> - </button> + <ExploreLink /> - <button> - <AddLink /> - </button> + <AddLink /> - <button> - <NotificationsLink> - {hasUnreadNotifications && ( - <div className="badge badge-primary h-[0.25rem] w-[0.25rem] p-[0.25rem] absolute left-[60%] top-[15%]" /> - )} - </NotificationsLink> - </button> + <NotificationsLink> + {hasUnreadNotifications && ( + <div className="badge badge-primary h-[0.25rem] w-[0.25rem] p-[0.25rem] absolute left-[60%] top-[15%]" /> + )} + </NotificationsLink> - <button> - <ProfileLink /> - </button> + <ProfileLink /> </div>
🧹 Nitpick comments (1)
sample-apps/react-demo/app/AppSkeleton.tsx (1)
19-19: Duplicatemax-h-fullclass.The class
max-h-fullappears twice in this className string. Remove the duplicate for cleaner code.🧹 Suggested fix
- <div className="drawer-content max-h-full min-h-0 h-full max-h-full flex flex-col gap-1 items-center justify-center"> + <div className="drawer-content max-h-full min-h-0 h-full flex flex-col gap-1 items-center justify-center">
1 participant
🎫 Ticket: https://linear.app/stream/issue/REACT-751/react-demo
💡 Overview
📝 Implementation notes
Summary by CodeRabbit
New Features
Improvements
Public API
Docs
✏️ Tip: You can customize this high-level summary in your review settings.