Add BitMap component (#10110)#12360
Open
msynk wants to merge 6 commits into
Open
Conversation
msynk
requested review from
Cyrus-Sushiant,
Copilot,
mhrastegari,
yasmoradi and
zahra-yousefi
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a provider-pluggable BitMap Blazor component with C# contracts/models, IJSRuntime wrappers, shared TypeScript helpers and GL base, multiple TypeScript provider implementations, SCSS styles, a demo page with 13 examples, navigation entry, and tests validating JS interop and lifecycle. ChangesBitMap Component
Estimated code review effort:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a new generic, provider-pluggable BitMap<TMapProvider> component in Bit.BlazorUI.Extras with seven map backends (Leaflet, MapLibre GL, Mapbox GL, OpenLayers, ArcGIS, Azure Maps, Cesium), exposing a unified imperative API for markers, vector layers, GeoJSON, tile overlays, and view manipulation, plus a full demo page. Closes #10110.
Changes:
- Adds
BitMapRazor component, supporting types (BitMapLatLng,Bounds,Marker,VectorPathStyle,TileOverlay,ViewState, callback args),IBitMapProvider/BitMapProviderBase, and concrete provider classes for each backend. - Adds the corresponding TypeScript providers (
BitMapLeaflet,BitMapGlBase+ MapLibre/Mapbox wrappers,BitMapOpenLayers,BitMapArcGis,BitMapAzureMaps,BitMapCesium) plus shared helpers, and wires SCSS viaextra-components.scss. - Registers the demo page, navigation entry, JS interop extensions, and ignores
.kiro/.cursoreditor folders.
Reviewed changes
Copilot reviewed 33 out of 40 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor[.cs] | New generic component + imperative API and JSInvokable bridge methods. |
| src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap{LatLng,LatLngBounds,Marker,TileOverlay,VectorPathStyle,ViewState,MarkerDragEndArgs}.cs | Shared domain types/payloads. |
| src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/IBitMapProvider.cs, BitMapProviderBase.cs | Provider abstraction and common options. |
| src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/Bit*MapProvider.cs | Seven concrete C# provider configurations. |
| src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMap*.ts | TypeScript runtime adapters (Leaflet, GL base + MapLibre/Mapbox, OpenLayers, ArcGIS, AzureMaps, Cesium, shared helpers). |
| src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapJsRuntimeExtensions.cs | Provider-agnostic JS interop helpers. |
| src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.scss + Styles/extra-components.scss | Component styles and aggregator. |
| src/BlazorUI/Demo/.../Pages/Components/Extras/Map/BitMapDemo.razor[.cs] | Demo page with 13 examples covering all providers. |
| src/BlazorUI/Demo/.../Shared/MainLayout.razor.NavItems.cs | Adds "Map" entry to demo nav. |
| .gitignore | Ignores .kiro and .cursor editor directories. |
Comments suppressed due to low confidence (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.ts:156
AddMarkerfirst adds the new feature tos.markerSource, then checks if a marker with the same id already exists and removes it. Because the new feature has just been registered with the same id (f.setId(markerId)),s.markerSource.addFeature(f)may throw an "AssertionError: Feature already added to the source" before the previous entry is cleaned up. Removes.markers[markerId]from the source before adding the new feature, mirroring the pattern used in the other providers.
f.setId(markerId);
f.setStyle(BitMapOpenLayers._markerStyle(ol, opts));
s.markerSource.addFeature(f);
const existing = s.markers[markerId];
if (existing) try { s.markerSource.removeFeature(existing); } catch { /* ignore */ }
s.markers[markerId] = f;
}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor (1)
203-203: ⚡ Quick winAdd explicit labels for token/key fields.
Line 203 and Line 233 currently rely on placeholders only. Add
Labelto improve screen-reader clarity and form accessibility.Proposed change
- <BitTextField `@bind-Value`="mapboxToken" Placeholder="pk.eyJ1…" Style="margin-bottom:0.5rem;max-width:480px" /> + <BitTextField Label="Mapbox access token" `@bind-Value`="mapboxToken" Placeholder="pk.eyJ1…" Style="margin-bottom:0.5rem;max-width:480px" /> ... - <BitTextField `@bind-Value`="azureMapsKey" Placeholder="Azure Maps subscription key" Style="margin-bottom:0.5rem;max-width:480px" /> + <BitTextField Label="Azure Maps subscription key" `@bind-Value`="azureMapsKey" Placeholder="Azure Maps subscription key" Style="margin-bottom:0.5rem;max-width:480px" />Also applies to: 233-233
🤖 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 `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor` at line 203, Add explicit Label attributes to the BitTextField components that currently only use Placeholder so screen readers can announce them; locate the BitTextField instances bound to mapboxToken (BitTextField `@bind-Value`="mapboxToken") and the other token/key field around the second occurrence (the BitTextField at line ~233) and add a meaningful Label (e.g., Label="Mapbox token" and Label="MapTiler key") to each component so they are accessible to assistive tech.
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cs`:
- Around line 302-308: OnProviderSet currently returns early when Provider is
null, which prevents reverting to a default provider after initialization;
change OnProviderSet so that when _initialized is true and Provider is null you
create/assign a default TMapProvider (or call ProviderFactory/default factory)
and proceed to call _js.BitMapSync(JsObject, _Id,
Provider.BuildOptionsPayload()) and set _activeProvider = Provider; ensure the
logic still skips work only when nothing changed (e.g., _activeProvider equals
Provider) but handles the "post-init reset to null" case by reinstating the
default provider and resending options.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.scss`:
- Around line 9-13: The CSS rule for .bit-map-canvas removes the browser focus
outline (outline: none), which hides keyboard focus for the focusable map
canvas; restore visible keyboard focus by removing the outline: none declaration
(or replace it with a sensible focus style such as outline: auto or add a
:focus/:focus-visible rule to provide an accessible focus ring) so that the
.bit-map-canvas element shows a clear focus indicator when tabbed to.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs`:
- Around line 35-41: In BuildOptionsPayload, validate the basemap/key combo
before returning the payload: if BasemapId is not "osm" (case-insensitive) and
ApiKey is null/empty, throw a clear ArgumentException (or
InvalidOperationException) with a message explaining that non‑OSM basemaps
require a valid ApiKey; use the existing GetCommonOptions(), BasemapId and
ApiKey symbols to locate where to add the check so invalid combinations fail
fast rather than emitting a runtime JS config error.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitAzureMapsMapProvider.cs`:
- Around line 10-37: The SubscriptionKey property is required but currently can
be empty and is passed into BuildOptionsPayload; update the BuildOptionsPayload
method in BitAzureMapsMapProvider to validate SubscriptionKey
(non-empty/non-whitespace) and throw a clear exception (e.g., ArgumentException)
if it's missing before calling GetCommonOptions; then only add
common["subscriptionKey"] = SubscriptionKey after validation so initialization
fails fast and provides a deterministic error.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts`:
- Around line 232-267: addGeoJson currently only handles Point, LineString, and
Polygon and silently drops MultiPoint, MultiLineString, MultiPolygon and
GeometryCollection; update addGeoJson to recursively expand multi-geometries and
GeometryCollection into their constituent geometries and create graphics for
each piece using the same attribute merge (props) and existing symbol helpers
(_lineSym, _fillSym, Point symbol); for MultiPoint create a Graphic per
coordinate using esri.Point, for MultiLineString create a Polyline per
coordinate array, for MultiPolygon create one or more Polygon graphics using
rings (maintaining spatialReference { wkid: 4326 }), and for GeometryCollection
iterate its geometries and dispatch by type; keep using esri.Graphic and ensure
all created graphics are added to s.view.graphics and included in
s.geoJsonLayers[layerId] so nothing is silently dropped.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.ts`:
- Around line 265-296: When adding a GeoJSON/vector layer (in addGeoJson and the
similar _addVectorLayer code paths referenced around lines 344-356) we currently
overwrite s.geoJsonLayers[layerId] without removing the previously-added map
layers and source, leaking stale layers; update both functions to first check
s.geoJsonLayers[layerId], and if present iterate its layerIds to remove each
layer from s.map.layers (s.map.layers.remove(...)) and remove the previous
source from s.map.sources (and dispose/clear the DataSource if appropriate),
then delete the registry entry before creating and registering the new
DataSource and layers so removeLayer will correctly clean up the latest
instance.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs`:
- Around line 10-41: The BuildOptionsPayload currently forwards StyleUrl and
AccessToken unchanged which allows a "mapbox://" style to be used with an empty
AccessToken; update BitMapboxMapProvider.BuildOptionsPayload to validate and
guard this: detect when StyleUrl starts with "mapbox://" and if
string.IsNullOrWhiteSpace(AccessToken) either throw a clear ArgumentException
(or return/fallback to a non-mapbox CDN style) so consumers get a deterministic
failure instead of runtime errors; modify the method to check
StyleUrl.StartsWith("mapbox://", StringComparison.OrdinalIgnoreCase) and handle
the empty AccessToken case before adding values to the common dictionary.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts`:
- Around line 258-273: addGeoJson only sets _bmLayerId on the DataSource, but
the click handler expects metadata on each picked Entity, so GeoJSON features
never trigger OnGeoJsonFeatureClick; update BitMapCesium.addGeoJson (and the
similar routine around lines 381-395) to iterate ds.entities.values and for each
entity set entity._bmLayerId = layerId and entity._bmKind = 'geojson' (or
whatever kind the click handler checks), and also apply any per-entity
style/metadata as needed so the pick logic can detect GeoJSON features; keep the
existing DataSource tagging but ensure every Entity gets the same metadata after
GeoJsonDataSource.load completes and before adding to viewer.
- Around line 150-160: The add/remove order is reversed causing Cesium to throw
on duplicate IDs; locate the marker creation blocks (uses s.viewer.entities.add,
id `bm-marker-${id}-${markerId}`, variables ent, existing and
s.markers[markerId]) and remove any existing entity first (check existing =
s.markers[markerId]; if (existing) try { s.viewer.entities.remove(existing); }
catch {/*ignore*/}) before calling s.viewer.entities.add(...) and then assign
s.markers[markerId] = ent; apply the same change to the other similar blocks
referenced (around the ranges mentioned).
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.ts`:
- Around line 292-297: The click handlers attached via s.map.on('click', ...)
(the handler created near lines with layerId/fillId/lineId) are never
unregistered, so update the code to store these handler references in the
vectorCatalog entry for the vector (e.g., keep the handler function(s) under
vectorCatalog[layerId].clickHandler or similar) and then, inside _removeVector,
call s.map.off('click', fillId, handler) and s.map.off('click', lineId, handler)
(or the exact stored handler refs) before removing the layers and source; ensure
you reference the same handler function objects when calling map.off so
listeners are fully removed and will not persist when layers are re-added.
- Line 177: The popup content is being injected unsafely via
setHTML(String(opts.popupHtml)) in BitMapGlBase; guard against XSS by checking
the source and type of opts.popupHtml and either (a) use marker.setPopup(new
gl.Popup({ offset: 25 }).setText(String(opts.popupHtml))) for
untrusted/plain-text input or (b) run opts.popupHtml through a
sanitizer/whitelist before calling setHTML; update the code path that constructs
the Popup in BitMapGlBase to choose setText when content is untrusted or
sanitize/validate the HTML first and only call setHTML when it is known-safe.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.ts`:
- Around line 34-48: The provider never maps public interaction toggles
(scrollWheelZoom, dragging, doubleClickZoom, keyboardNavigation) into
OpenLayers, so update the ol.Map creation and the sync path to pass and toggle
ol.interaction.defaults based on the component options object (o) — e.g., set
the map option interactions: ol.interaction.defaults({mouseWheelZoom:
o.scrollWheelZoom !== false, dragPan: o.dragging !== false, doubleClickZoom:
o.doubleClickZoom !== false, keyboard: o.keyboardNavigation !== false}) when
constructing the map in init, and in sync (or the equivalent update handler)
enable/disable those interactions on the existing map instance (map) to reflect
changes to o.scrollWheelZoom, o.dragging, o.doubleClickZoom, and
o.keyboardNavigation.
- Around line 230-238: The style function styleFn currently returns ol.Style({
stroke, fill }) for Point and MultiPoint geometries which doesn't render points;
update styleFn to check for 'Point' and 'MultiPoint' types and return an
ol.Style that includes an image (e.g., ol.style.Circle with radius, fill/stroke)
alongside stroke/fill so point features render; modify the branch where you
create new ol.Style({ stroke, fill }) to instead create new ol.Style({ image: /*
Circle style using fill/stroke */, stroke, fill }) ensuring compatibility with
ol.VectorLayer and the existing features variable.
---
Nitpick comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor`:
- Line 203: Add explicit Label attributes to the BitTextField components that
currently only use Placeholder so screen readers can announce them; locate the
BitTextField instances bound to mapboxToken (BitTextField
`@bind-Value`="mapboxToken") and the other token/key field around the second
occurrence (the BitTextField at line ~233) and add a meaningful Label (e.g.,
Label="Mapbox token" and Label="MapTiler key") to each component so they are
accessible to assistive tech.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7aaa1a15-5612-413b-a9bd-8deefa307b2b
⛔ Files ignored due to path filters (5)
src/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-shadow.pngis excluded by!**/*.png
📒 Files selected for processing (35)
.gitignoresrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLng.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLngBounds.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarker.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarkerDragEndArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapProviderBase.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapTileOverlay.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapVectorPathStyle.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapViewState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/IBitMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitAzureMapsMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitCesiumMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitLeafletMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLibreMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapLibre.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapbox.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitOpenLayersMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/leaflet-1.9.4.jssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (2)
src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs (1)
34-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a null/empty guard for
StyleUrlbeforeStartsWith.Line 36 can throw if
StyleUrlis null at runtime. Validate it first and fail with a deterministic message.Suggested fix
public override object BuildOptionsPayload() { + if (string.IsNullOrWhiteSpace(StyleUrl)) + { + throw new InvalidOperationException("BitMapboxMapProvider: StyleUrl is required."); + } + if (StyleUrl.StartsWith("mapbox://", StringComparison.OrdinalIgnoreCase) && string.IsNullOrWhiteSpace(AccessToken)) { throw new InvalidOperationException( $"BitMapboxMapProvider: An AccessToken is required when using a 'mapbox://' style ('{StyleUrl}'). " + "Provide a valid Mapbox access token or use a non-Mapbox style URL."); }🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs` around lines 34 - 41, In BuildOptionsPayload, guard against null/empty StyleUrl before calling StyleUrl.StartsWith to avoid a NullReferenceException: check string.IsNullOrWhiteSpace(StyleUrl) first and throw a clear InvalidOperationException (similar to the existing AccessToken check) when StyleUrl is null/empty; only then evaluate StartsWith("mapbox://", StringComparison.OrdinalIgnoreCase) and proceed with the current AccessToken validation (reference: BuildOptionsPayload, StyleUrl, AccessToken).src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs (1)
37-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden
BasemapIdvalidation and make OSM check case-insensitive.Line 37 treats
"OSM"as non-OSM and may enforceApiKeyincorrectly. Also guard emptyBasemapIdearly.Suggested fix
public override object BuildOptionsPayload() { - if (BasemapId is not "osm" && string.IsNullOrWhiteSpace(ApiKey)) + if (string.IsNullOrWhiteSpace(BasemapId)) + { + throw new InvalidOperationException("BitArcGisMapProvider: BasemapId is required."); + } + + if (!string.Equals(BasemapId, "osm", StringComparison.OrdinalIgnoreCase) && string.IsNullOrWhiteSpace(ApiKey)) { throw new InvalidOperationException( $"BitArcGisMapProvider: An ApiKey is required for the '{BasemapId}' basemap. " + "Only the 'osm' basemap works without an API 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs` around lines 37 - 42, Validate BasemapId in BitArcGisMapProvider by first guarding against null/empty/whitespace (throwing a clear InvalidOperationException if missing), then perform the OSM check using a case-insensitive comparison (e.g., compare BasemapId to "osm" with StringComparison.OrdinalIgnoreCase or normalize with ToLowerInvariant/Trim) so values like "OSM" or extra whitespace don't incorrectly require an ApiKey, and only throw the existing ApiKey-required InvalidOperationException when the normalized BasemapId is not "osm" and ApiKey is null/whitespace.
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapVectorPathStyle.cs`:
- Around line 14-22: The Opacity and FillOpacity properties on
BitMapVectorPathStyle accept any double but are documented as 0–1; clamp their
values to the valid range by implementing bounded setters for Opacity and
FillOpacity (or validate and clamp right before serialization) so any assigned
value <0 becomes 0 and >1 becomes 1; update the property implementations on the
BitMapVectorPathStyle class (the Opacity and FillOpacity members) to enforce
this clamping and preserve existing defaults.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitCesiumMapProvider.cs`:
- Around line 65-82: In BuildOptionsPayload, enforce Cesium token-dependent
options before returning the payload: check IonAccessToken (in
BuildOptionsPayload) and if it's null/empty disable token-required features (set
TerrainEnabled to false and override any Bing/ion-dependent ImageryStyle to a
safe default) so the payload never enables terrain or Bing imagery when
IonAccessToken is missing; update the logic around the TerrainEnabled,
ImageryStyle and IonAccessToken assignments in BuildOptionsPayload accordingly.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts`:
- Around line 64-72: BitMapArcGis.sync currently updates
center/zoom/basemap/scale but doesn't reapply interaction flags (e.g.,
scrollWheelZoom, dragging) that init sets, so update BitMapArcGis.sync to read
the same options (o.scrollWheelZoom, o.dragging or their exact names used in
init) and reapply them to the stored map/view instance s (the same
enable/disable logic used in BitMapArcGis.init), ensuring interaction state is
toggled on s.view or s.map the same way as during initialization; reference
BitMapArcGis.sync, BitMapArcGis.init, BitMapArcGis._maps and
BitMapArcGis._ensureScaleBar when making the change.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.ts`:
- Around line 318-327: The addTileOverlay implementation
(BitMapAzureMaps.addTileOverlay) currently overwrites s.tileOverlays[opts.id]
without removing any previous layer; before creating the new TileLayer (tlId),
check s.tileOverlays[opts.id] for an existing layer id, locate that layer in
s.map.layers (by the stored layer id) and remove it from the map (and from the
registry) so you don't leave stale state, then proceed to create and add the new
TileLayer and set s.tileOverlays[opts.id] = tlId.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts`:
- Around line 133-143: The SetView and FlyTo methods always default zoom to 4,
causing unintended zoom changes when zoom is omitted; change them to preserve
the current camera altitude when zoom is null by reading the current camera
height (e.g. from s.viewer.camera.positionCartographic.height or equivalent) and
only call BitMapCesium._zoomToAltitude when zoom is provided. Update both
BitMapCesium.setView and BitMapCesium.flyTo to compute altitude = zoom != null ?
BitMapCesium._zoomToAltitude(zoom) : currentCameraHeight and use that altitude
for Cesium.Cartesian3.fromDegrees.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.ts`:
- Around line 328-344: The addTileOverlay method currently returns early when a
source already exists, so new URL/opacity/maxZoom aren't applied; modify
addTileOverlay (and use the instance from BitMapGlBase._require) to detect
existing tileOverlayCatalog[opts.id] or s.map.getSource(sourceId) and remove the
old layer and source (s.map.removeLayer(layerId) and
s.map.removeSource(sourceId)) before adding the new source and layer, or
alternatively update the existing layer's paint and source options via
s.map.setPaintProperty and by recreating the source with the new tiles/maxzoom;
ensure you keep tileOverlayCatalog[opts.id] in sync with the new
sourceId/layerId after replacing/updating so subsequent calls reflect the new
URL/opacity/maxZoom.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.ts`:
- Around line 90-93: The sync currently forces a hardcoded center by building c
= [o.center?.lat ?? 51.505, o.center?.lng ?? -0.09] and always calling
s.map.setView(c, z), which can jump the map on option-only updates; change sync
so it only calls s.map.setView when options.center is explicitly provided (use
o.center and o.zoom or fall back to current s.map.getZoom for the z argument),
and if only options.zoom is provided call s.map.setZoom(o.zoom, { animate: false
}) instead of rebuilding a default center; reference variables/methods:
function/method sync in BitMapLeaflet.ts, variables o.center, o.zoom, c, z, and
calls s.map.setView / s.map.setZoom.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.ts`:
- Line 181: BitMapOpenLayers.openMarkerPopup currently silently no-ops; change
it to surface a clear failure by throwing a descriptive Error (or at minimum
logging and throwing) so caller code knows OpenLayers doesn't support this
built-in popup behavior; update the method BitMapOpenLayers.openMarkerPopup to
throw an Error that includes context (the passed _id and _markerId and that
OpenLayers requires a custom overlay/implementation) so provider-agnostic
callers receive a visible failure instead of silent no-op.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.ts`:
- Around line 45-53: The circleRingLngLat function can divide by points causing
NaN when points <= 0; add a guard at the start of circleRingLngLat to validate
and normalize points (e.g., if points is not a positive integer, set points =
Math.max(1, Math.floor(points)) or throw a clear error) before using it in the
bearing calculation and loop; ensure the rest of the function uses the
normalized points variable so the division (bearing = (i / points) * 2 *
Math.PI) and loop bounds are safe.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs`:
- Around line 37-42: Validate BasemapId in BitArcGisMapProvider by first
guarding against null/empty/whitespace (throwing a clear
InvalidOperationException if missing), then perform the OSM check using a
case-insensitive comparison (e.g., compare BasemapId to "osm" with
StringComparison.OrdinalIgnoreCase or normalize with ToLowerInvariant/Trim) so
values like "OSM" or extra whitespace don't incorrectly require an ApiKey, and
only throw the existing ApiKey-required InvalidOperationException when the
normalized BasemapId is not "osm" and ApiKey is null/whitespace.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs`:
- Around line 34-41: In BuildOptionsPayload, guard against null/empty StyleUrl
before calling StyleUrl.StartsWith to avoid a NullReferenceException: check
string.IsNullOrWhiteSpace(StyleUrl) first and throw a clear
InvalidOperationException (similar to the existing AccessToken check) when
StyleUrl is null/empty; only then evaluate StartsWith("mapbox://",
StringComparison.OrdinalIgnoreCase) and proceed with the current AccessToken
validation (reference: BuildOptionsPayload, StyleUrl, AccessToken).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5edad8b8-6807-4608-8452-a84d84564da4
⛔ Files ignored due to path filters (5)
src/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-shadow.pngis excluded by!**/*.png
📒 Files selected for processing (35)
.gitignoresrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLng.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLngBounds.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarker.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarkerDragEndArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapProviderBase.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapTileOverlay.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapVectorPathStyle.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapViewState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/IBitMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitAzureMapsMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitCesiumMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitLeafletMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLibreMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapLibre.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapbox.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitOpenLayersMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/leaflet-1.9.4.jssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs (1)
34-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a null/empty guard for
StyleUrlbeforeStartsWith.
StartsWithon a nullStyleUrlcauses an immediate runtime failure instead of a clear configuration error.Proposed fix
public override object BuildOptionsPayload() { + if (string.IsNullOrWhiteSpace(StyleUrl)) + { + throw new InvalidOperationException("BitMapboxMapProvider: StyleUrl is required."); + } + if (StyleUrl.StartsWith("mapbox://", StringComparison.OrdinalIgnoreCase) && string.IsNullOrWhiteSpace(AccessToken)) { throw new InvalidOperationException( $"BitMapboxMapProvider: An AccessToken is required when using a 'mapbox://' style ('{StyleUrl}'). " + "Provide a valid Mapbox access token or use a non-Mapbox style URL."); }🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs` around lines 34 - 37, In BuildOptionsPayload(), guard StyleUrl for null/empty before calling StyleUrl.StartsWith(...) — e.g., check string.IsNullOrWhiteSpace(StyleUrl) first and handle it (throw a clear configuration exception or return an appropriate error) so the subsequent StartsWith call is never invoked on null; also retain the existing AccessToken check and ensure the error message references StyleUrl and AccessToken for clarity.
🤖 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
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitCesiumMapProvider.cs`:
- Around line 67-77: The code treats whitespace-only IonAccessToken as present;
update the token presence check in the BitCesiumMapProvider by replacing
string.IsNullOrEmpty(IonAccessToken) with
string.IsNullOrWhiteSpace(IonAccessToken) (i.e., compute hasToken =
!string.IsNullOrWhiteSpace(IonAccessToken)) so TerrainEnabled/terrainEnabled and
imageryStyle logic correctly treat whitespace tokens as missing, and ensure
common["ionAccessToken"] is only set when hasToken is true (otherwise leave it
null/empty).
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitLeafletMapProvider.cs`:
- Around line 20-21: The TileOpacity property on BitLeafletMapProvider currently
allows any double but is documented as 0–1; update the TileOpacity property
setter (the TileOpacity auto-property) to clamp incoming values to the [0,1]
range (e.g., set to 0 if value < 0, to 1 if value > 1, otherwise keep value) so
the component honors its documented contract and use that clamped value
internally when raising any change notifications or rendering updates.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts`:
- Around line 323-335: In addTileOverlay (BitMapArcGis.addTileOverlay) remove
any existing tile layer from s.tileOverlays[opts.id] (and call s.map.remove on
it) before creating/adding the new esri.WebTileLayer; use
BitMapArcGis._require(id) to get s, check const existing =
s.tileOverlays[opts.id] and if present call s.map.remove(existing) and clear
s.tileOverlays[opts.id] before instantiating tl and calling s.map.add(tl), then
assign s.tileOverlays[opts.id] = tl.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitOpenLayersMapProvider.cs`:
- Around line 19-20: TileOpacity currently allows values outside the documented
0–1 range; change the auto-property to use a private backing field and clamp
assignments in the TileOpacity setter (e.g., use Math.Clamp(value, 0.0, 1.0)) so
any set (including the default) is constrained to 0..1 and keep the default at
1.0; update references to the property if they relied on direct field access.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs`:
- Around line 34-37: In BuildOptionsPayload(), guard StyleUrl for null/empty
before calling StyleUrl.StartsWith(...) — e.g., check
string.IsNullOrWhiteSpace(StyleUrl) first and handle it (throw a clear
configuration exception or return an appropriate error) so the subsequent
StartsWith call is never invoked on null; also retain the existing AccessToken
check and ensure the error message references StyleUrl and AccessToken for
clarity.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 581b08e6-756e-448c-8bbc-f77f5a867969
⛔ Files ignored due to path filters (5)
src/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-shadow.pngis excluded by!**/*.png
📒 Files selected for processing (35)
.gitignoresrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLng.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLngBounds.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarker.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarkerDragEndArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapProviderBase.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapTileOverlay.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapVectorPathStyle.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapViewState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/IBitMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitAzureMapsMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitCesiumMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitLeafletMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLibreMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapLibre.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapbox.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitOpenLayersMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/leaflet-1.9.4.jssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs
mhrastegari
approved these changes
May 18, 2026
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs (1)
37-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a case-insensitive
osmcheck in the ApiKey guard.
BasemapId is not "osm"is case-sensitive, so values like"OSM"fail incorrectly and require an API key.🔧 Proposed fix
- if (BasemapId is not "osm" && string.IsNullOrWhiteSpace(ApiKey)) + if (!string.Equals(BasemapId, "osm", StringComparison.OrdinalIgnoreCase) + && string.IsNullOrWhiteSpace(ApiKey)) { throw new InvalidOperationException( $"BitArcGisMapProvider: An ApiKey is required for the '{BasemapId}' basemap. " + "Only the 'osm' basemap works without an API 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs` around lines 37 - 42, The ApiKey guard in BitArcGisMapProvider uses a case-sensitive check ("BasemapId is not \"osm\"") so values like "OSM" wrongly require an API key; update the condition to perform a null-safe, case-insensitive comparison (e.g., use string.Equals or StringComparison.OrdinalIgnoreCase) against "osm" for BasemapId while keeping the same exception and message that references BasemapId and ApiKey.src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts (1)
270-314:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
GeometryCollectionis still silently dropped.
addGeoJsonnow handles the GeoJSON multi-types, but validGeometryCollectionfeatures still produce no graphics on this provider. That leaves the same payload rendering differently across backends.🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts` around lines 270 - 314, The loop building graphics silently skips GeoJSON GeometryCollection types; update the feature-processing block in the method that pushes into graphics (the code that checks t === 'Point'|'MultiPoint'|'LineString'|...) to detect t === 'GeometryCollection' and iterate f.geometry.geometries, treating each geometry element like a separate geometry: preserve props (layerId, bmKind), map each sub-geometry's type to the same creation logic that uses esri.Graphic, esri.Point/Polyline/Polygon and the helpers BitMapArcGis._lineSym and BitMapArcGis._fillSym, or refactor the geometry-to-graphic logic into a small helper and call it recursively for each geometry in the GeometryCollection so GeometryCollection features produce the same graphics as other providers.src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts (1)
247-257:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVector updates can still throw on duplicate Cesium entity IDs.
Each method calls
viewer.entities.add(...)before the previous entity for the samelayerIdis removed. Re-adding an existing vector layer can therefore fail instead of replacing it. Remove the current entity first, then add the new one.Also applies to: 260-275, 278-295, 297-312
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts` around lines 247 - 257, The addPolyline implementation (and the similar methods at the other ranges) can throw when re-adding an entity with the same Cesium id because you call viewer.entities.add(...) before removing an existing entity; change the flow in addPolyline (use s = BitMapCesium._require(id), compute the desired Cesium id string `bm-poly-${id}-${layerId}`, check for an existing entity via s.viewer.entities.getById(existingId) and if present call s.viewer.entities.remove(existingEntity) before calling s.viewer.entities.add(...), then continue to call BitMapCesium._setLayer(s, layerId, ent, 'polyline'); apply the same remove-then-add pattern to the other vector methods referenced (the blocks at ~260-275, 278-295, 297-312) so re-adding replaces instead of throwing.
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapTileOverlay.cs`:
- Around line 17-18: The Opacity property on BitMapTileOverlay currently accepts
any double but must be constrained to 0–1; replace the init-only auto-property
with a backing field (e.g. _opacity) and implement a getter and an init accessor
that clamps the incoming value with Math.Clamp(value, 0, 1) so Opacity always
remains within the documented range.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts`:
- Around line 64-69: The sync method in BitMapArcGis (BitMapArcGis.sync)
currently defaults to a hardcoded center when options.center is absent, which
can pan the map unintentionally; change it to reuse the map's current center
from s.view.center unless the caller supplies options.center. Concretely, in
BitMapArcGis.sync use the provided options.center when present (handling the
existing {lng, lat} shape) and otherwise obtain the center from s.view.center
(convert it to [lng, lat] if necessary) before calling s.view.goTo so non-center
option updates do not change the map position.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.ts`:
- Around line 281-287: In addGeoJson, parse and validate the incoming
geoJsonString before calling BitMapAzureMaps._removeExisting so we only remove
the existing layer when the new payload is known-good; specifically, move the
JSON.parse(geoJsonString) (and any validation of gj) above the call to
BitMapAzureMaps._removeExisting, ensure gj is validated (throwing the same
'Invalid GeoJSON string' error on failure), then call
BitMapAzureMaps._removeExisting(s, layerId) and continue with augment and
subsequent logic using the validated gj.
- Around line 54-58: The sync method in BitMapAzureMaps.sync resets to a
hardcoded default center when options.center is missing; instead retrieve the
current camera center from s.map.getCamera().center and use that (preserving
[lng, lat] order) unless options.center is explicitly provided. Update
BitMapAzureMaps.sync to compute center = options.center ? [options.center.lng,
options.center.lat] : s.map.getCamera().center, then call s.map.setCamera with
that center while leaving zoom and type logic intact so style/control syncs
don’t move the map unexpectedly.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts`:
- Around line 114-123: BitMapCesium.sync is resetting camera to defaults when
options omit center/zoom/altitude; change it to preserve the current camera
state unless those specific keys are provided: if options.center is undefined,
read the current camera Cartesian position from s.viewer.camera and convert to a
Cartographic (use s.Cesium.Cartographic.fromCartesian or
s.Cesium.Ellipsoid.WGS84.cartesianToCartographic) to derive lat/lng; if
options.altitude is undefined and zoom is undefined, use the current
cartographic.height for altitude; if zoom is provided still convert via
BitMapCesium._zoomToAltitude; finally build the destination from the computed
lat/lng/altitude and call s.viewer.camera.flyTo as before. Ensure you only
fallback to the existing defaults when there is no current camera position
available.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.ts`:
- Around line 303-307: In addGeoJson, avoid removing the existing vector before
validating the new payload: call JSON.parse(geoJsonString) first (handle parse
exceptions as now), and only after parsing succeeds obtain the provider via
BitMapGlBase._require(provider, id) and call BitMapGlBase._removeVector(s,
layerId) and proceed with swapping/adding the source/layers; this keeps the
existing layer intact on invalid GeoJSON. Reference the addGeoJson method and
helper methods BitMapGlBase._require and BitMapGlBase._removeVector when making
the change.
- Around line 89-92: The code recreates center with hardcoded defaults when
options.center is missing, causing unwanted pans; change the center logic in the
block that builds center/zoom before calling map.jumpTo — if options.center is
provided use that, otherwise get the current center from map.getCenter()
(convert to [lng, lat]) and keep existing zoom via map.getZoom(); then call
map.jumpTo({ center, zoom, essential: true }) so non-view-sync updates preserve
the current map center.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scss`:
- Line 7: The import in the stylesheet uses a .scss extension which violates the
scss/load-partial-extension rule; update the import statement that currently
reads `@import` "../Components/Map/BitMap.scss" to omit the extension (e.g.
`@import` "../Components/Map/BitMap") and ensure the target file is a Sass partial
(prefixed with an underscore like _BitMap.scss) so the linter and pipeline
accept it.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor.cs`:
- Around line 186-190: OnGeoJsonFeatureClick currently calls
e.Properties.TryGetProperty(...) directly which can throw if e.Properties is not
a JSON object (it may be JsonValueKind.Null); update the handler to first check
e.Properties.ValueKind == JsonValueKind.Object (or not JsonValueKind.Null)
before calling TryGetProperty, then only call n.GetString() if the found
property is a string (or fall back to a safe string conversion) and set
geoJsonLog accordingly; reference the OnGeoJsonFeatureClick method, the
e.Properties JsonElement checks, and the geoJsonLog assignment when making this
change.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs`:
- Around line 37-42: The ApiKey guard in BitArcGisMapProvider uses a
case-sensitive check ("BasemapId is not \"osm\"") so values like "OSM" wrongly
require an API key; update the condition to perform a null-safe,
case-insensitive comparison (e.g., use string.Equals or
StringComparison.OrdinalIgnoreCase) against "osm" for BasemapId while keeping
the same exception and message that references BasemapId and ApiKey.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts`:
- Around line 270-314: The loop building graphics silently skips GeoJSON
GeometryCollection types; update the feature-processing block in the method that
pushes into graphics (the code that checks t ===
'Point'|'MultiPoint'|'LineString'|...) to detect t === 'GeometryCollection' and
iterate f.geometry.geometries, treating each geometry element like a separate
geometry: preserve props (layerId, bmKind), map each sub-geometry's type to the
same creation logic that uses esri.Graphic, esri.Point/Polyline/Polygon and the
helpers BitMapArcGis._lineSym and BitMapArcGis._fillSym, or refactor the
geometry-to-graphic logic into a small helper and call it recursively for each
geometry in the GeometryCollection so GeometryCollection features produce the
same graphics as other providers.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts`:
- Around line 247-257: The addPolyline implementation (and the similar methods
at the other ranges) can throw when re-adding an entity with the same Cesium id
because you call viewer.entities.add(...) before removing an existing entity;
change the flow in addPolyline (use s = BitMapCesium._require(id), compute the
desired Cesium id string `bm-poly-${id}-${layerId}`, check for an existing
entity via s.viewer.entities.getById(existingId) and if present call
s.viewer.entities.remove(existingEntity) before calling
s.viewer.entities.add(...), then continue to call BitMapCesium._setLayer(s,
layerId, ent, 'polyline'); apply the same remove-then-add pattern to the other
vector methods referenced (the blocks at ~260-275, 278-295, 297-312) so
re-adding replaces instead of throwing.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0933595e-06fd-4731-9a8c-d996a62e1461
⛔ Files ignored due to path filters (5)
src/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-shadow.pngis excluded by!**/*.png
📒 Files selected for processing (35)
.gitignoresrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLng.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLngBounds.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarker.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarkerDragEndArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapProviderBase.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapTileOverlay.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapVectorPathStyle.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapViewState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/IBitMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitAzureMapsMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitCesiumMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitLeafletMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLibreMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapLibre.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapbox.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitOpenLayersMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/leaflet-1.9.4.jssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 40 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (2)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor:19
- User-facing text should use the proper product name "NuGet" (not "nuget").
src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.ts:345 - The common options include
boxZoom, but the OpenLayers provider never toggles the corresponding interaction (e.g.,DragZoom). As a result,BitMapProviderBase.BoxZoomhas no effect for OpenLayers even though it’s documented as supported. Please add handling here to enable/disable the box-zoom interaction based ono.boxZoom.
private static _applyInteractions(s: any, o: any) {
const map = s.map;
const interactions = map.getInteractions().getArray();
for (const interaction of interactions) {
const name = interaction.constructor?.name || '';
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.ts (1)
45-53:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrevent degenerate polygon rings by enforcing a minimum segment count.
Line 46 allows
pointsto be 1 or 2, which produces invalid/degenerate rings for circle polygons. Enforcepoints >= 3(and optionally short-circuit non-positive radius) before entering the loop.Suggested fix
- points = Math.max(1, Math.floor(points)); + points = Math.max(3, Math.floor(points)); + if (!Number.isFinite(radiusMeters) || radiusMeters <= 0) return [];🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.ts` around lines 45 - 53, In circleRingLngLat ensure the generated ring is not degenerate by enforcing points = Math.max(3, Math.floor(points)) (instead of Math.max(1,...)) and early-return an empty array (or a suitable degenerate value) when radiusMeters <= 0; update the function circleRingLngLat to apply the minimum segment count and handle non-positive radius before computing lat1/lng1/angular and entering the for loop so the produced polygon ring is valid.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapViewState.cs (1)
9-15: ⚡ Quick winMake non-null members
requiredto preserve null-safety contract.Line 9 and Line 15 are non-nullable but currently optional during initialization. Mark them
required(or provide defaults) to avoid accidental invalid DTO instances.Suggested fix
- public BitMapLatLng Center { get; init; } + public required BitMapLatLng Center { get; init; } @@ - public BitMapLatLngBounds Bounds { get; init; } + public required BitMapLatLngBounds Bounds { get; init; }🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapViewState.cs` around lines 9 - 15, The BitMapViewState DTO declares non-nullable properties (BitMapLatLng Center, double Zoom, BitMapLatLngBounds Bounds) but they are not enforced at initialization; update the BitMapViewState type so these members are required (e.g., mark Center and Bounds with the required modifier and ensure Zoom is required or given a safe default) in the class declaration to preserve null-safety and prevent invalid instances being created.
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapProviderBase.cs`:
- Around line 72-95: Add a guard at the start of GetCommonOptions to validate
zoom bounds and fail fast: if MinZoom is greater than MaxZoom, throw a clear
ArgumentException (or InvalidOperationException) with a descriptive message
indicating the invalid MinZoom/MaxZoom values; this check should live in
BitMapProviderBase's GetCommonOptions before building the options dictionary so
consumers get an immediate, explicit error rather than provider-specific
behavior.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapVectorPathStyle.cs`:
- Around line 11-13: The Weight property in BitMapVectorPathStyle currently
allows negative values; replace the auto-property with a backing field (e.g.,
_weight) and implement the setter for Weight to clamp incoming values to >= 0
(assign 0 when value is negative) while keeping the default of 3; update
references to use the Weight property as before.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts`:
- Around line 464-510: _store the Cesium.ScreenSpaceEventHandler and the moveEnd
callback timer on component state in _wireEvents (e.g., s._cesiumHandler and
s._viewTimer / s._moveEndCallback) instead of local vars, and in dispose() call
s._cesiumHandler.destroy() (if set), clearTimeout(s._viewTimer) and remove the
camera moveEnd listener by calling
viewer.camera.moveEnd.removeEventListener(s._moveEndCallback) (store the
callback when calling addEventListener in _wireEvents); also null out those
state slots after cleanup. This ensures the ScreenSpaceEventHandler, the
viewTimer, and the moveEnd callback are properly released when disposing
BitMapCesium.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.ts`:
- Around line 98-104: When swapping styles in BitMapGlBase (inside the block
that calls map.setStyle in the styleUrl change branch), unregister any
previously attached click handlers stored in s.vectorCatalog entries before you
clear s.vectorCatalog and s.tileOverlayCatalog; iterate s.vectorCatalog's
entries and call map.off('click', handler) (or map.off with the exact
event/handler signature used when registering) for each handler in each entry's
handlers array, then clear the catalogs as currently done in the
map.once('styledata') callback so you don't leak dangling listeners that cause
duplicate OnVectorClick/OnGeoJsonFeatureClick invocations when layers are
re-added.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.ts`:
- Around line 83-109: sync currently updates view, tiles and controls but
doesn't reapply interaction toggles, so runtime changes to scrollWheelZoom,
doubleClickZoom, boxZoom, dragging or keyboardNavigation are ignored; update
BitMapLeaflet.sync to read these boolean options (e.g. o.scrollWheelZoom,
o.doubleClickZoom, o.boxZoom, o.dragging, o.keyboardNavigation) and call the
corresponding Leaflet handler enable()/disable() on the map instance (use
s.map.scrollWheelZoom.enable()/disable(),
s.map.doubleClickZoom.enable()/disable(), s.map.boxZoom.enable()/disable(),
s.map.dragging.enable()/disable(), s.map.keyboard.enable()/disable()) only when
the option is explicitly provided (leave current state otherwise) so the runtime
changes take effect.
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.ts`:
- Around line 147-150: The BitMapOpenLayers provider is storing draggable on
ol.Feature but never wires up an OpenLayers drag interaction or emits the
OnMarkerDragEnd callback; fix by adding an ol.interaction.Translate (or
ol.interaction.Modify for points) to the map that is configured to only act on
features with feature.get('draggable') === true, register a 'translateend' (or
'modifyend') handler that reads the moved feature's markerId and new coordinates
(transform back from map projection to lon/lat with ol.proj.toLonLat) and
invokes the existing OnMarkerDragEnd emitter/callback for this provider, and
ensure this interaction is created when the map/features are initialized (e.g.,
where ol.Feature is created) and disposed/removed when the map is torn down;
also consider updating the feature style/cursor for draggable features so users
know they can drag.
- Around line 77-80: The sync logic always rebuilds center from defaults and
calls view.setCenter, causing the viewport to jump when options.center is
undefined; change the code in the sync routine (the block that declares const o
= options || {} and calls view.setCenter and view.setZoom) to only call
view.setCenter(ol.fromLonLat([lng, lat])) when options.center is explicitly
provided (i.e., o.center != null), and keep the existing conditional
view.setZoom(o.zoom) behavior; this prevents resetting the map to the hardcoded
51.505/-0.09 during non-view updates.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor`:
- Around line 162-165: The Provider is being reconstructed each render because
BuildAdvancedProvider() is called inline in the BitMap tag; cache the provider
in a readonly field (for example private readonly BitLeafletMapProvider
_advancedProvider or similar) and pass that field to the Provider parameter
instead of calling BuildAdvancedProvider() directly; create/assign the field
only when demo options intentionally change (mirroring the pattern used in the
other demos) so that the Provider's OnProviderSet callback and map view are not
reset on unrelated renders (refer to BuildAdvancedProvider(), advMapRef and the
Provider parameter to locate the change).
---
Duplicate comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.ts`:
- Around line 45-53: In circleRingLngLat ensure the generated ring is not
degenerate by enforcing points = Math.max(3, Math.floor(points)) (instead of
Math.max(1,...)) and early-return an empty array (or a suitable degenerate
value) when radiusMeters <= 0; update the function circleRingLngLat to apply the
minimum segment count and handle non-positive radius before computing
lat1/lng1/angular and entering the for loop so the produced polygon ring is
valid.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapViewState.cs`:
- Around line 9-15: The BitMapViewState DTO declares non-nullable properties
(BitMapLatLng Center, double Zoom, BitMapLatLngBounds Bounds) but they are not
enforced at initialization; update the BitMapViewState type so these members are
required (e.g., mark Center and Bounds with the required modifier and ensure
Zoom is required or given a safe default) in the class declaration to preserve
null-safety and prevent invalid instances being created.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c25074c4-4f17-4544-83f4-34f9f3f8341e
⛔ Files ignored due to path filters (5)
src/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-shadow.pngis excluded by!**/*.png
📒 Files selected for processing (35)
.gitignoresrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLng.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLngBounds.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarker.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarkerDragEndArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapProviderBase.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapTileOverlay.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapVectorPathStyle.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapViewState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/IBitMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitAzureMapsMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitCesiumMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitLeafletMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLibreMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapLibre.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapbox.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitOpenLayersMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/leaflet-1.9.4.jssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs
Member
Author
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
Comment on lines
+55
to
+57
| map.once('load', () => resolve()); | ||
| map.once('error', (e: any) => reject(e?.error ?? new Error('Map error'))); | ||
| setTimeout(() => reject(new Error('GL map load timeout (30s)')), 30000); |
Comment on lines
+539
to
+554
| private static async _loadOl(): Promise<any> { | ||
| if (BitMapOpenLayers._olLoadPromise) return BitMapOpenLayers._olLoadPromise; | ||
| BitMapOpenLayers._olLoadPromise = (async () => { | ||
| // Import the full OL bundle from a single entry point so all classes | ||
| // (Map, View, etc.) share the same module scope and instanceof checks work. | ||
| const dynImport: (url: string) => Promise<any> = new Function('u', 'return import(u);') as any; | ||
| const ol = await dynImport(`https://esm.sh/ol@${BitMapOpenLayers._OL_VER}?bundle`); | ||
| const olControl = await dynImport(`https://esm.sh/ol@${BitMapOpenLayers._OL_VER}/control?bundle`); | ||
| const olStyle = await dynImport(`https://esm.sh/ol@${BitMapOpenLayers._OL_VER}/style?bundle`); | ||
| const olGeom = await dynImport(`https://esm.sh/ol@${BitMapOpenLayers._OL_VER}/geom?bundle`); | ||
| const olSource = await dynImport(`https://esm.sh/ol@${BitMapOpenLayers._OL_VER}/source?bundle`); | ||
| const olLayer = await dynImport(`https://esm.sh/ol@${BitMapOpenLayers._OL_VER}/layer?bundle`); | ||
| const olFormat = await dynImport(`https://esm.sh/ol@${BitMapOpenLayers._OL_VER}/format?bundle`); | ||
| const olProj = await dynImport(`https://esm.sh/ol@${BitMapOpenLayers._OL_VER}/proj?bundle`); | ||
| // ol/interaction is loaded implicitly via the main bundle (default interactions). | ||
| const olInteraction = await dynImport(`https://esm.sh/ol@${BitMapOpenLayers._OL_VER}/interaction?bundle`); |
Comment on lines
+184
to
+186
| // Verify initial init was called | ||
| Assert.AreEqual(1, Context.JSInterop.Invocations.Count(i => i.Identifier == INIT)); | ||
| } |
Comment on lines
+513
to
+517
| <BitMap TMapProvider=""BitLeafletMapProvider"" | ||
| @ref=""advMapRef"" | ||
| Provider=""@BuildAdvancedProvider()"" | ||
| OnReady=""OnAdvancedReady"" | ||
| OnDoubleClick=""OnAdvancedDoubleClick"" />"; |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs (1)
37-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake OSM validation case-insensitive.
Line 37 currently treats
"OSM"(or mixed-case) as non-OSM and throws whenApiKeyis empty. Use an ordinal-ignore-case check so OSM remains keyless regardless of casing.Suggested fix
- if (BasemapId is not "osm" && string.IsNullOrWhiteSpace(ApiKey)) + if (!string.Equals(BasemapId, "osm", StringComparison.OrdinalIgnoreCase) + && string.IsNullOrWhiteSpace(ApiKey)) { throw new InvalidOperationException( $"BitArcGisMapProvider: An ApiKey is required for the '{BasemapId}' basemap. " + "Only the 'osm' basemap works without an API 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs` around lines 37 - 41, The current check in BitArcGisMapProvider that throws when BasemapId is not "osm" should be changed to a case-insensitive comparison so values like "OSM" or "OsM" are treated as OSM; update the if condition that references BasemapId and ApiKey to use an ordinal-ignore-case comparison (e.g., string.Equals(BasemapId, "osm", StringComparison.OrdinalIgnoreCase) or equivalent) so no ApiKey is required for any casing of "osm".src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts (1)
277-287:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the previous vector entity before
entities.add(...).These methods create a new entity with a deterministic id (
bm-poly-*,bm-polygon-*,bm-circle-*,bm-rect-*) and only remove the old layer inside_setLayerafterwards. Replacing an existinglayerIdwill therefore hit Cesium's duplicate-id path before cleanup runs.Suggested direction
public static addPolyline(id: string, layerId: string, latlngs: BitMapLL[], style: any) { const s = BitMapCesium._require(id); const Cesium = s.Cesium; const st = BitMapHelpers.readPathStyle(style); + const existing = s.layers[layerId]; + if (existing) try { s.viewer.entities.remove(existing.entity); } catch { /* ignore */ } const positions = Cesium.Cartesian3.fromDegreesArray(latlngs.flatMap(p => [p.lng, p.lat])); const ent = s.viewer.entities.add({ id: `bm-poly-${id}-${layerId}`, polyline: { positions, width: st.weight, material: BitMapCesium._color(Cesium, st.color, st.opacity) }, _bmLayerId: layerId, _bmVectorKind: 'polyline', }); - BitMapCesium._setLayer(s, layerId, ent, 'polyline'); + s.layers[layerId] = { entity: ent, kind: 'polyline' }; }Apply the same ordering to the other vector helpers.
Also applies to: 290-305, 308-324, 327-341
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts` around lines 277 - 287, The addPolyline implementation (and the similar vector helpers addPolygon, addCircle, addRectangle) currently calls s.viewer.entities.add(...) with a deterministic id like `bm-poly-${id}-${layerId}` before `_setLayer` removes the previous entity, causing Cesium to detect duplicate ids; fix each helper (e.g., BitMapCesium.addPolyline) to first remove any existing entity with that deterministic id (use s.viewer.entities.removeById or the equivalent on s.viewer.entities) for the same `id`/`layerId`, then call s.viewer.entities.add(...), and finally call BitMapCesium._setLayer(s, layerId, ent, 'polyline') as before.src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts (2)
73-80:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't reset omitted sync options to defaults.
syncalways calls_ensureScaleBarwith!!o.showScaleControland treats missingscrollWheelZoom/draggingas enabled, so any partial update can hide an existing scale bar and re-enable interactions the caller never changed. Only touch these settings when the option is explicitly present, or preserve the current state.Suggested change
- BitMapArcGis._ensureScaleBar(s, !!o.showScaleControl); + if ('showScaleControl' in o) { + BitMapArcGis._ensureScaleBar(s, !!o.showScaleControl); + } // Reapply interaction flags the same way init sets them via navigation.actionMap. const actionMap = s.view.navigation?.actionMap; if (actionMap) { - actionMap.mouseWheel = o.scrollWheelZoom !== false ? 'zoom' : null; - actionMap.dragPrimary = o.dragging !== false ? 'pan' : null; + if ('scrollWheelZoom' in o) { + actionMap.mouseWheel = o.scrollWheelZoom !== false ? 'zoom' : null; + } + if ('dragging' in o) { + actionMap.dragPrimary = o.dragging !== false ? 'pan' : null; + } }🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts` around lines 73 - 80, Only update scale control and interaction flags when the caller explicitly provided those options: call BitMapArcGis._ensureScaleBar(s, !!o.showScaleControl) only if o has the showScaleControl property, and set actionMap.mouseWheel and actionMap.dragPrimary only when o has scrollWheelZoom and dragging respectively (use Object.prototype.hasOwnProperty.call(o, '...') or equivalent checks); this preserves existing state when options are omitted and avoids resetting defaults in BitMapArcGis._ensureScaleBar, actionMap.mouseWheel, and actionMap.dragPrimary.
272-317:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
GeometryCollectioninstead of dropping it.
addGeoJsonnow covers the multi-geometry types, but a validGeometryCollectionstill falls through this branch chain and disappears silently. Recursing throughgeometry.geometrieshere would keep ArcGIS behavior aligned with the rest of the GeoJSON surface.🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts` around lines 272 - 317, The loop in addGeoJson drops features whose geometry.type === 'GeometryCollection'; update the feature-processing logic in BitMapArcGis (the block handling features and using BitMapArcGis._lineSym/_fillSym) to handle 'GeometryCollection' by iterating f.geometry.geometries and processing each geometry with the same props (bmKind/layerId) instead of skipping; implement this by either adding an else-if branch for 'GeometryCollection' that loops over geometries and reuses the existing creation logic (or delegates to a small helper like processFeatureGeometry(featureProps, geometry)) so points, multi-points, lines, polygons and multiparts are created as esri.Graphic entries just like other types.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs (1)
34-49: ⚡ Quick winAdd validation for null/empty StyleUrl.
The past review comment suggested validating both
StyleUrlandAccessToken. While theAccessTokenvalidation formapbox://styles has been added (lines 36-41), theStyleUrlvalidation is still missing. IfStyleUrlis null, line 36 will throw aNullReferenceExceptionwith an unclear error message. If it's an empty string, the invalid value will be passed to JavaScript unchecked.🛡️ Suggested validation to add
public override object BuildOptionsPayload() { + if (string.IsNullOrWhiteSpace(StyleUrl)) + throw new InvalidOperationException("BitMapboxMapProvider: StyleUrl is required."); + if (StyleUrl.StartsWith("mapbox://", StringComparison.OrdinalIgnoreCase) && string.IsNullOrWhiteSpace(AccessToken)) { throw new InvalidOperationException(🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs` around lines 34 - 49, BuildOptionsPayload currently calls StyleUrl.StartsWith which throws if StyleUrl is null and allows empty strings through; add validation at the top of BuildOptionsPayload to ensure StyleUrl is neither null nor whitespace (throw InvalidOperationException with a clear message referencing StyleUrl), then keep the existing Mapbox accessToken check; ensure after validation you still populate the common dictionary (GetCommonOptions, then set "accessToken","styleUrl","showNavigationControl","dragRotate") so invalid values are never passed to JavaScript.
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarker.cs`:
- Around line 63-67: The IconWidth and IconHeight properties on BitMapMarker may
be set to negative values; update their init accessors to prevent negatives by
clamping incoming values to >= 0 (e.g., set to Math.Max(0, value)) before
assignment, or alternatively throw an ArgumentOutOfRangeException if a negative
is provided; modify the IconWidth and IconHeight init logic in BitMapMarker to
enforce this validation so invalid icon dimensions are never emitted to JS
providers.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.ts`:
- Around line 309-331: addGeoJson currently only creates 'fill' and 'line'
layers so Point/MultiPoint features are not rendered; add a circle layer and its
handlers: create a circleId (e.g. `bm-circle-${id}-${layerId}`), call
s.map.addLayer({ id: circleId, type: 'circle', source: sourceId, paint:
BitMapGlBase._circlePaint(style) }) (or create a new paint helper if none
exists), register the same click handler with s.map.on('click', circleId,
handler), push { layerId: circleId, handler } into handlers, and include
circleId in s.vectorCatalog[layerId].layerIds so the circle layer is
tracked/removed alongside the fill and line layers in BitMapGlBase.addGeoJson.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.ts`:
- Around line 116-117: The calls to BitMapLeaflet._applyMaxBounds(s,
o.maxBounds) and BitMapLeaflet._ensureScaleControl(s, !!o.showScaleControl,
!!o.scaleControlImperial) are unconditional and can clear existing settings on
partial syncs; change the logic in the sync/update code so you only call
BitMapLeaflet._applyMaxBounds when o.maxBounds is explicitly provided (e.g., not
undefined/null) and only call BitMapLeaflet._ensureScaleControl when
o.showScaleControl or o.scaleControlImperial are present (or otherwise detect
and preserve the current scale-control state), so existing maxBounds and scale
control are not removed on partial updates.
- Around line 98-103: The sync routine currently always removes and recreates
s.baseTileLayer using o.tileUrl/o.tileMaxZoom/o.tileAttribution/o.tileOpacity
even when those options haven't changed; update sync to detect whether tile
options actually changed before recreating the layer by comparing the incoming
o.{tileUrl,tileMaxZoom,tileAttribution,tileOpacity} to a stored previous options
object (e.g., s._tileOptions) and only call s.map.removeLayer(s.baseTileLayer)
and create a new L.tileLayer when any value differs; if nothing changed, leave
s.baseTileLayer in place (but still call s.baseTileLayer.bringToBack() if
needed) and update s._tileOptions when you do recreate the layer.
In `@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/Map/BitMapTests.cs`:
- Around line 165-186: The test currently only renders a Leaflet provider and
asserts the initial INIT call; to exercise the re-initialization path you must
change the component's Provider to the OpenLayers provider and assert that the
old map is disposed and the new provider's init is invoked. Specifically, in
BitMapShouldReInitializeWhenJsObjectNameChanges, after the initial
RenderComponent<BitMap<BitLeafletMapProvider>> and Assert for INIT, call
component.SetParametersAndRender or re-render the component with
parameters.Add(p => p.Provider, new BitOpenLayersMapProvider()) (or the
appropriate OpenLayers provider type), then assert that DISPOSE (the old
provider dispose) and OL_DISPOSE/OL_INIT or OL_INIT (depending on expected
sequence) were invoked via Context.JSInterop.Invocations (e.g., count
i.Identifier == DISPOSE and i.Identifier == OL_INIT); this will exercise
BitMap.OnProviderSet() re-initialization logic.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cs`:
- Around line 37-41: The current check in BitArcGisMapProvider that throws when
BasemapId is not "osm" should be changed to a case-insensitive comparison so
values like "OSM" or "OsM" are treated as OSM; update the if condition that
references BasemapId and ApiKey to use an ordinal-ignore-case comparison (e.g.,
string.Equals(BasemapId, "osm", StringComparison.OrdinalIgnoreCase) or
equivalent) so no ApiKey is required for any casing of "osm".
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.ts`:
- Around line 73-80: Only update scale control and interaction flags when the
caller explicitly provided those options: call BitMapArcGis._ensureScaleBar(s,
!!o.showScaleControl) only if o has the showScaleControl property, and set
actionMap.mouseWheel and actionMap.dragPrimary only when o has scrollWheelZoom
and dragging respectively (use Object.prototype.hasOwnProperty.call(o, '...') or
equivalent checks); this preserves existing state when options are omitted and
avoids resetting defaults in BitMapArcGis._ensureScaleBar, actionMap.mouseWheel,
and actionMap.dragPrimary.
- Around line 272-317: The loop in addGeoJson drops features whose geometry.type
=== 'GeometryCollection'; update the feature-processing logic in BitMapArcGis
(the block handling features and using BitMapArcGis._lineSym/_fillSym) to handle
'GeometryCollection' by iterating f.geometry.geometries and processing each
geometry with the same props (bmKind/layerId) instead of skipping; implement
this by either adding an else-if branch for 'GeometryCollection' that loops over
geometries and reuses the existing creation logic (or delegates to a small
helper like processFeatureGeometry(featureProps, geometry)) so points,
multi-points, lines, polygons and multiparts are created as esri.Graphic entries
just like other types.
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.ts`:
- Around line 277-287: The addPolyline implementation (and the similar vector
helpers addPolygon, addCircle, addRectangle) currently calls
s.viewer.entities.add(...) with a deterministic id like
`bm-poly-${id}-${layerId}` before `_setLayer` removes the previous entity,
causing Cesium to detect duplicate ids; fix each helper (e.g.,
BitMapCesium.addPolyline) to first remove any existing entity with that
deterministic id (use s.viewer.entities.removeById or the equivalent on
s.viewer.entities) for the same `id`/`layerId`, then call
s.viewer.entities.add(...), and finally call BitMapCesium._setLayer(s, layerId,
ent, 'polyline') as before.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cs`:
- Around line 34-49: BuildOptionsPayload currently calls StyleUrl.StartsWith
which throws if StyleUrl is null and allows empty strings through; add
validation at the top of BuildOptionsPayload to ensure StyleUrl is neither null
nor whitespace (throw InvalidOperationException with a clear message referencing
StyleUrl), then keep the existing Mapbox accessToken check; ensure after
validation you still populate the common dictionary (GetCommonOptions, then set
"accessToken","styleUrl","showNavigationControl","dragRotate") so invalid values
are never passed to JavaScript.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a07b33db-ec5c-42ab-87ee-b20cc2d75a7c
⛔ Files ignored due to path filters (5)
src/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/layers.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon-2x.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-icon.pngis excluded by!**/*.pngsrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/images/marker-shadow.pngis excluded by!**/*.png
📒 Files selected for processing (36)
.gitignoresrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMap.scsssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLng.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapLatLngBounds.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarker.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarkerDragEndArgs.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapProviderBase.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapTileOverlay.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapVectorPathStyle.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapViewState.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/IBitMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitArcGisMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitAzureMapsMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitCesiumMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitLeafletMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapArcGis.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapCesium.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLibreMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapLibre.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapMapbox.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapShared.tssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapboxMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitOpenLayersMapProvider.cssrc/BlazorUI/Bit.BlazorUI.Extras/Styles/extra-components.scsssrc/BlazorUI/Bit.BlazorUI.Extras/wwwroot/leaflet/leaflet-1.9.4.jssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/Map/BitMapDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/Map/BitMapTests.cs
Comment on lines
+63
to
+67
| /// <summary>Width in pixels of the custom marker icon.</summary> | ||
| public int? IconWidth { get; init; } | ||
|
|
||
| /// <summary>Height in pixels of the custom marker icon.</summary> | ||
| public int? IconHeight { get; init; } |
There was a problem hiding this comment.
Prevent negative icon dimensions.
Line 64 and Line 67 allow negative sizes, which can generate invalid marker icon options in JS providers. Clamp to >= 0 (or reject with an exception) in init accessors.
Suggested fix
- public int? IconWidth { get; init; }
+ public int? IconWidth
+ {
+ get => _iconWidth;
+ init => _iconWidth = value is null ? null : Math.Max(0, value.Value);
+ }
- public int? IconHeight { get; init; }
+ public int? IconHeight
+ {
+ get => _iconHeight;
+ init => _iconHeight = value is null ? null : Math.Max(0, value.Value);
+ }
+
+ private int? _iconWidth;
+ private int? _iconHeight;📝 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.
Suggested change
| /// <summary>Width in pixels of the custom marker icon.</summary> | |
| public int? IconWidth { get; init; } | |
| /// <summary>Height in pixels of the custom marker icon.</summary> | |
| public int? IconHeight { get; init; } | |
| /// <summary>Width in pixels of the custom marker icon.</summary> | |
| public int? IconWidth | |
| { | |
| get => _iconWidth; | |
| init => _iconWidth = value is null ? null : Math.Max(0, value.Value); | |
| } | |
| /// <summary>Height in pixels of the custom marker icon.</summary> | |
| public int? IconHeight | |
| { | |
| get => _iconHeight; | |
| init => _iconHeight = value is null ? null : Math.Max(0, value.Value); | |
| } | |
| private int? _iconWidth; | |
| private int? _iconHeight; |
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/BitMapMarker.cs` around lines
63 - 67, The IconWidth and IconHeight properties on BitMapMarker may be set to
negative values; update their init accessors to prevent negatives by clamping
incoming values to >= 0 (e.g., set to Math.Max(0, value)) before assignment, or
alternatively throw an ArgumentOutOfRangeException if a negative is provided;
modify the IconWidth and IconHeight init logic in BitMapMarker to enforce this
validation so invalid icon dimensions are never emitted to JS providers.
Comment on lines
+309
to
+331
| public static addGeoJson(provider: string, id: string, layerId: string, geoJsonString: string, style: any) { | ||
| let gj: any; | ||
| try { gj = JSON.parse(geoJsonString); } catch { throw new Error('Invalid GeoJSON string'); } | ||
| const s = BitMapGlBase._require(provider, id); | ||
| BitMapGlBase._removeVector(s, layerId); | ||
| const sourceId = `bm-src-${id}-${layerId}`; | ||
| const fillId = `bm-fill-${id}-${layerId}`; | ||
| const lineId = `bm-line-${id}-${layerId}`; | ||
| s.map.addSource(sourceId, { type: 'geojson', data: gj }); | ||
| s.map.addLayer({ id: fillId, type: 'fill', source: sourceId, paint: BitMapGlBase._fillPaint(style) }); | ||
| s.map.addLayer({ id: lineId, type: 'line', source: sourceId, paint: BitMapGlBase._linePaint(style) }); | ||
| const dn = s.dotnetObj; | ||
| const handlers: { layerId: string, handler: any }[] = []; | ||
| if (dn) { | ||
| const handler = (e: any) => { | ||
| if (e.features?.[0]) dn.invokeMethodAsync('OnGeoJsonFeatureClick', layerId, e.features[0].properties || {}); | ||
| }; | ||
| s.map.on('click', fillId, handler); | ||
| s.map.on('click', lineId, handler); | ||
| handlers.push({ layerId: fillId, handler }); | ||
| handlers.push({ layerId: lineId, handler }); | ||
| } | ||
| s.vectorCatalog[layerId] = { sourceId, layerIds: [fillId, lineId], handlers }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the BitMapGlBase.ts file
find . -name "BitMapGlBase.ts" -type fRepository: bitfoundation/bitplatform
Length of output: 145
🏁 Script executed:
# Let's read the addGeoJson method and surrounding context
sed -n '300,340p' ./src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.ts | cat -nRepository: bitfoundation/bitplatform
Length of output: 2668
🏁 Script executed:
# Let's find Azure and OpenLayers provider implementations
find . -name "*Azure*" -o -name "*OpenLayers*" | grep -i "map\|provider" | head -20Repository: bitfoundation/bitplatform
Length of output: 403
🏁 Script executed:
# Let's search for addGeoJson in Azure and OpenLayers implementations
echo "=== BitMapAzureMaps.ts ===" && grep -n "addGeoJson" ./src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.ts
echo "=== BitMapOpenLayers.ts ===" && grep -n "addGeoJson" ./src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.tsRepository: bitfoundation/bitplatform
Length of output: 330
🏁 Script executed:
# Let's look at Azure's addGeoJson implementation (get a range around line 288)
echo "=== Azure Maps addGeoJson ===" && sed -n '288,330p' ./src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapAzureMaps.ts | cat -n
echo ""
echo "=== OpenLayers addGeoJson ===" && sed -n '295,350p' ./src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapOpenLayers.ts | cat -nRepository: bitfoundation/bitplatform
Length of output: 5895
Add point-layer support for GeoJSON on MapLibre/Mapbox backends.
The addGeoJson method only creates fill and line layers, so Point and MultiPoint features never render on MapLibre/Mapbox. This is inconsistent with Azure Maps (which includes a bubbleLayer for points) and OpenLayers (which uses geometry-aware styling). Point features are silently dropped instead of being rendered. Add a circle layer with appropriate styling and register its click handler in handlers and layerIds as well.
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapGlBase.ts`
around lines 309 - 331, addGeoJson currently only creates 'fill' and 'line'
layers so Point/MultiPoint features are not rendered; add a circle layer and its
handlers: create a circleId (e.g. `bm-circle-${id}-${layerId}`), call
s.map.addLayer({ id: circleId, type: 'circle', source: sourceId, paint:
BitMapGlBase._circlePaint(style) }) (or create a new paint helper if none
exists), register the same click handler with s.map.on('click', circleId,
handler), push { layerId: circleId, handler } into handlers, and include
circleId in s.vectorCatalog[layerId].layerIds so the circle layer is
tracked/removed alongside the fill and line layers in BitMapGlBase.addGeoJson.
Comment on lines
+98
to
+103
| if (s.baseTileLayer) s.map.removeLayer(s.baseTileLayer); | ||
| s.baseTileLayer = L.tileLayer( | ||
| o.tileUrl || "https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png", | ||
| { maxZoom: o.tileMaxZoom ?? 19, attribution: o.tileAttribution || "", opacity: o.tileOpacity ?? 1 } | ||
| ).addTo(s.map); | ||
| s.baseTileLayer.bringToBack(); |
There was a problem hiding this comment.
Preserve the current base tiles unless tile options changed.
This block runs on every sync and falls back to OSM/default tile settings when tileUrl, tileMaxZoom, tileAttribution, or tileOpacity are omitted. A partial update can therefore replace a custom basemap and trigger a full tile reload even though the caller never changed tile configuration.
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.ts`
around lines 98 - 103, The sync routine currently always removes and recreates
s.baseTileLayer using o.tileUrl/o.tileMaxZoom/o.tileAttribution/o.tileOpacity
even when those options haven't changed; update sync to detect whether tile
options actually changed before recreating the layer by comparing the incoming
o.{tileUrl,tileMaxZoom,tileAttribution,tileOpacity} to a stored previous options
object (e.g., s._tileOptions) and only call s.map.removeLayer(s.baseTileLayer)
and create a new L.tileLayer when any value differs; if nothing changed, leave
s.baseTileLayer in place (but still call s.baseTileLayer.bringToBack() if
needed) and update s._tileOptions when you do recreate the layer.
Comment on lines
+116
to
+117
| BitMapLeaflet._applyMaxBounds(s, o.maxBounds); | ||
| BitMapLeaflet._ensureScaleControl(s, !!o.showScaleControl, !!o.scaleControlImperial); |
There was a problem hiding this comment.
Don't clear max-bounds and scale control on partial syncs.
_applyMaxBounds(s, o.maxBounds) and _ensureScaleControl(s, !!o.showScaleControl, ...) run unconditionally, so omitting those options clears an existing bounds restriction and removes the scale control. Gate these calls on the option being present, or preserve the current state.
🤖 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 `@src/BlazorUI/Bit.BlazorUI.Extras/Components/Map/Providers/BitMapLeaflet.ts`
around lines 116 - 117, The calls to BitMapLeaflet._applyMaxBounds(s,
o.maxBounds) and BitMapLeaflet._ensureScaleControl(s, !!o.showScaleControl,
!!o.scaleControlImperial) are unconditional and can clear existing settings on
partial syncs; change the logic in the sync/update code so you only call
BitMapLeaflet._applyMaxBounds when o.maxBounds is explicitly provided (e.g., not
undefined/null) and only call BitMapLeaflet._ensureScaleControl when
o.showScaleControl or o.scaleControlImperial are present (or otherwise detect
and preserve the current scale-control state), so existing maxBounds and scale
control are not removed on partial updates.
Comment on lines
+165
to
+186
| public void BitMapShouldReInitializeWhenJsObjectNameChanges() | ||
| { | ||
| // Use OpenLayers provider which has a different JsObjectName than Leaflet | ||
| const string OL_INIT = "BitBlazorUI.BitMapOpenLayers.init"; | ||
| const string OL_DISPOSE = "BitBlazorUI.BitMapOpenLayers.dispose"; | ||
|
|
||
| Context.JSInterop.SetupVoid(INIT_STYLESHEETS); | ||
| Context.JSInterop.SetupVoid(INIT_SCRIPTS); | ||
| Context.JSInterop.SetupVoid(INIT); | ||
| Context.JSInterop.SetupVoid(DISPOSE); | ||
| Context.JSInterop.SetupVoid(OL_INIT); | ||
| Context.JSInterop.SetupVoid(OL_DISPOSE); | ||
|
|
||
| // Start with Leaflet | ||
| var component = RenderComponent<BitMap<BitLeafletMapProvider>>(parameters => | ||
| { | ||
| parameters.Add(p => p.Provider, new BitLeafletMapProvider()); | ||
| }); | ||
|
|
||
| // Verify initial init was called | ||
| Assert.AreEqual(1, Context.JSInterop.Invocations.Count(i => i.Identifier == INIT)); | ||
| } |
There was a problem hiding this comment.
Make this test exercise the re-initialization path.
This never changes the provider or JsObjectName; it only proves the initial Leaflet init call. As written, the provider-swap branch in OnProviderSet() can break without this test failing.
🤖 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 `@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/Map/BitMapTests.cs`
around lines 165 - 186, The test currently only renders a Leaflet provider and
asserts the initial INIT call; to exercise the re-initialization path you must
change the component's Provider to the OpenLayers provider and assert that the
old map is disposed and the new provider's init is invoked. Specifically, in
BitMapShouldReInitializeWhenJsObjectNameChanges, after the initial
RenderComponent<BitMap<BitLeafletMapProvider>> and Assert for INIT, call
component.SetParametersAndRender or re-render the component with
parameters.Add(p => p.Provider, new BitOpenLayersMapProvider()) (or the
appropriate OpenLayers provider type), then assert that DISPOSE (the old
provider dispose) and OL_DISPOSE/OL_INIT or OL_INIT (depending on expected
sequence) were invoked via Context.JSInterop.Invocations (e.g., count
i.Identifier == DISPOSE and i.Identifier == OL_INIT); this will exercise
BitMap.OnProviderSet() re-initialization logic.
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
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.
closes #10110
Summary by CodeRabbit
New Features
Documentation / Demos
Style
Tests
Chores