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(geo-layers): TileLayer default autoHighlight #8798

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

Pessimistress
Copy link
Collaborator

@Pessimistress Pessimistress commented Apr 16, 2024

Closes #8787

CompositeLayer by default auto highlights all sub layers, assuming they all share the same data. This is true for most layers except overridden in GeoJsonLayer (which selectively passes highlight down depending on the feature), and in tiled layers. The basic TileLayer does not have any knowledge of how renderSubLayers uses the data, therefore there isn't a default behavior that will work for every use case. However, it is safer (and more correct?) to only highlight the sub layer that is being picked, than to assume that all sublayers should be highlighted at the same picked index.

For discussion: this is technically a breaking change (bug originally introduced in a 8.8 patch #7650), is it ok to include it as a patch?

Change List

  • Add picked sub layer to TileLayerPickingInfo
  • Only auto highlight the picked sub layer

Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

For discussion: this is technically a breaking change (bug originally introduced in a 8.8 patch), is it ok to include it as a patch?

No good sense for whether apps have come to rely on the buggy behavior, but there seems to be no easy way of getting back to that behavior should it be desired.

Perhaps it would help to include the upgrade-guide bullet, and doc update for the change into this PR?

@coveralls
Copy link

Coverage Status

coverage: 90.01% (+0.008%) from 90.002%
when pulling 73ad676 on x/tile-layer-autohighlight
into fa029dd on master.

@Pessimistress
Copy link
Collaborator Author

there seems to be no easy way of getting back to that behavior should it be desired

That is correct. To highlight multiple layers simultaneously in a tile, you'll need to wrap them in a CompositeLayer.

The buggy behavior was introduced at the end of v8.8 timeframe, and part of 8.9.0-beta.2. I think most users of the TileLayer use GeoJsonLayer/BitmapLayer to render their tiles, and they shouldn't be affected either way.

@Pessimistress Pessimistress merged commit 8827177 into master Apr 18, 2024
4 checks passed
@Pessimistress Pessimistress deleted the x/tile-layer-autohighlight branch April 18, 2024 01:10
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.

[Bug] autoHighlight on a TileLayer with multiple sub layers
4 participants