-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Make Select interaction work when there are multiple instances #10699
Conversation
feature.setStyle(this.featureStyleAssociation_[key]); | ||
delete this.featureStyleAssociation_[key]; | ||
const selectInteractions = /** @type {Array<Select>} */ (this.getMap().getInteractions().getArray().filter(function(interaction) { | ||
return interaction instanceof Select && interaction.getFeatures().getArray().indexOf(feature) !== -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ahocevar for this PR. It seems to me that the filter condition should check if the feature has style.
return interaction instanceof Select && interaction.getStyle() && interaction.getFeatures().getArray().indexOf(feature) !== -1;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michalzielanski interaction.getStyle() !== false
to be precise:
style
: Style for the selected features. By default the default edit style is used (see module:ol/style). If set to false the selected feature's style will not change.
Can you create a follow-up pull request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other places there is no strong type checking. There is only if (this.style_)
, so null
instead of false
will also work. Using interaction.getStyle() !== false
only here will cause a malfunction if someone uses null
instead of false
.
Creating PR would take me half a day, you will do it quicker :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #10708. Feel free to buy me a beer @michalzielanski ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
Fixes #10501