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

new feature: set arbitrary layer properties #303

Merged
merged 30 commits into from
Nov 13, 2023

Conversation

m0nac0
Copy link
Collaborator

@m0nac0 m0nac0 commented Sep 23, 2023

This introduces a new method: MaplibreMapController#setLayerProperties to change any properties of any given layer at runtime.

I had to fix LayerPropertyConverter for android and iOS because there was a problem with the visibility property (which gets somewhat special treatment, and did not work because it received a string like ""visible"" instead of "visible" (notice the unnecessary quotes)). In the process I also fixed the script to generate these files.

TODO:

  • update example app to test other properties and layer types

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for review: this file was automatically generated by the script.
Apparently it was manually formatted in the past, but I did not do this again.
It is much easier to review when enabling "hide whitespace changes"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also automatically generated

@m0nac0 m0nac0 marked this pull request as ready for review October 5, 2023 09:45
@m0nac0 m0nac0 marked this pull request as draft October 5, 2023 09:45
@m0nac0 m0nac0 marked this pull request as ready for review October 6, 2023 09:18
@m0nac0 m0nac0 changed the title WIP: set arbitrary layer properties new feature: set arbitrary layer properties Oct 6, 2023
@m0nac0
Copy link
Collaborator Author

m0nac0 commented Oct 6, 2023

Closes #250

@JulianBissekkou
Copy link
Collaborator

@stefanschaller can you take a look?

Comment on lines 889 to 898
try {
_map.setLayoutProperty(layerId, entry.key, entry.value);
} catch (e) {
print('Caught exception (usually safe to ignore): $e');
}
try {
_map.setPaintProperty(layerId, entry.key, entry.value);
} catch (e) {
print('Caught exception (usually safe to ignore): $e');
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
try {
_map.setLayoutProperty(layerId, entry.key, entry.value);
} catch (e) {
print('Caught exception (usually safe to ignore): $e');
}
try {
_map.setPaintProperty(layerId, entry.key, entry.value);
} catch (e) {
print('Caught exception (usually safe to ignore): $e');
}
try {
_map.setLayoutProperty(layerId, entry.key, entry.value);
_map.setPaintProperty(layerId, entry.key, entry.value);
} catch (e) {
print('Caught exception (usually safe to ignore): $e');
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think with your suggestion, if it is a paint property, the setLayoutProperty method would (usually?) throw an error and the setPaintProperty method would not be called.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stefanschaller What do you think, is it fine to keep the code as is with that explanation?

@stefanschaller
Copy link
Collaborator

Looks good to me. I didn't have any issues with the layer api @JulianBissekkou

@m0nac0
Copy link
Collaborator Author

m0nac0 commented Nov 10, 2023

@stefanschaller @JulianBissekkou Can we merge this?

@JulianBissekkou JulianBissekkou enabled auto-merge (squash) November 13, 2023 05:37
@JulianBissekkou JulianBissekkou merged commit 218a7fb into maplibre:main Nov 13, 2023
6 checks passed
@m0nac0 m0nac0 deleted the m0-set-layer-properties branch November 13, 2023 10:01
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.

None yet

3 participants