Feature/remove extrudedbuildings if appconfig true #613
Conversation
…doors SDK to version 4.48.0. Added useEffect to manage extruded buildings visibility based on app config.
…p-template to version 1.89.10 in package-lock.json
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe PR updates the MapsIndoors SDK from version 4.47.1 to 4.48.0 with a corresponding integrity hash, adds a changelog entry documenting a new feature, and implements a React effect to hide Mapbox extruded buildings when configured via appSettings. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✏️ Tip: You can disable this entire section by setting 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 |
…box Extruded buildings via App Config.
…as a string in app config.
![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
🤖 Fix all issues with AI agents
In `@packages/map-template/src/components/MapTemplate/MapTemplate.jsx`:
- Around line 587-595: The effect currently calls
mapsIndoorsInstance.getMapView().setExtrudedBuildingsVisibility(false) unguarded
and can throw for non-Mapbox providers; update the useEffect that watches
mapsIndoorsInstance and appConfig to first read the mapType (from recoil, as
used elsewhere) and only call setExtrudedBuildingsVisibility when mapType ===
mapTypes.Mapbox and appConfig?.appSettings?.removeMapboxExtrudedBuildings ===
true, leaving all other logic unchanged and referencing the existing symbols
mapsIndoorsInstance, appConfig, setExtrudedBuildingsVisibility, mapType and
mapTypes.Mapbox.
- Around line 236-237: The code is loading an unreleased MapsIndoors SDK
(4.48.0) via miSdkApiTag.setAttribute; change the src to the latest official
release (replace '.../mapsindoors-4.48.0.js.gz' with the 4.47.0 artifact) and
update the corresponding integrity attribute to the official SHA384 for v4.47.0
(replace the current hash string). Locate the two setAttribute calls on the
miSdkApiTag and swap the URL and integrity values to the exact v4.47.0 CDN path
and its published integrity hash (obtain the correct hash from the MapsIndoors
release/changelog or CDN metadata) so the injected script references an
official, released SDK.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
packages/map-template/CHANGELOG.mdpackages/map-template/src/components/MapTemplate/MapTemplate.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/map-template/src/components/MapTemplate/MapTemplate.jsx (10)
packages/map-template/src/components/Directions/Directions.jsx (2)
mapsIndoorsInstance(65-65)appConfig(97-97)packages/map-template/src/hooks/useMapBoundsDeterminer.js (1)
mapsIndoorsInstance(50-50)packages/map-template/src/components/Search/Search.jsx (2)
mapsIndoorsInstance(84-84)appConfig(55-55)packages/react-components/src/components/MIMap/MIMap.jsx (2)
mapsIndoorsInstance(56-56)appConfig(60-60)packages/map-template/src/components/LocationDetails/LocationDetails.jsx (2)
mapsIndoorsInstance(75-75)appConfig(108-108)packages/map-template/src/components/ResetKioskViewButton/ResetKioskViewButton.jsx (1)
mapsIndoorsInstance(25-25)packages/map-template/src/components/MapWrapper/MapWrapper.jsx (2)
mapsIndoorsInstance(76-76)appConfig(88-88)packages/map-template/src/components/Search/Categories/Categories.jsx (1)
mapsIndoorsInstance(66-66)packages/map-template/src/components/ViewSelector/ViewSelector.jsx (1)
mapsIndoorsInstance(34-34)packages/map-template/src/components/Wayfinding/Wayfinding.jsx (1)
appConfig(126-126)
🔇 Additional comments (1)
packages/map-template/CHANGELOG.md (1)
8-12: LGTM!The changelog entry follows the established format and correctly documents the new feature as an "Added" item with a minor version bump, consistent with similar previous entries (e.g., 1.89.0 for removing Wayfinding button via App Config).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| miSdkApiTag.setAttribute('src', 'https://app.mapsindoors.com/mapsindoors/js/sdk/4.48.0/mapsindoors-4.48.0.js.gz'); | ||
| miSdkApiTag.setAttribute('integrity', 'sha384-8RD2aLt72/N4zA+aIClVnyqTRD4fGzPBg16I/LvUTuSRhhaf5puTxQ2rdcTZjjyk'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the integrity hash of the MapsIndoors SDK 4.48.0
echo "Fetching SDK and computing integrity hash..."
curl -s 'https://app.mapsindoors.com/mapsindoors/js/sdk/4.48.0/mapsindoors-4.48.0.js.gz' | openssl dgst -sha384 -binary | openssl base64
echo ""
echo "Expected hash: 8RD2aLt72/N4zA+aIClVnyqTRD4fGzPBg16I/LvUTuSRhhaf5puTxQ2rdcTZjjyk"Repository: MapsPeople/web-ui
Length of output: 295
🏁 Script executed:
#!/bin/bash
python3 <<'EOF'
import hashlib
import base64
import urllib.request
url = 'https://app.mapsindoors.com/mapsindoors/js/sdk/4.48.0/mapsindoors-4.48.0.js.gz'
expected_hash = '8RD2aLt72/N4zA+aIClVnyqTRD4fGzPBg16I/LvUTuSRhhaf5puTxQ2rdcTZjjyk'
try:
print("Fetching SDK and computing integrity hash...")
with urllib.request.urlopen(url) as response:
data = response.read()
# Compute SHA-384 hash
sha384_hash = hashlib.sha384(data).digest()
# Convert to base64
computed_hash = base64.b64encode(sha384_hash).decode('ascii')
print(f"Computed hash: {computed_hash}")
print(f"Expected hash: {expected_hash}")
if computed_hash == expected_hash:
print("\n✓ Integrity hash is VALID")
else:
print("\n✗ Integrity hash MISMATCH - hashes do not match")
except Exception as e:
print(f"Error: {e}")
EOFRepository: MapsPeople/web-ui
Length of output: 147
🌐 Web query:
MapsIndoors SDK 4.48.0 integrity hash sha384
💡 Result:
Which MapsIndoors SDK/platform do you mean (Web, iOS, Android, React Native, or other)? I can look up the sha384 integrity hash for v4.48.0 — shall I search now?
🌐 Web query:
MapsIndoors Web SDK 4.48.0 release documentation
💡 Result:
I couldn't find release documentation for MapsIndoors Web SDK v4.48.0. The official Web SDK changelog shows the latest published entry is v4.47.0 (2026-01-09). See the V4 changelog on the MapsIndoors docs site. [1]
Sources
- MapsIndoors Web SDK (V4) changelog — latest entry v4.47.0 (2026-01-09). [1]
Do not use SDK version 4.48.0 — it does not appear to be officially released.
According to the official MapsIndoors Web SDK changelog, v4.47.0 (published 2026-01-09) is the latest released version. There is no release documentation or official build for v4.48.0. Using an unreleased or non-existent SDK version risks broken functionality and potential security issues. Update to the latest official version (v4.47.0) or wait for v4.48.0 to be officially released.
🤖 Prompt for AI Agents
In `@packages/map-template/src/components/MapTemplate/MapTemplate.jsx` around
lines 236 - 237, The code is loading an unreleased MapsIndoors SDK (4.48.0) via
miSdkApiTag.setAttribute; change the src to the latest official release (replace
'.../mapsindoors-4.48.0.js.gz' with the 4.47.0 artifact) and update the
corresponding integrity attribute to the official SHA384 for v4.47.0 (replace
the current hash string). Locate the two setAttribute calls on the miSdkApiTag
and swap the URL and integrity values to the exact v4.47.0 CDN path and its
published integrity hash (obtain the correct hash from the MapsIndoors
release/changelog or CDN metadata) so the injected script references an
official, released SDK.
| /** | ||
| * React on changes to the app config. | ||
| * If the removeMapboxExtrudedBuildings is true, set the extruded buildings visibility to false. | ||
| */ | ||
| useEffect(() => { | ||
| if (mapsIndoorsInstance && appConfig?.appSettings?.removeMapboxExtrudedBuildings === true) { | ||
| mapsIndoorsInstance.getMapView().setExtrudedBuildingsVisibility(false); | ||
| } | ||
| }, [mapsIndoorsInstance, appConfig]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for getMapView() usage patterns in the codebase
rg "getMapView" -A 3 -B 1Repository: MapsPeople/web-ui
Length of output: 8334
🏁 Script executed:
# Look for setExtrudedBuildingsVisibility usage
rg "setExtrudedBuildingsVisibility" -A 2 -B 2Repository: MapsPeople/web-ui
Length of output: 644
🏁 Script executed:
# Check for MapsIndoors SDK information
fd "package.json" | head -5 | xargs grep -l "mapsindoors" 2>/dev/null || echo "Searching for mapsindoors..."
rg "mapsindoors|MapsIndoors" --type json | head -20Repository: MapsPeople/web-ui
Length of output: 1465
🏁 Script executed:
# Look at the full MapTemplate component for context
fd "MapTemplate.jsx" | xargs wc -lRepository: MapsPeople/web-ui
Length of output: 129
🏁 Script executed:
# Check for map provider type checks in the codebase
rg "getProvider|provider.*===|map.*type" packages/map-template/src/components/MapTemplate/ -A 2Repository: MapsPeople/web-ui
Length of output: 1459
🏁 Script executed:
# Check MapsIndoors SDK version in map-template package.json
cat packages/map-template/package.json | grep -A 5 -B 5 "mapsindoors\|sdk"Repository: MapsPeople/web-ui
Length of output: 689
🏁 Script executed:
# Search for any map provider type checking in MapTemplate.jsx itself
rg "mapType|getProvider" packages/map-template/src/components/MapTemplate/MapTemplate.jsxRepository: MapsPeople/web-ui
Length of output: 43
🏁 Script executed:
# Look at how Directions.jsx handles the mapType check to understand the pattern
rg "mapType.*mapbox|mapType.*google" -B 2 -A 5 packages/map-template/src/components/Directions/Directions.jsxRepository: MapsPeople/web-ui
Length of output: 375
🏁 Script executed:
# Check if mapType is available in MapTemplate.jsx or how map provider is determined
rg "mapType\|provider\|getMap()" packages/map-template/src/components/MapTemplate/MapTemplate.jsx | head -20Repository: MapsPeople/web-ui
Length of output: 43
🏁 Script executed:
# Look for where mapType might be set or derived in MapTemplate
sed -n '1,100p' packages/map-template/src/components/MapTemplate/MapTemplate.jsx | rg "const|let|var|import"Repository: MapsPeople/web-ui
Length of output: 4127
🏁 Script executed:
# Search for how the app determines which map provider is being used
rg "mapbox|google" packages/map-template/src/components/MapTemplate/MapTemplate.jsx | head -10Repository: MapsPeople/web-ui
Length of output: 1360
🏁 Script executed:
# Look at how MapWrapper determines or uses the map provider
rg "mapType|mapbox|google" packages/map-template/src/components/MapWrapper/MapWrapper.jsx | head -15Repository: MapsPeople/web-ui
Length of output: 900
🏁 Script executed:
# Search for how the map instance is created to understand provider context
rg "new.*Map|mapboxgl|google.maps" packages/map-template/src/ -A 2 | head -30Repository: MapsPeople/web-ui
Length of output: 2604
🏁 Script executed:
# Check if there's any SDK documentation or comments about setExtrudedBuildingsVisibility
rg "setExtrudedBuildingsVisibility|extrudedBuildings" -B 5 -A 5Repository: MapsPeople/web-ui
Length of output: 1253
Add map provider type check before calling Mapbox-specific API.
The setting removeMapboxExtrudedBuildings is explicitly Mapbox-specific, but the code calls setExtrudedBuildingsVisibility() without verifying the map provider type. On Google Maps instances, this method may not exist, causing a runtime error. The codebase already establishes a pattern for this (see Directions.jsx and MapWrapper.jsx), where provider-specific APIs are guarded with if (mapType === mapTypes.Mapbox) checks.
Suggested fix with provider check
useEffect(() => {
if (mapsIndoorsInstance && appConfig?.appSettings?.removeMapboxExtrudedBuildings === true) {
- mapsIndoorsInstance.getMapView().setExtrudedBuildingsVisibility(false);
+ if (mapType === mapTypes.Mapbox) {
+ mapsIndoorsInstance.getMapView().setExtrudedBuildingsVisibility(false);
+ }
}
- }, [mapsIndoorsInstance, appConfig]);
+ }, [mapsIndoorsInstance, appConfig, mapType]);Ensure mapType is imported from the Recoil state (already available in this component).
🤖 Prompt for AI Agents
In `@packages/map-template/src/components/MapTemplate/MapTemplate.jsx` around
lines 587 - 595, The effect currently calls
mapsIndoorsInstance.getMapView().setExtrudedBuildingsVisibility(false) unguarded
and can throw for non-Mapbox providers; update the useEffect that watches
mapsIndoorsInstance and appConfig to first read the mapType (from recoil, as
used elsewhere) and only call setExtrudedBuildingsVisibility when mapType ===
mapTypes.Mapbox and appConfig?.appSettings?.removeMapboxExtrudedBuildings ===
true, leaving all other logic unchanged and referencing the existing symbols
mapsIndoorsInstance, appConfig, setExtrudedBuildingsVisibility, mapType and
mapTypes.Mapbox.
matbmapspeople
deleted the
feature/remove_extrudedbuildings_if_appconfig_true
branch
Summary by CodeRabbit
Release Notes
New Features
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.