Conversation
📝 WalkthroughWalkthroughThis PR migrates HTML rendering from WebView components to a new cross-platform HtmlRenderer component across multiple pages and modules. It adds style prop support to UI layout components (Box, Card, Center, HStack, VStack) and enhances map initialization with proper coordinate handling, resize observation, and error safeguards. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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: 11
🤖 Fix all issues with AI agents
In `@src/app/call/`[id].tsx:
- Line 312: The HtmlRenderer call should guard against null/undefined values so
it doesn't render "null"/"undefined" as text; update the jsx that uses
HtmlRenderer (e.g., HtmlRenderer html={call.Note ...} and the similar usage for
call.Nature) to pass a safe default like html={call.Note ?? ''} (and
html={call.Nature ?? ''}) so the component always receives a string; preserve
the existing style prop (StyleSheet.flatten([styles.container, { height: 200
}])) when making this change.
- Around line 463-468: The visibility checks for the map incorrectly use
truthiness on coordinates (coordinates.latitude && coordinates.longitude and
!coordinates.latitude || !coordinates.longitude) which fails for valid 0 values;
update both checks to explicit null checks (e.g., ensure coordinates.latitude
!== null && coordinates.longitude !== null when showing the StaticMap and use
!== null checks when gating behavior elsewhere) so 0 is treated as a valid
coordinate — look for references to coordinates.latitude, coordinates.longitude
and the StaticMap usage to apply the change.
In `@src/components/calls/call-card.tsx`:
- Around line 1-5: Remove the stray blank line and reorder the import groups so
external packages and framework imports are grouped before local/project
imports: keep imports for 'lucide-react-native', 'react', and 'react-native'
together, then import '@/components/ui/html-renderer' after them; after
adjusting the ordering, run the project's linter autofix (npx eslint --fix) to
ensure the import sorting (simple-import-sort/imports) passes CI.
In `@src/components/maps/map-view.web.tsx`:
- Around line 243-251: The current map error handler in the newMap.on('error',
...) callback relies on a fragile minified symbol check ('r[3]'); remove that
check and stop matching on minified internals — instead only special-case the
stable "Invalid LngLat" message (if you still want to suppress that silently)
and for all other errors log them at warn level without rethrowing. Update the
handler inside MapView.web.tsx (the newMap.on('error' ...) callback) to drop the
msg.includes('r[3]') branch, keep or add a single stable string check for
"Invalid LngLat", and change the console warning code to consistently log
remaining mapbox-gl errors at warn level.
- Around line 386-421: The effect currently unconditionally skips the first
centerCoordinate update via hasInitialized, which prevents maps without an
initialCenter prop from being moved on first render; change the logic in the
useEffect inside map-view.web.tsx so you only return early when an initialCenter
was actually provided to the MapView. Concretely, detect whether initialCenter
(or a boolean like initialCenterProvided) exists, then update the block around
hasInitialized.current to set hasInitialized.current = true and only return if
initialCenter was provided; otherwise allow the subsequent camera flyTo/easeTo
to run on the first render. Reference: hasInitialized, useEffect, map,
centerCoordinate, initialCenter, animationMode, animationDuration, flyTo,
easeTo.
In `@src/components/maps/static-map.tsx`:
- Around line 30-36: The current truthiness check in the StaticMap component
(variables latitude and longitude) uses "!latitude || !longitude", which treats
0 as falsy and hides valid coordinates; change the guard to explicitly check for
null/undefined (e.g., latitude == null || longitude == null or typeof latitude
=== 'undefined' || latitude === null || ... ) so that 0 is accepted as a valid
coordinate and only missing values trigger the "no_location" fallback.
- Line 40: Remove the non-API props initialCenter and initialZoom from the
Mapbox.MapView invocation (currently passing initialCenter={[longitude,
latitude]} and initialZoom={zoom}); these props are not supported by
`@rnmapbox/maps` and cause inconsistency with the existing Mapbox.Camera usage—use
Mapbox.Camera alone to control position/zoom across native and web; keep the
existing mapStyle, styles.map/height, logoEnabled, attributionEnabled,
compassEnabled, zoomEnabled, and rotateEnabled props intact and ensure Camera is
configured with longitude, latitude and zoom.
In `@src/components/notes/note-details-sheet.tsx`:
- Around line 66-72: Guard against null/undefined selectedNote.Body before
passing it to HtmlRenderer by using a safe fallback (e.g. selectedNote?.Body ??
'' or String(selectedNote?.Body || '')) so HtmlRenderer never receives
undefined/null; update the HtmlRenderer prop in the Note details sheet component
(where HtmlRenderer is rendered with html={selectedNote.Body}) to use the
fallback expression instead.
In `@src/components/protocols/protocol-details-sheet.tsx`:
- Around line 87-95: HtmlRenderer is being passed selectedProtocol.ProtocolText
which can be null/undefined and will cause the iframe body to render the literal
"undefined"/"null"; fix by ensuring the prop is a safe string (e.g., pass an
empty string when missing) or conditionally render the HtmlRenderer only when
ProtocolText is present. Update the call site around HtmlRenderer (the component
using selectedProtocol.ProtocolText and selectedProtocolId) to either use a
default like selectedProtocol.ProtocolText || '' or wrap the HtmlRenderer render
in a guard that verifies selectedProtocol.ProtocolText before rendering.
In `@src/components/ui/html-renderer/index.web.tsx`:
- Around line 109-120: The message handler handleMessage must validate the
message origin/source before acting; update handleMessage to only accept link
messages where event.data?.type === 'html-renderer-link' && event.data.url and
event.source === iframeRef.current?.contentWindow (or additionally check
event.origin if you need origin verification) so only the trusted iframe
triggers onLinkPress or Linking.openURL; reference iframeRef, onLinkPress, and
Linking.openURL in the updated logic and keep the rest of the callback
unchanged.
- Line 135: The inline ESLint disable comment "// eslint-disable-next-line
react-native/no-inline-styles" in the iframe render of the HtmlRenderer web
component is referencing a rule that isn't configured and breaks CI; remove that
invalid disable comment in src/components/ui/html-renderer/index.web.tsx (the
iframe element inside the HtmlRenderer/HtmlRendererWeb render function) and
either move the inline style into a named constant (e.g. iframeStyle) or, if you
must suppress a linter, replace it with a valid web-specific rule disable that
exists in our config; ensure the change targets the iframe render location and
that no invalid eslint-disable lines remain.
🧹 Nitpick comments (13)
src/components/ui/shared-tabs.tsx (1)
17-20: Shared global zustand store will cause cross-instance state conflicts.
useTabStoreis a module-level singleton. If two or moreSharedTabsinstances on the same screen both use theonChangeprop, they'll read and write the sameactiveIndex, causing tabs to sync incorrectly across unrelated tab groups. This is pre-existing, but since this file is being touched, it's worth noting.Consider either removing the global store in favor of local state only, or creating a store per instance (e.g., via
useRef(() => create(...))).Also applies to: 58-59, 65-65
src/components/maps/map-view.web.tsx (2)
306-316:minHeightfallback silently overrides intentional zero-height.
style?.height || style?.minHeight || 100uses falsy coercion, so a deliberateheight: 0from a parent would still produceminHeight: 100. This is likely acceptable for the map use-case (zero-height map is never useful), butstyle?.height ?? style?.minHeight ?? 100would be more precise if numeric zero ever needs to propagate.
593-619: Dual cleanup returns inUserLocationeffect are correct but fragile.The
if (map.loaded())branch falls through to the cleanup at line 613, while theelsebranch returns its own cleanup at line 603. Both paths correctly remove the geolocate control. However, this structure is easy to misread and could be broken by future edits.A single cleanup return at the end would be clearer:
Suggested simplification
if (map.loaded()) { geolocate.trigger(); } else { - const onMapLoad = () => { - geolocate.trigger(); - }; + const onMapLoad = () => geolocate.trigger(); map.on('load', onMapLoad); - - return () => { - try { - map.off('load', onMapLoad); - map.removeControl(geolocate); - } catch { - // map may already be destroyed during route transitions - } - }; } return () => { try { + map.off?.('load', undefined); // no-op if no listener map.removeControl(geolocate); } catch { // map may already be destroyed during route transitions } };Alternatively, store
onMapLoadin a ref or local variable and conditionally clean it up in a single return block.src/components/contacts/__tests__/contact-details-sheet.test.tsx (1)
7-14: Stale testID"mock-webview"in the HtmlRenderer mock.The mock renders with
testID="mock-webview"which is a leftover from the WebView era. While no test in this file queries it, it's inconsistent with the migration and could mislead future test authors. Consider renaming to"html-renderer"for consistency with other test files in this PR.Proposed fix
- HtmlRenderer: (props: any) => <View testID="mock-webview" {...props} />, + HtmlRenderer: (props: any) => <View testID="html-renderer" {...props} />,src/components/contacts/contact-notes-list.tsx (2)
85-152: Consider memoizing thecustomCSSstring.The large
customCSStemplate literal is recreated on every render ofContactNoteCard. Since it only depends oncolorScheme, wrapping it inuseMemowould avoid unnecessary string allocations, especially when rendering multiple cards in a list.
90-90: RedundantonLinkPress— HtmlRenderer already defaults toLinking.openURL.The HtmlRenderer component defaults its link handler to
Linking.openURLwhenonLinkPressis not provided (seeindex.tsxline ~93). Passing(url) => Linking.openURL(url)is functionally identical but creates a new closure each render and enablesonShouldStartLoadWithRequest/navigation interception logic unnecessarily.You can either omit
onLinkPressentirely (to use the default) or keep it if you want the explicit link-interception behavior on native.src/components/protocols/__tests__/protocol-details-sheet.test.tsx (1)
28-63: Overly detailed mock couples tests to HtmlRenderer internals.This mock replicates the full HTML document structure (DOCTYPE, viewport meta, CSS styles) that the real
HtmlRendererproduces. Tests in the "WebView Content" describe block (lines 281-314) then assert on those mock-generated strings — effectively testing the mock, not the component under test.A simpler mock (like the one in
contact-notes-list.test.tsx) that just exposes thehtmlprop would make these tests less brittle while still verifying the correct content is passed through.src/components/ui/card/index.web.tsx (1)
9-11:StyleSheet.flattenalready handles non-array inputs — theArray.isArrayguard is redundant.
StyleSheet.flattenaccepts both single styles and arrays, so you can simplify:Simplification
- const flatStyle = Array.isArray(style) ? StyleSheet.flatten(style) : style; + const flatStyle = StyleSheet.flatten(style);src/components/ui/html-renderer/index.web.tsx (1)
55-65:linkScriptis computed on every render but referenced insideuseMemodeps — consider memoizing it.
linkScriptis derived only fromonLinkPresstruthiness, yet it's a string that's recreated every render, causingfullHtmlto recompute even when nothing meaningful changed. You could fold the script directly into theuseMemobody (keying on!!onLinkPressinstead oflinkScript) to avoid unnecessary recalculations.Suggested simplification
- const linkScript = onLinkPress - ? `<script>...</script>` - : ''; - const fullHtml = useMemo( - () => `...${html}${linkScript}</body>...`, - [html, scrollEnabled, showsVerticalScrollIndicator, resolvedTextColor, resolvedBgColor, customCSS, linkScript] + () => { + const script = onLinkPress + ? `<script> + document.addEventListener('click', function(e) { + var anchor = e.target.closest('a'); + if (anchor && anchor.href) { + e.preventDefault(); + window.parent.postMessage({ type: 'html-renderer-link', url: anchor.href }, '*'); + } + }); + </script>` + : ''; + return `...${html}${script}</body>...`; + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [html, scrollEnabled, showsVerticalScrollIndicator, resolvedTextColor, resolvedBgColor, customCSS, !!onLinkPress] );Also applies to: 102-102
src/components/ui/html-renderer/index.tsx (4)
65-88:fullHtmlis not memoized — inconsistent with the web variant and rebuilt every render.The web variant wraps this template in
useMemo. The native variant should do the same to avoid handing a newsourceobject toWebViewon every render, which can cause unnecessary re-renders or flickers.Proposed fix
+ import React, { useMemo } from 'react'; ... - const fullHtml = ` + const fullHtml = useMemo(() => ` <!DOCTYPE html> <html> ... <body>${html}</body> </html> - `; + `, [html, resolvedTextColor, resolvedBgColor, customCSS]);
90-118: Inline anonymous functions are recreated every render — wrap withuseCallback.
handleLinkPress,onShouldStartLoadWithRequest, andonNavigationStateChangeare all recreated on each render. Wrapping them inuseCallback(or extracting them) would avoid unnecessary WebView prop changes. As per coding guidelines: "Avoid anonymous functions in … event handlers to prevent re-renders."Proposed fix sketch
+ import { useCallback } from 'react'; ... - const handleLinkPress = onLinkPress ?? ((url: string) => Linking.openURL(url)); + const handleLinkPress = useCallback( + (url: string) => { + if (onLinkPress) { onLinkPress(url); } + else { Linking.openURL(url); } + }, + [onLinkPress] + ); + + const handleShouldStartLoad = useCallback( + (request: { url: string }) => { + if (request.url.startsWith('about:') || request.url.startsWith('data:')) return true; + handleLinkPress(request.url); + return false; + }, + [handleLinkPress] + ); + + const handleNavStateChange = useCallback( + (navState: { url: string }) => { + if (navState.url && !navState.url.startsWith('about:') && !navState.url.startsWith('data:')) { + handleLinkPress(navState.url); + } + }, + [handleLinkPress] + );
6-10:THEME_COLORSandHtmlRendererPropsare duplicated between the native and web variants.Both
index.tsxandindex.web.tsxdefine identicalTHEME_COLORSandHtmlRendererProps. Consider extracting them into a shared module (e.g.src/components/ui/html-renderer/types.tsandsrc/components/ui/html-renderer/constants.ts) so changes only need to happen in one place.Also applies to: 12-36
86-86: Consider adding HTML sanitization for defense-in-depth security.While the current implementation mitigates XSS risk by disabling JavaScript by default and using only trusted backend data sources, the HTML string is interpolated directly without sanitization. To follow defense-in-depth principles and protect against potential data corruption at the API layer, consider using a sanitization library (e.g., DOMPurify via
isomorphic-dompurify) on thehtmlprop before interpolation. Alternatively, document that thehtmlprop accepts only trusted content from vetted backend sources.
| }} | ||
| androidLayerType="software" | ||
| /> | ||
| <HtmlRenderer html={call.Note} style={StyleSheet.flatten([styles.container, { height: 200 }])} /> |
There was a problem hiding this comment.
No guard for call.Note being null/undefined.
Same as call.Nature on line 458 — if these fields are null, HtmlRenderer will render "null"/"undefined" as literal text in the HTML body. Consider defaulting: html={call.Note ?? ''}.
🤖 Prompt for AI Agents
In `@src/app/call/`[id].tsx at line 312, The HtmlRenderer call should guard
against null/undefined values so it doesn't render "null"/"undefined" as text;
update the jsx that uses HtmlRenderer (e.g., HtmlRenderer html={call.Note ...}
and the similar usage for call.Nature) to pass a safe default like
html={call.Note ?? ''} (and html={call.Nature ?? ''}) so the component always
receives a string; preserve the existing style prop
(StyleSheet.flatten([styles.container, { height: 200 }])) when making this
change.
| {/* Map - only show when valid coordinates exist */} | ||
| {coordinates.latitude && coordinates.longitude ? ( | ||
| <Box className="w-full"> | ||
| <StaticMap latitude={coordinates.latitude} longitude={coordinates.longitude} address={call.Address} zoom={15} height={200} showUserLocation={true} /> | ||
| </Box> | ||
| ) : null} |
There was a problem hiding this comment.
Truthiness check on coordinates breaks for latitude/longitude of 0.
coordinates.latitude && coordinates.longitude evaluates to false when either value is 0 (a valid coordinate — the equator or prime meridian). The state type is number | null, so you should use a null check instead.
The same issue exists at line 198 (!coordinates.latitude || !coordinates.longitude).
Proposed fix
- {coordinates.latitude && coordinates.longitude ? (
+ {coordinates.latitude !== null && coordinates.longitude !== null ? (And at line 198:
- if (!coordinates.latitude || !coordinates.longitude) {
+ if (coordinates.latitude === null || coordinates.longitude === null) {📝 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.
| {/* Map - only show when valid coordinates exist */} | |
| {coordinates.latitude && coordinates.longitude ? ( | |
| <Box className="w-full"> | |
| <StaticMap latitude={coordinates.latitude} longitude={coordinates.longitude} address={call.Address} zoom={15} height={200} showUserLocation={true} /> | |
| </Box> | |
| ) : null} | |
| {/* Map - only show when valid coordinates exist */} | |
| {coordinates.latitude !== null && coordinates.longitude !== null ? ( | |
| <Box className="w-full"> | |
| <StaticMap latitude={coordinates.latitude} longitude={coordinates.longitude} address={call.Address} zoom={15} height={200} showUserLocation={true} /> | |
| </Box> | |
| ) : null} |
🤖 Prompt for AI Agents
In `@src/app/call/`[id].tsx around lines 463 - 468, The visibility checks for the
map incorrectly use truthiness on coordinates (coordinates.latitude &&
coordinates.longitude and !coordinates.latitude || !coordinates.longitude) which
fails for valid 0 values; update both checks to explicit null checks (e.g.,
ensure coordinates.latitude !== null && coordinates.longitude !== null when
showing the StaticMap and use !== null checks when gating behavior elsewhere) so
0 is treated as a valid coordinate — look for references to
coordinates.latitude, coordinates.longitude and the StaticMap usage to apply the
change.
| import { AlertTriangle, MapPin, Phone } from 'lucide-react-native'; | ||
| import React from 'react'; | ||
| import { StyleSheet } from 'react-native'; | ||
| import WebView from 'react-native-webview'; | ||
|
|
||
| import { HtmlRenderer } from '@/components/ui/html-renderer'; |
There was a problem hiding this comment.
Fix import sort order to unblock CI.
The pipeline reports simple-import-sort/imports: Run autofix to sort these imports. — this is a CI-blocking error. The blank line between lines 4 and 5 likely breaks the expected import grouping.
Proposed fix
import { AlertTriangle, MapPin, Phone } from 'lucide-react-native';
import React from 'react';
import { StyleSheet } from 'react-native';
-
import { HtmlRenderer } from '@/components/ui/html-renderer';
+
import { Box } from '@/components/ui/box';Run the project's lint autofix (npx eslint --fix) to confirm the exact expected ordering.
🧰 Tools
🪛 ESLint
[error] 1-14: Run autofix to sort these imports!
(simple-import-sort/imports)
🪛 GitHub Actions: React Native CI/CD
[error] 1-1: simple-import-sort/imports: Run autofix to sort these imports.
🪛 GitHub Check: test
[failure] 1-1:
Run autofix to sort these imports!
🤖 Prompt for AI Agents
In `@src/components/calls/call-card.tsx` around lines 1 - 5, Remove the stray
blank line and reorder the import groups so external packages and framework
imports are grouped before local/project imports: keep imports for
'lucide-react-native', 'react', and 'react-native' together, then import
'@/components/ui/html-renderer' after them; after adjusting the ordering, run
the project's linter autofix (npx eslint --fix) to ensure the import sorting
(simple-import-sort/imports) passes CI.
| // Suppress non-fatal mapbox-gl error events (e.g. "Invalid LngLat object: (NaN, NaN)") | ||
| // that occur when mouse events fire while the map canvas is resizing. | ||
| newMap.on('error', (e: { error?: Error }) => { | ||
| const msg = e.error?.message ?? ''; | ||
| if (msg.includes('Invalid LngLat') || msg.includes('r[3]')) { | ||
| return; | ||
| } | ||
| console.warn('[MapView.web] mapbox-gl error:', e.error); | ||
| }); |
There was a problem hiding this comment.
Fragile error suppression based on minified symbol name.
The check msg.includes('r[3]') matches against a minified internal variable name from a specific mapbox-gl build. This will silently break (either suppressing unrelated errors or failing to suppress the target one) when mapbox-gl is updated or built differently.
Consider matching on a more stable pattern or simply suppressing all mapbox-gl errors at this level and logging them all at warn level, since this handler is already a last-resort safety net.
Proposed fix
newMap.on('error', (e: { error?: Error }) => {
const msg = e.error?.message ?? '';
- if (msg.includes('Invalid LngLat') || msg.includes('r[3]')) {
+ if (msg.includes('Invalid LngLat') || msg.includes('null is not an object')) {
return;
}
console.warn('[MapView.web] mapbox-gl error:', e.error);
});📝 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.
| // Suppress non-fatal mapbox-gl error events (e.g. "Invalid LngLat object: (NaN, NaN)") | |
| // that occur when mouse events fire while the map canvas is resizing. | |
| newMap.on('error', (e: { error?: Error }) => { | |
| const msg = e.error?.message ?? ''; | |
| if (msg.includes('Invalid LngLat') || msg.includes('r[3]')) { | |
| return; | |
| } | |
| console.warn('[MapView.web] mapbox-gl error:', e.error); | |
| }); | |
| // Suppress non-fatal mapbox-gl error events (e.g. "Invalid LngLat object: (NaN, NaN)") | |
| // that occur when mouse events fire while the map canvas is resizing. | |
| newMap.on('error', (e: { error?: Error }) => { | |
| const msg = e.error?.message ?? ''; | |
| if (msg.includes('Invalid LngLat') || msg.includes('null is not an object')) { | |
| return; | |
| } | |
| console.warn('[MapView.web] mapbox-gl error:', e.error); | |
| }); |
🤖 Prompt for AI Agents
In `@src/components/maps/map-view.web.tsx` around lines 243 - 251, The current map
error handler in the newMap.on('error', ...) callback relies on a fragile
minified symbol check ('r[3]'); remove that check and stop matching on minified
internals — instead only special-case the stable "Invalid LngLat" message (if
you still want to suppress that silently) and for all other errors log them at
warn level without rethrowing. Update the handler inside MapView.web.tsx (the
newMap.on('error' ...) callback) to drop the msg.includes('r[3]') branch, keep
or add a single stable string check for "Invalid LngLat", and change the console
warning code to consistently log remaining mapbox-gl errors at warn level.
| useEffect(() => { | ||
| if (!map) return; | ||
|
|
||
| if (centerCoordinate) { | ||
| map.easeTo( | ||
| { | ||
| center: centerCoordinate, | ||
| zoom: zoomLevel, | ||
| bearing: heading, | ||
| pitch: pitch, | ||
| duration: animationDuration, | ||
| }, | ||
| { _programmatic: true } | ||
| ); | ||
| if ( | ||
| centerCoordinate && | ||
| centerCoordinate.length === 2 && | ||
| isFinite(centerCoordinate[0]) && | ||
| isFinite(centerCoordinate[1]) | ||
| ) { | ||
| // Skip the first render — the MapView already initialized at the correct | ||
| // position via initialCenter/initialZoom, so no programmatic move needed. | ||
| if (!hasInitialized.current) { | ||
| hasInitialized.current = true; | ||
| return; | ||
| } | ||
|
|
||
| // For subsequent coordinate/zoom changes, animate to the new position. | ||
| const cameraOptions = { | ||
| center: centerCoordinate as [number, number], | ||
| zoom: zoomLevel, | ||
| bearing: heading, | ||
| pitch: pitch, | ||
| duration: animationDuration, | ||
| }; | ||
|
|
||
| try { | ||
| if (animationMode === 'flyTo') { | ||
| map.flyTo(cameraOptions, { _programmatic: true }); | ||
| } else { | ||
| map.easeTo(cameraOptions, { _programmatic: true }); | ||
| } | ||
| } catch { | ||
| // Suppress projection-matrix errors during resize/transition | ||
| } | ||
| } | ||
| }, [map, centerCoordinate, zoomLevel, heading, pitch, animationDuration]); | ||
| }, [map, centerCoordinate?.[0], centerCoordinate?.[1], zoomLevel, heading, pitch, animationDuration, animationMode]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f \( -name "*.tsx" -o -name "*.ts" \) | head -20Repository: Resgrid/Unit
Length of output: 753
🏁 Script executed:
rg -n 'initialCenter' --type=tsx --type=ts -C2Repository: Resgrid/Unit
Length of output: 83
🏁 Script executed:
rg -n '<MapView' --type=tsx --type=ts -A15Repository: Resgrid/Unit
Length of output: 83
🏁 Script executed:
rg -n '<Camera' --type=tsx --type=ts -B3 -A10Repository: Resgrid/Unit
Length of output: 83
🏁 Script executed:
fd -e tsx -e ts | head -30Repository: Resgrid/Unit
Length of output: 981
🏁 Script executed:
rg 'initialCenter' -C2Repository: Resgrid/Unit
Length of output: 2663
🏁 Script executed:
rg '<MapView' -A15Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
rg '<Camera' -B3 -A10Repository: Resgrid/Unit
Length of output: 1331
🏁 Script executed:
cat -n src/components/maps/map-view.web.tsx | sed -n '380,425p'Repository: Resgrid/Unit
Length of output: 1686
🏁 Script executed:
rg 'map-view.web' -A5 -B2Repository: Resgrid/Unit
Length of output: 50369
🏁 Script executed:
rg 'from.*map-view' -A5Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
rg 'MapView' src/components -B2 -A15 | head -120Repository: Resgrid/Unit
Length of output: 9290
🏁 Script executed:
cat -n src/components/maps/map-view.web.tsx | sed -n '1,100p'Repository: Resgrid/Unit
Length of output: 3849
🏁 Script executed:
rg '<MapView' -B2 -A20 | head -200Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
rg 'import.*MapView' -B1 -A5Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
fd -name "*.tsx" -exec grep -l "MapView\|Camera" {} \; | head -20Repository: Resgrid/Unit
Length of output: 286
🏁 Script executed:
rg 'Mapbox\.MapView' -B2 -A15Repository: Resgrid/Unit
Length of output: 16333
🏁 Script executed:
rg 'Mapbox\.Camera' -B2 -A10Repository: Resgrid/Unit
Length of output: 6790
🏁 Script executed:
rg 'from.*mapbox'Repository: Resgrid/Unit
Length of output: 709
🏁 Script executed:
fd -name "*.tsx" -exec grep -H "MapView\|Camera" {} \;Repository: Resgrid/Unit
Length of output: 286
🏁 Script executed:
cat -n src/app/\(app\)/index.tsx | sed -n '1,150p'Repository: Resgrid/Unit
Length of output: 6982
🏁 Script executed:
cat -n src/app/\(app\)/index.tsx | sed -n '150,250p'Repository: Resgrid/Unit
Length of output: 3904
🏁 Script executed:
cat -n src/components/maps/map-view.web.tsx | sed -n '200,280p'Repository: Resgrid/Unit
Length of output: 3894
🏁 Script executed:
cat -n src/components/maps/map-view.web.tsx | sed -n '280,360p'Repository: Resgrid/Unit
Length of output: 2951
🏁 Script executed:
cat -n src/components/maps/map-view.web.tsx | sed -n '340,430p'Repository: Resgrid/Unit
Length of output: 3530
Camera effect skips first render when MapView lacks initialCenter—map may render at default US location.
The hasInitialized flag skips the first centerCoordinate update unconditionally, assuming MapView initialized via initialCenter. However, in location-picker.tsx, full-screen-location-picker.tsx, and the main map screen (src/app/(app)/index.tsx), MapView is used without initialCenter while Camera provides centerCoordinate. On first render, the Camera effect returns early (line 399) and never moves the map, leaving it at the default US center [-98.5795, 39.8283] until a subsequent update triggers the camera move.
Either pass initialCenter to all MapView instances that use Camera, or track whether initialCenter was provided and skip the first render only when it was explicitly set.
🤖 Prompt for AI Agents
In `@src/components/maps/map-view.web.tsx` around lines 386 - 421, The effect
currently unconditionally skips the first centerCoordinate update via
hasInitialized, which prevents maps without an initialCenter prop from being
moved on first render; change the logic in the useEffect inside map-view.web.tsx
so you only return early when an initialCenter was actually provided to the
MapView. Concretely, detect whether initialCenter (or a boolean like
initialCenterProvided) exists, then update the block around
hasInitialized.current to set hasInitialized.current = true and only return if
initialCenter was provided; otherwise allow the subsequent camera flyTo/easeTo
to run on the first render. Reference: hasInitialized, useEffect, map,
centerCoordinate, initialCenter, animationMode, animationDuration, flyTo,
easeTo.
| <Box style={[styles.container, { height }]}> | ||
| <Mapbox.MapView style={styles.map} styleURL={mapStyle} logoEnabled={false} attributionEnabled={false} compassEnabled={true} zoomEnabled={true} rotateEnabled={true}> | ||
| <Box style={StyleSheet.flatten([styles.container, { height }])}> | ||
| <Mapbox.MapView style={StyleSheet.flatten([styles.map, { height }])} styleURL={mapStyle} logoEnabled={false} attributionEnabled={false} compassEnabled={true} zoomEnabled={true} rotateEnabled={true} initialCenter={[longitude, latitude]} initialZoom={zoom}> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find mapbox-related files in the components/maps directory
find src/components/maps -type f -name "*mapbox*" -o -name "*map*" | head -20Repository: Resgrid/Unit
Length of output: 250
🏁 Script executed:
# List all files in the maps directory
ls -la src/components/maps/ 2>/dev/null || echo "Directory not found, searching for maps directory"
find . -type d -name "maps" -path "*/components/*" 2>/dev/nullRepository: Resgrid/Unit
Length of output: 876
🏁 Script executed:
# Search for the Mapbox wrapper definition
rg -l "Mapbox" src/components/ | head -10Repository: Resgrid/Unit
Length of output: 424
🏁 Script executed:
cat -n src/components/maps/mapbox.tsRepository: Resgrid/Unit
Length of output: 2776
🏁 Script executed:
cat -n src/components/maps/static-map.tsxRepository: Resgrid/Unit
Length of output: 4505
🏁 Script executed:
cat -n src/components/maps/map-view.native.tsxRepository: Resgrid/Unit
Length of output: 1332
🏁 Script executed:
head -100 src/components/maps/map-view.web.tsxRepository: Resgrid/Unit
Length of output: 3149
🏁 Script executed:
rg "initialCenter|initialZoom" src/components/maps/map-view.web.tsx -A 2 -B 2Repository: Resgrid/Unit
Length of output: 1171
Remove initialCenter and initialZoom props—they're not part of @rnmapbox/maps API.
On native, these props are ignored since the Mapbox.MapView wrapper directly re-exports the native library without support for them. The Mapbox.Camera component (already present at line 41) is the correct approach for positioning on native platforms.
On web, initialCenter and initialZoom are custom props in the wrapper, but passing them alongside the Camera component creates inconsistency and confusion. Use only the Camera component for consistent cross-platform positioning.
🤖 Prompt for AI Agents
In `@src/components/maps/static-map.tsx` at line 40, Remove the non-API props
initialCenter and initialZoom from the Mapbox.MapView invocation (currently
passing initialCenter={[longitude, latitude]} and initialZoom={zoom}); these
props are not supported by `@rnmapbox/maps` and cause inconsistency with the
existing Mapbox.Camera usage—use Mapbox.Camera alone to control position/zoom
across native and web; keep the existing mapStyle, styles.map/height,
logoEnabled, attributionEnabled, compassEnabled, zoomEnabled, and rotateEnabled
props intact and ensure Camera is configured with longitude, latitude and zoom.
| {/* Note content */} | ||
| <Box className="w-full flex-1 rounded-lg bg-gray-50 p-1 dark:bg-gray-700"> | ||
| <WebView | ||
| style={[styles.container, { height: 120 }]} | ||
| originWhitelist={['*']} | ||
| scrollEnabled={false} | ||
| showsVerticalScrollIndicator={false} | ||
| source={{ | ||
| html: ` | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0"> | ||
| <style> | ||
| body { | ||
| color: ${textColor}; | ||
| font-family: system-ui, -apple-system, sans-serif; | ||
| margin: 0; | ||
| padding: 8px; | ||
| font-size: 16px; | ||
| line-height: 1.5; | ||
| background-color: ${colorScheme === 'dark' ? '#374151' : '#F9FAFB'}; | ||
| } | ||
| * { | ||
| max-width: 100%; | ||
| } | ||
| </style> | ||
| </head> | ||
| <body>${selectedNote.Body}</body> | ||
| </html> | ||
| `, | ||
| }} | ||
| androidLayerType="software" | ||
| <HtmlRenderer | ||
| html={selectedNote.Body} | ||
| style={StyleSheet.flatten([styles.container, { height: 120 }])} | ||
| /> | ||
| </Box> |
There was a problem hiding this comment.
Guard against null/undefined Body before passing to HtmlRenderer.
If selectedNote.Body is null or undefined, HtmlRenderer will interpolate it literally into the HTML (<body>undefined</body>). Consider a fallback.
Proposed fix
- <HtmlRenderer
- html={selectedNote.Body}
+ <HtmlRenderer
+ html={selectedNote.Body ?? ''}🤖 Prompt for AI Agents
In `@src/components/notes/note-details-sheet.tsx` around lines 66 - 72, Guard
against null/undefined selectedNote.Body before passing it to HtmlRenderer by
using a safe fallback (e.g. selectedNote?.Body ?? '' or
String(selectedNote?.Body || '')) so HtmlRenderer never receives undefined/null;
update the HtmlRenderer prop in the Note details sheet component (where
HtmlRenderer is rendered with html={selectedNote.Body}) to use the fallback
expression instead.
| {/* Protocol content */} | ||
| <Box className="w-full flex-1 rounded-lg bg-gray-50 p-1 dark:bg-gray-700"> | ||
| <WebView | ||
| key={selectedProtocolId} | ||
| <HtmlRenderer | ||
| html={selectedProtocol.ProtocolText} | ||
| style={styles.container} | ||
| originWhitelist={['*']} | ||
| scrollEnabled={true} | ||
| showsVerticalScrollIndicator={true} | ||
| source={{ | ||
| html: ` | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0"> | ||
| <style> | ||
| body { | ||
| color: ${textColor}; | ||
| font-family: system-ui, -apple-system, sans-serif; | ||
| margin: 0; | ||
| padding: 8px; | ||
| font-size: 16px; | ||
| line-height: 1.5; | ||
| background-color: ${colorScheme === 'dark' ? '#374151' : '#F9FAFB'}; | ||
| } | ||
| * { | ||
| max-width: 100%; | ||
| } | ||
| </style> | ||
| </head> | ||
| <body>${selectedProtocol.ProtocolText}</body> | ||
| </html> | ||
| `, | ||
| }} | ||
| androidLayerType="software" | ||
| rendererKey={selectedProtocolId} | ||
| /> |
There was a problem hiding this comment.
Guard against rendering "undefined" when ProtocolText is missing.
HtmlRenderer receives html={selectedProtocol.ProtocolText}. If ProtocolText is undefined or null, the template literal inside HtmlRenderer (<body>${html}</body>) will render the literal string "undefined" or "null" in the iframe/WebView body.
Consider adding a conditional render or defaulting to an empty string:
Proposed fix (option A: guard render)
{/* Protocol content */}
- <Box className="w-full flex-1 rounded-lg bg-gray-50 p-1 dark:bg-gray-700">
+ {selectedProtocol.ProtocolText ? (
+ <Box className="w-full flex-1 rounded-lg bg-gray-50 p-1 dark:bg-gray-700">
<HtmlRenderer
html={selectedProtocol.ProtocolText}
style={styles.container}
scrollEnabled={true}
showsVerticalScrollIndicator={true}
rendererKey={selectedProtocolId}
/>
</Box>
+ ) : null}Proposed fix (option B: default to empty string)
<HtmlRenderer
- html={selectedProtocol.ProtocolText}
+ html={selectedProtocol.ProtocolText ?? ''}
style={styles.container}📝 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.
| {/* Protocol content */} | |
| <Box className="w-full flex-1 rounded-lg bg-gray-50 p-1 dark:bg-gray-700"> | |
| <WebView | |
| key={selectedProtocolId} | |
| <HtmlRenderer | |
| html={selectedProtocol.ProtocolText} | |
| style={styles.container} | |
| originWhitelist={['*']} | |
| scrollEnabled={true} | |
| showsVerticalScrollIndicator={true} | |
| source={{ | |
| html: ` | |
| <!DOCTYPE html> | |
| <html> | |
| <head> | |
| <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0"> | |
| <style> | |
| body { | |
| color: ${textColor}; | |
| font-family: system-ui, -apple-system, sans-serif; | |
| margin: 0; | |
| padding: 8px; | |
| font-size: 16px; | |
| line-height: 1.5; | |
| background-color: ${colorScheme === 'dark' ? '#374151' : '#F9FAFB'}; | |
| } | |
| * { | |
| max-width: 100%; | |
| } | |
| </style> | |
| </head> | |
| <body>${selectedProtocol.ProtocolText}</body> | |
| </html> | |
| `, | |
| }} | |
| androidLayerType="software" | |
| rendererKey={selectedProtocolId} | |
| /> | |
| {/* Protocol content */} | |
| {selectedProtocol.ProtocolText ? ( | |
| <Box className="w-full flex-1 rounded-lg bg-gray-50 p-1 dark:bg-gray-700"> | |
| <HtmlRenderer | |
| html={selectedProtocol.ProtocolText} | |
| style={styles.container} | |
| scrollEnabled={true} | |
| showsVerticalScrollIndicator={true} | |
| rendererKey={selectedProtocolId} | |
| /> | |
| </Box> | |
| ) : null} |
🤖 Prompt for AI Agents
In `@src/components/protocols/protocol-details-sheet.tsx` around lines 87 - 95,
HtmlRenderer is being passed selectedProtocol.ProtocolText which can be
null/undefined and will cause the iframe body to render the literal
"undefined"/"null"; fix by ensuring the prop is a safe string (e.g., pass an
empty string when missing) or conditionally render the HtmlRenderer only when
ProtocolText is present. Update the call site around HtmlRenderer (the component
using selectedProtocol.ProtocolText and selectedProtocolId) to either use a
default like selectedProtocol.ProtocolText || '' or wrap the HtmlRenderer render
in a guard that verifies selectedProtocol.ProtocolText before rendering.
| const handleMessage = useCallback( | ||
| (event: MessageEvent) => { | ||
| if (event.data?.type === 'html-renderer-link' && event.data.url) { | ||
| if (onLinkPress) { | ||
| onLinkPress(event.data.url); | ||
| } else { | ||
| Linking.openURL(event.data.url); | ||
| } | ||
| } | ||
| }, | ||
| [onLinkPress] | ||
| ); |
There was a problem hiding this comment.
Validate event.origin in the message handler to prevent spoofed link-click messages.
Any same-origin frame or opener can post a { type: 'html-renderer-link', url: '...' } message and trigger onLinkPress or Linking.openURL with an arbitrary URL. Filter by event.source === iframeRef.current?.contentWindow to ensure only the trusted iframe can trigger link handling.
🛡️ Proposed fix
const handleMessage = useCallback(
(event: MessageEvent) => {
+ if (event.source !== iframeRef.current?.contentWindow) return;
if (event.data?.type === 'html-renderer-link' && event.data.url) {
if (onLinkPress) {
onLinkPress(event.data.url);
} else {
Linking.openURL(event.data.url);
}
}
},
- [onLinkPress]
+ [onLinkPress, iframeRef]
);This also gives the currently-unused iframeRef a meaningful purpose.
🤖 Prompt for AI Agents
In `@src/components/ui/html-renderer/index.web.tsx` around lines 109 - 120, The
message handler handleMessage must validate the message origin/source before
acting; update handleMessage to only accept link messages where event.data?.type
=== 'html-renderer-link' && event.data.url and event.source ===
iframeRef.current?.contentWindow (or additionally check event.origin if you need
origin verification) so only the trusted iframe triggers onLinkPress or
Linking.openURL; reference iframeRef, onLinkPress, and Linking.openURL in the
updated logic and keep the rest of the callback unchanged.
|
|
||
| return ( | ||
| <View style={flatStyle} key={rendererKey}> | ||
| {/* eslint-disable-next-line react-native/no-inline-styles -- iframe not a RN element */} |
There was a problem hiding this comment.
ESLint rule react-native/no-inline-styles is not configured — CI fails.
The static analysis reports that the rule definition was not found, which causes the test check to fail. Since this is a .web.tsx file that renders a native <iframe> element (not a React Native component), the rule is irrelevant here. Remove the disable comment or replace it with a valid rule.
Proposed fix
- {/* eslint-disable-next-line react-native/no-inline-styles -- iframe not a RN element */}
<iframe📝 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.
| {/* eslint-disable-next-line react-native/no-inline-styles -- iframe not a RN element */} |
🧰 Tools
🪛 ESLint
[error] 135-135: Definition for rule 'react-native/no-inline-styles' was not found.
(react-native/no-inline-styles)
🪛 GitHub Check: test
[failure] 135-135:
Definition for rule 'react-native/no-inline-styles' was not found
🤖 Prompt for AI Agents
In `@src/components/ui/html-renderer/index.web.tsx` at line 135, The inline ESLint
disable comment "// eslint-disable-next-line react-native/no-inline-styles" in
the iframe render of the HtmlRenderer web component is referencing a rule that
isn't configured and breaks CI; remove that invalid disable comment in
src/components/ui/html-renderer/index.web.tsx (the iframe element inside the
HtmlRenderer/HtmlRendererWeb render function) and either move the inline style
into a named constant (e.g. iframeStyle) or, if you must suppress a linter,
replace it with a valid web-specific rule disable that exists in our config;
ensure the change targets the iframe render location and that no invalid
eslint-disable lines remain.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes & Improvements