feat(example): add hotel carousel custom marker demo#42
Conversation
Demonstrates view-shot rasterized RN pins with carousel focus sync, native clustering, and a copy-paste snippet for custom marker workflows. Co-authored-by: Cursor <cursoragent@cursor.com>
|
React Doctor found 7 issues in 7 files · 7 warnings · score 86 / 100 (Great) · full project 7 warnings
Reviewed by React Doctor for commit |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an example "Hotel carousel custom marker demo" screen that rasterizes custom pin images via ViewShot and displays them on a clustered map synchronized with a horizontal carousel. Adds hotel data, a HotelPin component, App.tsx entry points, GestureHandlerRootView wrapping, new dependencies, and a documentation snippet. ChangesHotel carousel demo feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant HotelCarouselExample
participant PinCapture
participant ViewShot
participant MapView
HotelCarouselExample->>PinCapture: mount offscreen pin (normal/focused)
PinCapture->>ViewShot: capture()
ViewShot-->>PinCapture: tempfile URI
PinCapture-->>HotelCarouselExample: onCaptured(uri)
HotelCarouselExample->>HotelCarouselExample: batch captured URIs into markers
User->>HotelCarouselExample: scroll carousel / tap card
HotelCarouselExample->>HotelCarouselExample: focusHotel(id)
HotelCarouselExample->>MapView: animate camera to hotel
MapView-->>HotelCarouselExample: onMarkerPress(id)
Suggested reviewers: Right, let's not sugarcoat this: it's 568 lines of a single new file crammed with refs, microtask flushes, timeouts, and dedup logic that nobody bothered to unit test. If your 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
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: 5
🧹 Nitpick comments (1)
example/HotelCarouselExample.tsx (1)
445-465: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff24 simultaneous ViewShot mounts/captures on screen open.
Every hotel renders two hidden
PinCaptureinstances that all capture on mount at once. Fine for 12 hotels, but worth a note if this dataset grows — batching or staggering captures would keep startup smooth on lower-end devices.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/HotelCarouselExample.tsx` around lines 445 - 465, The HotelCarouselExample render path mounts two hidden PinCapture components per hotel, causing all view-shot captures to start simultaneously on screen open. Update the HOTELS mapping and PinCapture usage so captures are batched, staggered, or otherwise deferred instead of firing all at once, keeping the initial mount work lighter while preserving the existing handlePinCaptured flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@example/App.tsx`:
- Around line 367-381: The dock currently renders two “Open hotel carousel
custom marker demo” controls at the same time, because the Hotels pill in the
dock row is always shown even when the expanded hotel demo banner is visible.
Update the dock rendering in App.tsx so the collapsed-row Hotels button is
conditionally hidden when expanded, matching the existing conditional behavior
used by the neighboring chevron iconButton and keeping onLaunchHotelDemo exposed
only once.
In `@example/examples/hotelCarouselSnippet.md`:
- Around line 113-118: The marker anchor is centered vertically instead of being
pinned to the tip, so update the anchor used in the hotel carousel marker
definition to bottom-align the bitmap. Adjust the anchor in the marker/image
configuration near the focused/normal pin rendering so the hotel location maps
to the pin tip rather than the middle of the image.
- Around line 81-97: The capture flow in useCallback’s capture helper marks a
pin as captured before rasterization completes, so a failed captureRef call
blocks retries permanently. Move the captured.current.add(key) update to after
captureRef succeeds, and only then call setUris so the hotel can retry on
transient failures. Keep the change localized to capture and preserve the
existing key/ref lookup logic.
- Around line 12-26: The hotelCarouselSnippet example is missing the captureRef
import even though the snippet uses captureRef later, so the copy-paste sample
breaks. Update the import block alongside ViewShot to bring in captureRef from
react-native-view-shot, and keep the rest of the snippet unchanged so the
referenced capture logic resolves correctly.
In `@example/HotelCarouselExample.tsx`:
- Around line 108-122: The pin capture flow in PinCapture/HotelCarouselExample
currently only logs failures, which leaves the hotel’s marker URI empty forever
and causes markers() to filter it out permanently. Update the failure path in
onCaptureFailure to recover by either marking the capture as completed with a
fallback/default marker or triggering a retry/remount of PinCapture (for example
by bumping a key), and make sure pendingPinUris.current and the capture
completion path are always advanced so onCaptured-style state does not get
stuck. Also review the timeout handling around the capture status update so it
degrades gracefully instead of only showing a rebuild message.
---
Nitpick comments:
In `@example/HotelCarouselExample.tsx`:
- Around line 445-465: The HotelCarouselExample render path mounts two hidden
PinCapture components per hotel, causing all view-shot captures to start
simultaneously on screen open. Update the HOTELS mapping and PinCapture usage so
captures are batched, staggered, or otherwise deferred instead of firing all at
once, keeping the initial mount work lighter while preserving the existing
handlePinCaptured flow.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9bc766f-9e86-4c18-81fe-bbf1145b8a6f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
example/App.tsxexample/HotelCarouselExample.tsxexample/HotelPin.tsxexample/examples/hotelCarouselSnippet.mdexample/examples/hotels.tsexample/index.jsexample/package.json
| <ScalePressable | ||
| onPress={onLaunchHotelDemo} | ||
| accessibilityRole="button" | ||
| accessibilityLabel="Open hotel carousel custom marker demo" | ||
| style={styles.hotelDemoBanner} | ||
| > | ||
| <View style={styles.hotelDemoBannerText}> | ||
| <Text style={styles.hotelDemoBannerTitle}>Hotel carousel demo</Text> | ||
| <Text style={styles.hotelDemoBannerSubtitle}> | ||
| Custom RN pins · carousel sync · clustering | ||
| </Text> | ||
| </View> | ||
| <Text style={styles.hotelDemoBannerChevron}>›</Text> | ||
| </ScalePressable> | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Two buttons, one job, zero shame — fire the redundant one.
Look closely: the "Hotel carousel demo" banner (367-381) only renders when expanded is true, but the "Hotels" pill in dockRow (473-481) has no such manners — it renders unconditionally. Compare it to its neighbor, the chevron iconButton at lines 498-508, which politely disappears via {!expanded ? (...) : null} when the panel opens. Nobody taught the "Hotels" button that trick.
End result: expand the dock and you get two buttons, both wired to onLaunchHotelDemo, both barking the identical accessibilityLabel="Open hotel carousel custom marker demo" at screen readers simultaneously. That's not a feature, that's a UI hiccup shipped straight to your users' accessibility tree.
🩹 Proposed fix — gate the collapsed-row button like its sibling
<View style={styles.dockRow}>
- <ScalePressable
- onPress={onLaunchHotelDemo}
- accessibilityRole="button"
- accessibilityLabel="Open hotel carousel custom marker demo"
- style={styles.hotelDemoLaunch}
- >
- <Text style={styles.hotelDemoLaunchIcon}>⌁</Text>
- <Text style={styles.hotelDemoLaunchText}>Hotels</Text>
- </ScalePressable>
+ {!expanded ? (
+ <ScalePressable
+ onPress={onLaunchHotelDemo}
+ accessibilityRole="button"
+ accessibilityLabel="Open hotel carousel custom marker demo"
+ style={styles.hotelDemoLaunch}
+ >
+ <Text style={styles.hotelDemoLaunchIcon}>⌁</Text>
+ <Text style={styles.hotelDemoLaunchText}>Hotels</Text>
+ </ScalePressable>
+ ) : null}
<ScrollViewAlso applies to: 472-481, 498-508
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@example/App.tsx` around lines 367 - 381, The dock currently renders two “Open
hotel carousel custom marker demo” controls at the same time, because the Hotels
pill in the dock row is always shown even when the expanded hotel demo banner is
visible. Update the dock rendering in App.tsx so the collapsed-row Hotels button
is conditionally hidden when expanded, matching the existing conditional
behavior used by the neighboring chevron iconButton and keeping
onLaunchHotelDemo exposed only once.
| import { useCallback, useMemo, useRef, useState } from 'react'; | ||
| import { | ||
| ScrollView, | ||
| StyleSheet, | ||
| Text, | ||
| View, | ||
| type NativeScrollEvent, | ||
| type NativeSyntheticEvent, | ||
| } from 'react-native'; | ||
| import ViewShot from 'react-native-view-shot'; | ||
| import { | ||
| MapView, | ||
| type MapViewRef, | ||
| type MarkerDescriptor, | ||
| } from 'react-native-better-maps'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file list =="
git ls-files 'example/examples/*' 'example/**/*view-shot*' 'example/**/*hotelCarousel*' | sed -n '1,200p'
echo "== outline snippet file (if supported) =="
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline example/examples/hotelCarouselSnippet.md || true
fi
echo "== read target snippet =="
wc -l example/examples/hotelCarouselSnippet.md
cat -n example/examples/hotelCarouselSnippet.md | sed -n '1,220p'
echo "== search for captureRef and react-native-view-shot usage =="
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/.git/**' 'captureRef|react-native-view-shot|ViewShot' example . | sed -n '1,240p'Repository: gmi-software/react-native-better-maps
Length of output: 10375
Import captureRef here. The snippet calls captureRef(...) later, but this import block only brings in ViewShot, so the example is broken as a copy-paste sample.
🔧 Minimal fix
-import ViewShot from 'react-native-view-shot';
+import ViewShot, { captureRef } from 'react-native-view-shot';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { useCallback, useMemo, useRef, useState } from 'react'; | |
| import { | |
| ScrollView, | |
| StyleSheet, | |
| Text, | |
| View, | |
| type NativeScrollEvent, | |
| type NativeSyntheticEvent, | |
| } from 'react-native'; | |
| import ViewShot from 'react-native-view-shot'; | |
| import { | |
| MapView, | |
| type MapViewRef, | |
| type MarkerDescriptor, | |
| } from 'react-native-better-maps'; | |
| import { useCallback, useMemo, useRef, useState } from 'react'; | |
| import { | |
| ScrollView, | |
| StyleSheet, | |
| Text, | |
| View, | |
| type NativeScrollEvent, | |
| type NativeSyntheticEvent, | |
| } from 'react-native'; | |
| import ViewShot, { captureRef } from 'react-native-view-shot'; | |
| import { | |
| MapView, | |
| type MapViewRef, | |
| type MarkerDescriptor, | |
| } from 'react-native-better-maps'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@example/examples/hotelCarouselSnippet.md` around lines 12 - 26, The
hotelCarouselSnippet example is missing the captureRef import even though the
snippet uses captureRef later, so the copy-paste sample breaks. Update the
import block alongside ViewShot to bring in captureRef from
react-native-view-shot, and keep the rest of the snippet unchanged so the
referenced capture logic resolves correctly.
| const capture = useCallback(async (id: string, focused: boolean) => { | ||
| const key = `${id}:${focused ? 'f' : 'n'}`; | ||
| if (captured.current.has(key)) return; | ||
|
|
||
| const ref = focused ? focusedRefs.current[id] : normalRefs.current[id]; | ||
| if (ref == null) return; | ||
|
|
||
| captured.current.add(key); | ||
| const uri = await captureRef(ref, { format: 'png', result: 'tmpfile' }); | ||
| setUris((prev) => ({ | ||
| ...prev, | ||
| [id]: { | ||
| normal: focused ? (prev[id]?.normal ?? '') : uri, | ||
| focused: focused ? uri : (prev[id]?.focused ?? ''), | ||
| }, | ||
| })); | ||
| }, []); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don’t mark a pin as captured before rasterization succeeds.
A transient captureRef failure will permanently lock this hotel out because captured.current.add(key) happens before the await. The result is a missing marker with no retry path.
🛠 Safer retry flow
- captured.current.add(key);
- const uri = await captureRef(ref, { format: 'png', result: 'tmpfile' });
- setUris((prev) => ({
- ...prev,
- [id]: {
- normal: focused ? (prev[id]?.normal ?? '') : uri,
- focused: focused ? uri : (prev[id]?.focused ?? ''),
- },
- }));
+ try {
+ const uri = await captureRef(ref, { format: 'png', result: 'tmpfile' });
+ captured.current.add(key);
+ setUris((prev) => ({
+ ...prev,
+ [id]: {
+ normal: focused ? (prev[id]?.normal ?? '') : uri,
+ focused: focused ? uri : (prev[id]?.focused ?? ''),
+ },
+ }));
+ } catch {
+ captured.current.delete(key);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const capture = useCallback(async (id: string, focused: boolean) => { | |
| const key = `${id}:${focused ? 'f' : 'n'}`; | |
| if (captured.current.has(key)) return; | |
| const ref = focused ? focusedRefs.current[id] : normalRefs.current[id]; | |
| if (ref == null) return; | |
| captured.current.add(key); | |
| const uri = await captureRef(ref, { format: 'png', result: 'tmpfile' }); | |
| setUris((prev) => ({ | |
| ...prev, | |
| [id]: { | |
| normal: focused ? (prev[id]?.normal ?? '') : uri, | |
| focused: focused ? uri : (prev[id]?.focused ?? ''), | |
| }, | |
| })); | |
| }, []); | |
| const capture = useCallback(async (id: string, focused: boolean) => { | |
| const key = `${id}:${focused ? 'f' : 'n'}`; | |
| if (captured.current.has(key)) return; | |
| const ref = focused ? focusedRefs.current[id] : normalRefs.current[id]; | |
| if (ref == null) return; | |
| try { | |
| const uri = await captureRef(ref, { format: 'png', result: 'tmpfile' }); | |
| captured.current.add(key); | |
| setUris((prev) => ({ | |
| ...prev, | |
| [id]: { | |
| normal: focused ? (prev[id]?.normal ?? '') : uri, | |
| focused: focused ? uri : (prev[id]?.focused ?? ''), | |
| }, | |
| })); | |
| } catch { | |
| captured.current.delete(key); | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@example/examples/hotelCarouselSnippet.md` around lines 81 - 97, The capture
flow in useCallback’s capture helper marks a pin as captured before
rasterization completes, so a failed captureRef call blocks retries permanently.
Move the captured.current.add(key) update to after captureRef succeeds, and only
then call setUris so the hotel can retry on transient failures. Keep the change
localized to capture and preserve the existing key/ref lookup logic.
| image: { | ||
| uri: focused ? pair.focused : pair.normal, | ||
| width: PIN_WIDTH, | ||
| height: PIN_HEIGHT, | ||
| }, | ||
| anchor: { x: 0.5, y: 0.5 }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Anchor the marker at the pin tip.
y: 0.5 centers the bitmap on the coordinate, but the actual demo uses bottom anchoring so the pin tip lands on the hotel location. This sample will otherwise draw every marker too high.
📍 Fix the anchor
- anchor: { x: 0.5, y: 0.5 },
+ anchor: { x: 0.5, y: 1 },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image: { | |
| uri: focused ? pair.focused : pair.normal, | |
| width: PIN_WIDTH, | |
| height: PIN_HEIGHT, | |
| }, | |
| anchor: { x: 0.5, y: 0.5 }, | |
| image: { | |
| uri: focused ? pair.focused : pair.normal, | |
| width: PIN_WIDTH, | |
| height: PIN_HEIGHT, | |
| }, | |
| anchor: { x: 0.5, y: 1 }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@example/examples/hotelCarouselSnippet.md` around lines 113 - 118, The marker
anchor is centered vertically instead of being pinned to the tip, so update the
anchor used in the hotel carousel marker definition to bottom-align the bitmap.
Adjust the anchor in the marker/image configuration near the focused/normal pin
rendering so the hotel location maps to the pin tip rather than the middle of
the image.
| return ( | ||
| <View collapsable={false}> | ||
| <ViewShot | ||
| options={{ format: 'png', result: 'tmpfile' }} | ||
| captureMode="mount" | ||
| onCapture={handleCapture} | ||
| onCaptureFailure={(error) => { | ||
| console.warn(`Hotel pin capture failed (${hotelId}:${target})`, error); | ||
| }} | ||
| > | ||
| <HotelPin price={price} category={category} focused={focused} /> | ||
| </ViewShot> | ||
| </View> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No recovery path when a pin capture fails — marker vanishes permanently.
onCaptureFailure only warns (line 114-116); it never calls onCaptured/marks the key as done, so a failed capture leaves pendingPinUris.current[hotelId] with an empty string forever. markers (line 195-220) filters out any hotel whose normal/focused uri is empty, so that hotel's marker never renders — not now, not after the 8s timeout, not ever. The timeout at line 229-240 only flips a status string telling the user to rebuild the dev client; it doesn't retry the capture or degrade gracefully (e.g. fallback to a default marker image). On a device where even a single ViewShot capture flakes (which does happen on lower-end Android in particular), that hotel is silently missing from the map with no way to recover short of restarting the screen.
💡 Suggested fallback
onCaptureFailure={(error) => {
console.warn(`Hotel pin capture failed (${hotelId}:${target})`, error);
+ onCaptured(hotelId, target, FALLBACK_PIN_URI);
}}Or at minimum, retry the capture (e.g. by remounting PinCapture with a key bump) instead of leaving the slot permanently empty.
Also applies to: 166-193, 229-240
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@example/HotelCarouselExample.tsx` around lines 108 - 122, The pin capture
flow in PinCapture/HotelCarouselExample currently only logs failures, which
leaves the hotel’s marker URI empty forever and causes markers() to filter it
out permanently. Update the failure path in onCaptureFailure to recover by
either marking the capture as completed with a fallback/default marker or
triggering a retry/remount of PinCapture (for example by bumping a key), and
make sure pendingPinUris.current and the capture completion path are always
advanced so onCaptured-style state does not get stuck. Also review the timeout
handling around the capture status update so it degrades gracefully instead of
only showing a rebuild message.
1 participant
Demonstrates view-shot rasterized RN pins with carousel focus sync, native clustering, and a copy-paste snippet for custom marker workflows.