Skip to content

FeatureTable support and serialization bug fixes#512

Open
TimPurdum wants to merge 2 commits intodevelopfrom
feature-table-widget
Open

FeatureTable support and serialization bug fixes#512
TimPurdum wants to merge 2 commits intodevelopfrom
feature-table-widget

Conversation

@TimPurdum
Copy link
Collaborator

Summary

  • FeatureTableWidget layer support: Renamed IFeatureTableWidgetLayers to IFeatureTableWidgetLayer (singular) and added FeatureTableLayerType property to the interface. Implemented the interface on CSVLayer, FeatureLayer, GeoJSONLayer, ImageryLayer, WFSLayer, and Sublayer, enabling these layer types to work with the Pro FeatureTableWidget.
  • Widget parameter comparison fix: Fixed a bug in Widget.razor.cs where parameter change detection (SetParametersAsync) did not correctly compare arrays or generic collections. Previously, re-renders could be triggered incorrectly (or missed) for collection-typed parameters. The fix adds proper element-wise comparison for arrays and ICollection types, and also guards UpdateWidget calls behind MapRendered checks.
  • SizeVariable serialization fix: Fixed SizeVariableConverter to exclude Parent, View, and Layer navigation properties during JSON serialization, preventing circular reference errors.
  • IDomUiEvent deserialization: Added a DomUiEventConverter JSON converter for the IDomUiEvent interface, enabling polymorphic deserialization into DomPointerEvent, DomMouseEvent, or DomKeyboardEvent based on the properties present in the JSON payload.
  • CSS cleanup: Reformatted site.css with consistent indentation, added :not(.esri-feature-table) exclusion to .esri-widget color reset to avoid style conflicts with the FeatureTable widget, and added Calcite design token variables for text and foreground colors.

Test plan

  • Verify FeatureTableWidget renders correctly with FeatureLayer, CSVLayer, GeoJSONLayer, ImageryLayer, WFSLayer, and Sublayer
  • Confirm widget parameter changes (especially collection-typed params) trigger re-renders correctly
  • Validate SizeVariable serialization no longer throws circular reference errors
  • Test DomPointerEvent / DomMouseEvent / DomKeyboardEvent deserialization through IDomUiEvent

🤖 Generated with Claude Code

@claude
Copy link

claude bot commented Feb 24, 2026

Claude finished @TimPurdum's task —— View job


PR Review Analysis

Todo List:

  • Read key changed files to understand implementation
  • Review widget parameter comparison logic for correctness
  • Check serialization fixes for completeness
  • Verify interface changes for consistency
  • Provide specific feedback on any issues found

Issues Found:

1. Null Reference Exception in Widget Parameter Comparison

File: src/dymaptic.GeoBlazor.Core/Components/Widgets/Widget.razor.cs:430-436

The code assumes previousValue is not null when getting its type, but this could fail:

Type paramType = previousValue!.GetType(); // Line 430

if (paramType.IsArray)
{
    Array prevArray = (Array)previousValue;
    Array currArray = (Array)kvp.Value!; // Potential null reference

If kvp.Value is null but previousValue was an array, this will throw a NullReferenceException on the cast.

Fix: Add null checks for kvp.Value before casting in both array and collection comparison blocks.

2. Incomplete Loop Control in Parameter Comparison

File: src/dymaptic.GeoBlazor.Core/Components/Widgets/Widget.razor.cs:447-491

The nested loops for array and collection comparison have break statements that only exit the inner loop, not the outer parameter iteration loop. This means if an array/collection element differs, it breaks the element comparison but continues checking other parameters instead of triggering an update.

Expected behavior: When any difference is found, should break the outer loop and trigger widget update.
Current behavior: Breaks inner loop, continues outer loop, may miss triggering updates.

3. Exception Handling in DomUiEventConverter

File: src/dymaptic.GeoBlazor.Core/Events/DomPointerEvent.cs:183-244

The converter creates a clone of the JsonReader but doesn't properly handle exceptions during deserialization. If any of the JsonSerializer.Deserialize calls fail, the exception bubbles up without context about which event type was being attempted.

Recommendation: Add try-catch blocks around the deserialization attempts with more specific error handling.

4. Type Inconsistency in Sublayer Implementation

File: src/dymaptic.GeoBlazor.Core/Components/Layers/Sublayer.cs:6

All other layer types return Type.ToString() for FeatureTableLayerType, but Sublayer returns nameof(Sublayer). This inconsistency could cause issues if the feature table widget expects consistent type naming.

Fix: Should return Type.ToString() like other layers, or document why this is intentionally different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant