Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Style color object for IconStyleOptions. #87

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

Horndev
Copy link
Contributor

@Horndev Horndev commented Oct 17, 2024

  • OL js represents RGBA as int array and setting Color breaks JSInterop when string. This is due to an object seriallizatoin error if color is not null for methods such as new feature added, or feature clicked.
  • Future extensions could include allowing Color application along with other style options in the SetCustomImageStyle method.
  • Setting the style color allows image markers to be tinted.

Without public access to Color, the style can now be set for image markers:

newMarker.Styles[0].Icon.Color = new int[] { 125, 0, 0, 255 }; // For example

The effect is that the same image (icon) can be used to represent different things but styled with different color tints.

Possible alternative could be to create a custom Json Serializer that maps string RGB / Hex values in C# with int[] arrays being serialized on js side. Passing a string to color does almost work, but any JSInterop calls break as OL converts the strings to int arrays in the js object (causing serialization back to C# object to fail).

- OL js represents RGBA as int array and setting Color breaks JSInterop when string.
- Future extensions could include allowing Color Setting in the SetCustomImageStyle method.
- Setting the style color allows image markers to be tinted.
@lolochristen
Copy link
Owner

Thanks for your PR and input. The Color object is indeed an array of int. At other places it supports the object ColorLike which is a string, but not here. To make this all proper, I suggest having a Color struct on the Blazor side, which is internally an array but supports assigning strings to keep compatibility. However, I'll approve your PR because for IconStyle.Color only colors as arrays are accepted by API. thx

@lolochristen lolochristen reopened this Oct 28, 2024
@lolochristen lolochristen self-assigned this Oct 28, 2024
@lolochristen lolochristen merged commit 7d73045 into lolochristen:main Oct 28, 2024
3 of 6 checks passed
@Horndev
Copy link
Contributor Author

Horndev commented Oct 28, 2024

Thanks for your PR and input. The Color object is indeed an array of int. At other places it supports the object ColorLike which is a string, but not here. To make this all proper, I suggest having a Color struct on the Blazor side, which is internally an array but supports assigning strings to keep compatibility. However, I'll approve your PR because for IconStyle.Color only colors as arrays are accepted by API. thx

I agree! This is more of a quick fix that "makes thing work" for now without breaking anything else. A full solution should be implemented, like you point out. Thanks :)

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.

2 participants