Repository navigation
Keep an explicitly set markerTilingOptions through the content builder - #26
Merged
Merged
Conversation
MapViewContent.merge carried markerTilingOptions over only when it was
enabled, because a fresh content's .Default and a deliberate .Disabled
could not be told apart. Every builder closure goes through buildBlock,
which merges into a fresh content, so .Disabled set inside the builder
(e.g. the sample's `{ var c = MapViewContent(); ...; return c }()` form)
came out as the default and the markers were tiled anyway. Returning the
content from the map view's closure with `return` skipped the builder,
which is why that form worked.
markerTilingOptions now records whether it was set. merge takes an
explicit setting from the other side whatever its value, so the later
explicit one wins, and leaves an unset one alone, which still reads
.Default. It is the only non-collection field of MapViewContent; the
others are appended. The public property keeps its type and get/set; the
API baseline only changes from a stored to a computed property.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MapViewContent.merge only carried markerTilingOptions over when it was enabled, so it could not tell a fresh content's .Default from a deliberately set .Disabled. buildBlock merges even a single expression into a new MapViewContent, so .Disabled set inside the map's content builder was dropped and tiling stayed on.
Now the content remembers whether the option was set; merge carries any explicitly set value (enabled or disabled, last one wins) and leaves an unset one alone. The public property keeps its type and get/set; the API baseline changes only from a stored property to { get set }.
Tests: MapViewContentMergeTests (7), which fail on the old merge; full suite 334 pass. On an iPad with 2,500 markers on MapLibre, .Disabled in the builder used to give one marker tile source and no individual markers; now no tile source and individual markers. Enabled tiling still tiles. All ios-for-* providers read content.markerTilingOptions the same way, so no provider changes.