Conversation
- Fix duplicate sign-in messages on profile tab - Redirect unauthenticated users to profile on Create Listing (web) - Fix browser tab title persisting after leaving a listing - Style placeholder text with lighter grey across profile and create listing forms - Standardize Create Listing button to brand green (#154734) - Capitalize category and condition values on listing detail page - Add profile setup check before creating listings with visible warning banner - Replace Alert.alert with cross-platform showAlert for web compatibility - Add Delete button to My Listings with confirmation dialog
- Extract showAlert into shared utils/showAlert.ts module - Prevent concurrent deletes in my-listings (guard + disable all buttons) - Fix year reset to empty string in settings signed-out useEffect - Remove unused Alert/Platform imports from new.tsx
…nk, image lightbox - Add 8s timeout + retry button to auth 'checking' step to prevent infinite spinner - Replace empty grey seller avatar with initials (first letter of name) on listing detail - Make seller block tappable, navigates to public profile - Change 'Open in App' to 'Download App' with placeholder App Store URL - Add download hint link to OpenInAppPrompt component - Create ImageLightbox component for full-screen uncropped image viewing - Wrap all listing hero images in Pressable to open lightbox on click - Support keyboard navigation (Escape, ←, →) in lightbox on web
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds a profile-check timeout and retry to the login flow, introduces a reusable full-screen ImageLightbox component, makes listing images openable in a lightbox, converts web app CTA to a shared App Store URL, makes seller profiles navigable, and adds a cross-platform showAlert utility. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Page as ListingPage
participant Lightbox as ImageLightbox
participant Modal as NativeModal / WebOverlay
participant ImageComp as Image
User->>Page: tap/click image
Page->>Lightbox: open(images, initialIndex)
Lightbox->>Modal: render overlay (web: overlay, native: Modal)
Modal->>ImageComp: display image at currentIndex
User->>Lightbox: next/prev/dot/escape/close
Lightbox->>ImageComp: update currentIndex / onClose -> Page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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📝 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
frontend/components/OpenInAppPrompt.tsx (1)
7-8: Avoid shipping hardcoded placeholder store URLs.This is fine temporarily, but this constant should come from environment/config (and eventually be platform-specific) to reduce release risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/OpenInAppPrompt.tsx` around lines 7 - 8, The APP_STORE_URL constant in OpenInAppPrompt.tsx is a hardcoded placeholder; change it to read from configuration/environment instead (e.g., use NEXT_PUBLIC_APP_STORE_URL or a platform-specific config accessor) so store URLs are not baked into the build; update the code that references APP_STORE_URL to use the env/config value with a safe fallback or runtime assertion and remove the hardcoded 'https://polybuys.com/download' literal so store URLs can be injected per environment or platform.frontend/components/ImageLightbox.tsx (1)
62-68: Consider auto-focusing the overlay for keyboard navigation on web.The overlay sets
tabIndex={0}but doesn't receive focus automatically when the lightbox opens. Users must click or tab to the overlay before keyboard navigation (Escape, ArrowLeft, ArrowRight) will work.Adding a ref with auto-focus when visible would improve keyboard accessibility:
♿ Proposed enhancement for auto-focus
+import { useEffect, useState, useRef } from 'react'; ... export default function ImageLightbox({ images, initialIndex = 0, visible, onClose, }: ImageLightboxProps) { const [currentIndex, setCurrentIndex] = useState(initialIndex); const { width: screenWidth, height: screenHeight } = useWindowDimensions(); + const overlayRef = useRef<View>(null); // Sync index when the lightbox opens with a new initialIndex useEffect(() => { if (visible) { setCurrentIndex(initialIndex); + // Auto-focus for keyboard navigation on web + if (Platform.OS === 'web' && overlayRef.current) { + (overlayRef.current as unknown as HTMLElement).focus?.(); + } } }, [visible, initialIndex]);Then add the ref to the overlay View:
const content = ( <View + ref={overlayRef} style={styles.overlay} // `@ts-expect-error` — onKeyDown is web-only onKeyDown={Platform.OS === 'web' ? handleKeyDown : undefined} tabIndex={Platform.OS === 'web' ? 0 : undefined} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/ImageLightbox.tsx` around lines 62 - 68, The overlay doesn't auto-focus on web so keyboard handlers (handleKeyDown) don't receive events; in the ImageLightbox component create a ref for the overlay element (e.g., overlayRef), attach it to the same View that currently has onKeyDown/tabIndex, and in a useEffect run when the lightbox becomes visible (the component's visibility prop/state) and Platform.OS === 'web' call overlayRef.current?.focus() to autofocus; keep onKeyDown and tabIndex={0} as-is so Escape/Arrow keys work once focused.frontend/app/listings/[id].tsx (1)
38-39: Centralize theAPP_STORE_URLconstant to avoid duplication.This constant is also defined in
frontend/components/OpenInAppPrompt.tsxwith the same value. Duplicating configuration constants risks inconsistency if one is updated without the other.♻️ Suggested approach
Create a shared constants file and import from both locations:
// frontend/constants/app.ts export const APP_SCHEME = 'polybuys'; // TODO: Replace with actual App Store / Play Store URLs once published export const APP_STORE_URL = 'https://polybuys.com/download';Then update both files to import from the shared location.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/listings/`[id].tsx around lines 38 - 39, The APP_STORE_URL constant is duplicated; create a single shared constants module that exports APP_STORE_URL (and APP_SCHEME) with the existing TODO comment, then replace the local const in the listings page and the OpenInAppPrompt component to import APP_STORE_URL (and APP_SCHEME) from that shared module so both use the same source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/auth/login.tsx`:
- Around line 113-118: handleCheckingRetry currently starts a 100ms setTimeout
(the setTimeout inside handleCheckingRetry) but never stores or clears the
timer, which can cause stale state updates; fix this by storing the timer id in
a ref (e.g., retryTimerRef), clear any existing timer before creating a new one,
use the ref to clearTimeout in the component cleanup (useEffect return) and also
clear it before navigating/unmounting, and keep the existing logic that
setsStep('email') then schedules setStep('checking').
In `@frontend/app/listings/`[id].tsx:
- Around line 279-284: The inline comment in the onPress handler is misleading
because no deep link is attempted; either remove or update the comment to
correctly describe that the handler opens the App Store via
Linking.openURL(APP_STORE_URL), or implement the intended deep-link-first
behavior by attempting Linking.openURL(DEEP_LINK_URL) and falling back to
Linking.openURL(APP_STORE_URL) on failure; update the onPress handler and its
comment to reference the exact symbols Linking.openURL, APP_STORE_URL (and
DEEP_LINK_URL if you add it) so the code and comment remain consistent.
- Around line 568-573: The lightbox is given filtered images but initialIndex
(imageIndex) references the unfiltered mappedUrls, causing index mismatches; fix
by either passing the unfiltered mappedUrls array to ImageLightbox (so call
images={mappedUrls} and update ImageLightboxProps to accept (string|null)[] if
needed) or compute the correct initialIndex by mapping imageIndex from the
original mappedUrls into the filtered array (e.g., find the nth non-null
position) before rendering ImageLightbox; update the call-site around
ImageLightbox and the state used to open it (imageIndex, lightboxOpen,
setLightboxOpen) so they remain consistent with the chosen approach.
In `@frontend/components/OpenInAppPrompt.tsx`:
- Around line 69-71: The accessibilityLabel on the Touchable/Link in
OpenInAppPrompt.tsx is platform-specific ("Download from App Store"); change it
to a platform-neutral label such as "Download the PolyBuys app" by updating the
accessibilityLabel prop on the element in the OpenInAppPrompt component (or
replace with an i18n key if you use localization) so the prompt is accurate
across platforms.
- Around line 67-73: The Pressable currently calls
ExpoLinking.openURL(APP_STORE_URL) without awaiting or handling errors; update
the onPress handler to mirror handleOpenInApp by making it async, awaiting
ExpoLinking.openURL(APP_STORE_URL) inside a try/catch and calling the same
Alert.alert fallback on error so link failures are surfaced to users; reference
the existing handleOpenInApp implementation to copy its try/catch/alert pattern
and use APP_STORE_URL and ExpoLinking.openURL in the new handler.
---
Nitpick comments:
In `@frontend/app/listings/`[id].tsx:
- Around line 38-39: The APP_STORE_URL constant is duplicated; create a single
shared constants module that exports APP_STORE_URL (and APP_SCHEME) with the
existing TODO comment, then replace the local const in the listings page and the
OpenInAppPrompt component to import APP_STORE_URL (and APP_SCHEME) from that
shared module so both use the same source of truth.
In `@frontend/components/ImageLightbox.tsx`:
- Around line 62-68: The overlay doesn't auto-focus on web so keyboard handlers
(handleKeyDown) don't receive events; in the ImageLightbox component create a
ref for the overlay element (e.g., overlayRef), attach it to the same View that
currently has onKeyDown/tabIndex, and in a useEffect run when the lightbox
becomes visible (the component's visibility prop/state) and Platform.OS ===
'web' call overlayRef.current?.focus() to autofocus; keep onKeyDown and
tabIndex={0} as-is so Escape/Arrow keys work once focused.
In `@frontend/components/OpenInAppPrompt.tsx`:
- Around line 7-8: The APP_STORE_URL constant in OpenInAppPrompt.tsx is a
hardcoded placeholder; change it to read from configuration/environment instead
(e.g., use NEXT_PUBLIC_APP_STORE_URL or a platform-specific config accessor) so
store URLs are not baked into the build; update the code that references
APP_STORE_URL to use the env/config value with a safe fallback or runtime
assertion and remove the hardcoded 'https://polybuys.com/download' literal so
store URLs can be injected per environment or platform.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eeb3d1f7-f352-415b-9ff9-edd679a674e1
📒 Files selected for processing (5)
frontend/app/auth/login.tsxfrontend/app/listings/[id].tsxfrontend/components/ImageLightbox.tsxfrontend/components/OpenInAppPrompt.tsxfrontend/utils/showAlert.ts
- Fix login retry timer leak - Fix ImageLightbox filtered array index mismatch and add auto-focus on web - Improve OpenInAppPrompt link error handling and accessibility labels - Move APP_STORE_URL and APP_SCHEME to shared constants
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/auth/login.tsx (1)
85-111:⚠️ Potential issue | 🔴 CriticalMove the delayed redirect out of this effect.
Line 99 changes
stepto'success', so React tears this effect down on the next commit and the cleanup from Lines 100-103 cancels the redirect before it fires. Users who already have a profile will stop on the success screen instead of navigating away.Proposed fix
useEffect(() => { if (step !== 'checking') { setCheckingTimedOut(false); return; } if (profileData === undefined) { // Start a timeout — if the profile query hasn't resolved after 8 s, // surface an error state so the user isn't stuck on an infinite spinner. const timeout = setTimeout(() => setCheckingTimedOut(true), 8000); return () => clearTimeout(timeout); } if (profileData) { - setStep('success'); - const t = setTimeout(() => { - router.replace(postAuthRedirect); - }, 1500); - return () => clearTimeout(t); + setStep('success'); + return; } if (authLoading || !isAuthenticated) { return; } setStep('profile'); }, [step, profileData, authLoading, isAuthenticated, postAuthRedirect, router]); + + useEffect(() => { + if (step !== 'success') { + return; + } + + const timeout = setTimeout(() => { + router.replace(postAuthRedirect); + }, 1500); + + return () => clearTimeout(timeout); + }, [step, postAuthRedirect, router]);Then let
finishAndRedirectonly callsetStep('success')so both success paths share the same redirect effect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/auth/login.tsx` around lines 85 - 111, The delayed router.replace call must be removed from the main useEffect so the effect teardown doesn't cancel it; instead create a new useEffect that watches step === 'success' and starts a timeout to call router.replace(postAuthRedirect) with proper cleanup, and change any success path (the profileData branch and the finishAndRedirect helper) to only call setStep('success') (remove their own setTimeouts). Update references to setStep, finishAndRedirect, router.replace, and postAuthRedirect accordingly so the single redirect effect handles navigation.
🧹 Nitpick comments (1)
frontend/components/ImageLightbox.tsx (1)
179-194: Consider usingReactDOM.createPortalfor the web overlay to ensure full viewport coverage.The lightbox component renders inside the parent ScrollView. On web,
absoluteFillObjectwithzIndex: 9999positions the overlay relative to the ScrollView container, not the viewport. Since the parent component'scardstyle includesoverflow: 'hidden'andheroImageWrapusesposition: 'relative', the overlay may be clipped or constrained depending on the layout hierarchy. UsingReactDOM.createPortalto render directly intodocument.bodywould ensure the overlay always covers the full viewport regardless of parent positioning context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/ImageLightbox.tsx` around lines 179 - 194, The web branch of ImageLightbox currently returns content inline (inside the parent ScrollView) which can be clipped by parent styles; update the Platform.OS === 'web' branch to render content via ReactDOM.createPortal into document.body instead of returning content directly—locate the web conditional around Platform.OS === 'web' and replace the direct return of the content variable with a portal render (using ReactDOM.createPortal(content, document.body)), ensuring imports include ReactDOM and that any event handlers (onClose) and style expectations remain attached to the same content element.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/app/auth/login.tsx`:
- Around line 85-111: The delayed router.replace call must be removed from the
main useEffect so the effect teardown doesn't cancel it; instead create a new
useEffect that watches step === 'success' and starts a timeout to call
router.replace(postAuthRedirect) with proper cleanup, and change any success
path (the profileData branch and the finishAndRedirect helper) to only call
setStep('success') (remove their own setTimeouts). Update references to setStep,
finishAndRedirect, router.replace, and postAuthRedirect accordingly so the
single redirect effect handles navigation.
---
Nitpick comments:
In `@frontend/components/ImageLightbox.tsx`:
- Around line 179-194: The web branch of ImageLightbox currently returns content
inline (inside the parent ScrollView) which can be clipped by parent styles;
update the Platform.OS === 'web' branch to render content via
ReactDOM.createPortal into document.body instead of returning content
directly—locate the web conditional around Platform.OS === 'web' and replace the
direct return of the content variable with a portal render (using
ReactDOM.createPortal(content, document.body)), ensuring imports include
ReactDOM and that any event handlers (onClose) and style expectations remain
attached to the same content element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a1911d52-aa0f-4975-bddf-08f65b7dbf16
📒 Files selected for processing (5)
frontend/app/auth/login.tsxfrontend/app/listings/[id].tsxfrontend/components/ImageLightbox.tsxfrontend/components/OpenInAppPrompt.tsxfrontend/constants/app.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/constants/app.ts
This was referenced Apr 16, 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
Addresses four UI/UX issues identified by Rishi:
checkingprofile step. If it hangs, it transitions to an error state with a retry button to prevent infinite spinners.https://polybuys.com/download). Added a download hint to theOpenInAppPromptcomponent.ImageLightboxcomponent for unrestricted full-screen image viewing withresizeMode='contain'. Wrapped listing hero images in aPressableto trigger it. Added a dark backdrop, navigation arrows, and web keyboard support (Escape, Left, Right).Summary by CodeRabbit