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

Document and simplify clusterproperties interface #237

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

westnordost
Copy link
Collaborator

In relation to #236

Apart from adding documentation, the only thing I changed in this draft PR was so that in the ClusterPointsDemo I could write

clusterProperties = mapOf("range" to GeoJsonOptions.ClusterProperty("+", "current_range_meters"))

instead of

clusterProperties = mapOf("range" to GeoJsonOptions.ClusterProperty("+", feature.get("current_range_meters")))

(I don't mind to revert this before the (documentation) PR is merged, I was just experimenting around.)

Anyway, for some reason, this doesn't lead to a crash anymore. Can you make something of that?


Also, I can't test this on iOS, but according to the source code what seems to be expected is a dictionary with strings as keys and arrays with two expressions as values. The current/previous code seems to be just one expression.

Comment on lines +18 to +23
*
* - `cluster`: Is `true` if the point is a cluster
* - `cluster_id`: A unique id for the cluster to be used in conjunction with the cluster
* inspection methods.
* - `point_count`: Number of original points grouped into this cluster
* - `point_count_abbreviated`: An abbreviated point count
Copy link
Contributor

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
*
* - `cluster`: Is `true` if the point is a cluster
* - `cluster_id`: A unique id for the cluster to be used in conjunction with the cluster
* inspection methods.
* - `point_count`: Number of original points grouped into this cluster
* - `point_count_abbreviated`: An abbreviated point count
* - `cluster`: Is `true` if the point is a cluster
* - `cluster_id`: A unique id for the cluster to be used in conjunction with the cluster
* inspection methods.
* - `point_count`: Number of original points grouped into this cluster
* - `point_count_abbreviated`: An abbreviated point count

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change would break formatting this as a list in Android Studio.

Bad dog!

Comment on lines +31 to +32
* - the top of the viewport if [illuminationAnchor] = [IlluminationAnchor.Viewport] or
* - north if [illuminationAnchor] = [IlluminationAnchor.Map]
Copy link
Contributor

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
* - the top of the viewport if [illuminationAnchor] = [IlluminationAnchor.Viewport] or
* - north if [illuminationAnchor] = [IlluminationAnchor.Map]
* - the top of the viewport if [illuminationAnchor] = [IlluminationAnchor.Viewport] or
* - north if [illuminationAnchor] = [IlluminationAnchor.Map]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change would break formatting this as a list in Android Studio.

Bad dog!

westnordost and others added 3 commits January 8, 2025 21:04
…ecompose/core/source/GeoJsonOptions.kt

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@sargunv sargunv added this to the v0.6.0 milestone Jan 8, 2025
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