rungwiroon / BlazorGoogleMaps

Blazor interop for GoogleMap library
MIT License
319 stars 102 forks source link

Incorrect Serialization of Enums #293

Closed cruiserkernan closed 4 weeks ago

cruiserkernan commented 9 months ago

Description

The BlazorGoogleMaps library does not always use the EnumMember attribute when serializing enums to JSON. This causes map controls to not be correctly positioned on the map. For instance, the enum TOP_LEFT gets serialized as topLeft, leading to incorrect behavior.

Additional Details

Expected Behavior

Current Behavior

Steps to Reproduce

  1. Define a map control with a specific position using enum, like TOP_LEFT.
  2. Observe that the control is not positioned as TOP_LEFT but as BOTTOM_CENTER.

Suggested Fix

Additional Information

This serialization issue significantly impacts the functionality and usability of the BlazorGoogleMaps library, as controls and other enum-dependent features do not behave as expected.

Example of Incorrect Code

// Incorrect serialization of Enums in various classes
[JsonConverter(typeof(JsonStringEnumConverter))]

Potential Impact

Contribution and Proposed Changes

I am willing to contribute to the resolution of this issue and will submit a pull request with the necessary changes. The proposed changes include updating the JsonConverter attribute to respect the EnumMember attribute in enum serialization and correctly implementing the JsonStringEnumConverterEx in all the enums and add the converter in Helper.cs.

Thank you for your attention to this matter.

valentasm1 commented 9 months ago

_control is not positioned as TOP_LEFT but as BOTTOMCENTER. this is different bug. Not related to serialization. Solved with https://github.com/rungwiroon/BlazorGoogleMaps/issues/281

cruiserkernan commented 9 months ago

I believe it is another bug. Upon running the Blazor Server Demo and examining the MapLegendPage, I observed that its intended position, TOP_LEFT, is not reflected in the actual placement; it appears at BOTTOM_CENTER instead. During debugging, I noticed that the addControl method in the objectManager is receiving "topLeft" as the enum value. However, this doesn't seem to match any case in the switch statement, causing it to revert to the default case of BOTTOM_CENTER.

I initially noticed this in my current application when trying to position the control on RIGHT_CENTER.

using System.Threading.Tasks;
using GoogleMapsComponents;
using GoogleMapsComponents.Maps;
using Microsoft.AspNetCore.Components;
using Microsoft.JSInterop;

namespace ServerSideDemo.Pages;

public partial class MapLegendPage
{
    private GoogleMap map1;

    private MapOptions mapOptions;

    [Inject] private IJSRuntime jsRuntime { get; set; }

    protected ElementReference legendReference { get; set; }

    protected override void OnInitialized()
    {
        mapOptions = new MapOptions()
        {
            Zoom = 13,
            Center = new LatLngLiteral()
            {
                Lat = 13.505892,
                Lng = 100.8162
            },
            MapTypeId = MapTypeId.Roadmap
        };
    }

    protected override async Task OnAfterRenderAsync(bool firstRender)
    {
        if (firstRender)
        {
            await jsRuntime.InvokeAsync<object>("initMapLegend");
        }
    }

    private async Task AfterMapInit()
    {
        await map1.InteropObject.AddControl(ControlPosition.TopLeft, legendReference);
    }
}

image

valentasm1 commented 9 months ago

Please register new issue then. One bug one issue.

cruiserkernan commented 9 months ago

I can do that, I just thought that since it was a caused by the serialization it was one bigger issue, that control position was just one case where I noticed it. But I understand that it might be a separate thing and could be fixed with less of a code change. I will create a issue for only the control position and and a pull request only for that.

valentasm1 commented 9 months ago

No need PR. Just sample of code how it should is enough since change is very small

valentasm1 commented 9 months ago

I merger PR. It dont work. I tried for 30min and give up. Reverted. If you do functionality. Startup server demo and try markers diff func, drawing polygons, polylines. Maybe smth else. I am a bit disappointed.

cruiserkernan commented 9 months ago

Sry my bad, I should have tested it more. I will update the code so that it works and thoroughly test it.

Enritski commented 8 months ago

I've been digging into this, and experimenting with a different custom serializer for enums. I've come to the conclusion that a big part of the problem is due to the use of discriminated unions (OneOf) for various properties. If a type has a property that uses OneOf, the OneOfConverterFactory is used to serialize that property and does so by using the default serializer for whichever of the underlying types is being used. So, for example, in the case of the Symbol class, the Path property is a discriminated union: OneOf<SymbolPath, string>. The SymbolPath type is an enum and will serialize properly using the custom serializer (e.g., JsonStringEnumConverterEx), but not when it is part of a OneOf as is the case here. I experimented with making the Path property just a simple SymbolPath instead of OneOf<SymbolPath, string> and that worked. I've been trying to work out how to have the OneOfConverterFactory use the appropriate serializer for each of the possible types in the discriminated union, but haven't got there yet. I just thought I'd add this comment here in case anyone else has an idea how to achieve this.

valentasm1 commented 8 months ago

Sharing knowledge is always great. Thank you.

valentasm1 commented 4 weeks ago

Closing due to i dont get issues regarding enum serialization. Current implementation not perfect, but good enough.