Fix select interaction regression caused by #4391 #4450

Merged
merged 1 commit into from Nov 19, 2015

Conversation

Projects
None yet
3 participants
@ahocevar
Member

ahocevar commented Nov 18, 2015

The select interaction determines selection candidate features by using ol.Map#forEachFeatureAtPixel. It uses an internal unmanaged layer that selected features are added to, and adds selected features to the skippedFeatures hash on the map. Before the change made with this pull request, a selected feature was immediately removed from the selection as soon as it was added to the unmanaged layer, because the unmanaged layer was ignored in forEachFeatureAtPixel (null as layer arg and if (layer && filter())). Now already selected features remain in the selection, because a null layer is accpted in the forEachFeatureAtPixel handler (if (!layer || filter())).

Fixes #4441.

@tschaub

This comment has been minimized.

Show comment
Hide comment
@tschaub

tschaub Nov 18, 2015

Member

@ahocevar can you provide a bit of a description and summary of the situation. The diff alone is a bit mysterious, and the link to #4391 leads to another ticket without a description. It would be great to get a bit of a knowledge dump on some of these tickets.

Thanks for the work getting to the bottom of this.

Member

tschaub commented Nov 18, 2015

@ahocevar can you provide a bit of a description and summary of the situation. The diff alone is a bit mysterious, and the link to #4391 leads to another ticket without a description. It would be great to get a bit of a knowledge dump on some of these tickets.

Thanks for the work getting to the bottom of this.

@ahocevar

This comment has been minimized.

Show comment
Hide comment
@ahocevar

ahocevar Nov 19, 2015

Member

@tschaub I updated the descriptions of both tickets. I hope it is clearer now - I'm a bit slow typing today, so I didn't get too verbose.

Member

ahocevar commented Nov 19, 2015

@tschaub I updated the descriptions of both tickets. I hope it is clearer now - I'm a bit slow typing today, so I didn't get too verbose.

@tschaub tschaub added the regression label Nov 19, 2015

@tschaub

This comment has been minimized.

Show comment
Hide comment
@tschaub

tschaub Nov 19, 2015

Member

Thanks for the explanation @ahocevar. This is very helpful.

Please merge.

Member

tschaub commented Nov 19, 2015

Thanks for the explanation @ahocevar. This is very helpful.

Please merge.

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

@ahocevar ahocevar merged commit aa260f8 into openlayers:master Nov 19, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.004%) to 75.395%
Details

@ahocevar ahocevar deleted the ahocevar:fix-select branch Nov 19, 2015

@tohu12

This comment has been minimized.

Show comment
Hide comment
@tohu12

tohu12 Nov 22, 2015

@ahocevar, I use two "ol.interaction.Select" for two vector layers with different style for hover.
When you move with pointer to one Feature, show two different symbol from both interaction, because every interaction work with both unmanaged layer.

Example http://jsfiddle.net/tohu/Lh76thmz/2/

What is right solution ? With v3.11.1 http://jsfiddle.net/tohu/Lh76thmz/1/ is alright, but now no.
Thanks

tohu12 commented Nov 22, 2015

@ahocevar, I use two "ol.interaction.Select" for two vector layers with different style for hover.
When you move with pointer to one Feature, show two different symbol from both interaction, because every interaction work with both unmanaged layer.

Example http://jsfiddle.net/tohu/Lh76thmz/2/

What is right solution ? With v3.11.1 http://jsfiddle.net/tohu/Lh76thmz/1/ is alright, but now no.
Thanks

@ahocevar

This comment has been minimized.

Show comment
Hide comment
@ahocevar

ahocevar Nov 22, 2015

Member

The behavior you are seeing now is the same as in 3.10.1, but I think it should be considered a (separate) bug.

Member

ahocevar commented Nov 22, 2015

The behavior you are seeing now is the same as in 3.10.1, but I think it should be considered a (separate) bug.

@ahocevar

This comment has been minimized.

Show comment
Hide comment
@ahocevar

ahocevar Nov 22, 2015

Member

@tohu12 I created #4471 to track this separate issue.

Member

ahocevar commented Nov 22, 2015

@tohu12 I created #4471 to track this separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment