Pass null as forEachFeatureAtPixel layer arg for unmanaged layers #4391

Merged
merged 3 commits into from Nov 9, 2015

Conversation

Projects
None yet
2 participants
@ahocevar
Member

ahocevar commented Nov 6, 2015

Features on the removed ol.FeatureOverlay were detected by ol.Map#forEachFeatureAtPixel, and the layer arg passed to the handler function was null. This change makes it so the same behaviour applies to features on an unmanaged vector layer. Previously the forEachFeatureAtPixel handler was called with a reference to the unmanaged layer as layer arg.

See #4143 (comment).

Fixes #4114.

@ahocevar

This comment has been minimized.

Show comment
Hide comment
@ahocevar

ahocevar Nov 6, 2015

Member

@bartvde, you may want to have a look to see how this affects #4114.

Member

ahocevar commented Nov 6, 2015

@bartvde, you may want to have a look to see how this affects #4114.

@bartvde

This comment has been minimized.

Show comment
Hide comment
@bartvde

bartvde Nov 7, 2015

Member

#4114 is this still the same @ahocevar with this change
Assuming you mean if I still see the issue in your branch?

Member

bartvde commented Nov 7, 2015

#4114 is this still the same @ahocevar with this change
Assuming you mean if I still see the issue in your branch?

@ahocevar

This comment has been minimized.

Show comment
Hide comment
@ahocevar

ahocevar Nov 7, 2015

Member

@bartvde I meant you should look at my branch to see if it makes your life easier, so you don't need a code snippet like the one you posted in your application.

Your issue is unlikely to be fixed with this branch, but it might. Because now forEachFeatureAtPixel is only called once for features that are on multiple unmanaged layers.

Member

ahocevar commented Nov 7, 2015

@bartvde I meant you should look at my branch to see if it makes your life easier, so you don't need a code snippet like the one you posted in your application.

Your issue is unlikely to be fixed with this branch, but it might. Because now forEachFeatureAtPixel is only called once for features that are on multiple unmanaged layers.

@ahocevar

This comment has been minimized.

Show comment
Hide comment
@ahocevar

ahocevar Nov 8, 2015

Member

@bartvde I added a commit that fixes #4114. Please review.

Member

ahocevar commented Nov 8, 2015

@bartvde I added a commit that fixes #4114. Please review.

@bartvde

This comment has been minimized.

Show comment
Hide comment
@bartvde

bartvde Nov 8, 2015

Member

thanks @ahocevar I can confirm that this now fixes my issue from #4114
I'll do a review of the code tomorrow morning if no one gets to it before then.

Member

bartvde commented Nov 8, 2015

thanks @ahocevar I can confirm that this now fixes my issue from #4114
I'll do a review of the code tomorrow morning if no one gets to it before then.

@bartvde

This comment has been minimized.

Show comment
Hide comment
@bartvde

bartvde Nov 9, 2015

Member

LGTM @ahocevar does this deserve a mention in the release notes?

Member

bartvde commented Nov 9, 2015

LGTM @ahocevar does this deserve a mention in the release notes?

@ahocevar

This comment has been minimized.

Show comment
Hide comment
@ahocevar

ahocevar Nov 9, 2015

Member

I can add that, yes.

Member

ahocevar commented Nov 9, 2015

I can add that, yes.

@ahocevar

This comment has been minimized.

Show comment
Hide comment
Member

ahocevar commented Nov 9, 2015

Done @bartvde.

@bartvde

This comment has been minimized.

Show comment
Hide comment
@bartvde

bartvde Nov 9, 2015

Member

LGTM @ahocevar please merge if Travis agrees again

Member

bartvde commented Nov 9, 2015

LGTM @ahocevar please merge if Travis agrees again

ahocevar added a commit that referenced this pull request Nov 9, 2015

Merge pull request #4391 from ahocevar/unmanaged-layer-foreachfeature…
…atpixel

Pass null as forEachFeatureAtPixel layer arg for unmanaged layers

@ahocevar ahocevar merged commit bfbb802 into openlayers:master Nov 9, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 75.408%
Details

@ahocevar ahocevar deleted the ahocevar:unmanaged-layer-foreachfeatureatpixel branch Nov 9, 2015

ahocevar added a commit to ahocevar/openlayers that referenced this pull request Nov 18, 2015

ahocevar added a commit that referenced this pull request Nov 19, 2015

Merge pull request #4450 from ahocevar/fix-select
Fix select interaction regression caused by #4391
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment