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

Allow custom mousemove conditions for Select interaction #3033

Merged
merged 1 commit into from
Dec 15, 2014

Conversation

ahocevar
Copy link
Member

When configuring a Select interaction with a custom condition that includes mousemove, panning the map will not work any more. This is because the return value of handleMapBrowserEvent is determined by checking for a default condition function. By checking for the underlying event type instead, we gain flexibility with custom condition functions.

This change makes Select interaction configurations like the following work:

new ol.interaction.Select({
  condition: function(evt) {
    return evt.originalEvent.type == 'mousemove' ||
        evt.type == 'singleclick';
  }
});

This configuration is useful for hover selection with a click fallback on mobile devices that do not have hover.

@ahocevar
Copy link
Member Author

@tsauerwein, you may be interested in reviewing this.

@@ -179,7 +179,7 @@ ol.interaction.Select.prototype.handleMapBrowserEvent =
}
features.extend(selected);
}
return this.condition_ == ol.events.condition.mouseMove;
return mapBrowserEvent.originalEvent.type == 'mousemove';
Copy link
Member

Choose a reason for hiding this comment

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

or return ol.events.condition.mouseMouve(mapBrowserEvent) to avoid duplication?

Copy link
Member

Choose a reason for hiding this comment

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

What duplication? Sorry I'm not getting it :-)

Copy link
Member

Choose a reason for hiding this comment

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

the test is the exact content of the mouseMove condition function, no?

On 15 December 2014 at 11:36, Éric Lemoine notifications@github.com wrote:

In src/ol/interaction/selectinteraction.js
#3033 (diff):

@@ -179,7 +179,7 @@ ol.interaction.Select.prototype.handleMapBrowserEvent =
}
features.extend(selected);
}

  • return this.condition_ == ol.events.condition.mouseMove;
  • return mapBrowserEvent.originalEvent.type == 'mousemove';

What duplication? Sorry I'm not getting it :-)


Reply to this email directly or view it on GitHub
https://github.com/openlayers/ol3/pull/3033/files#r21815875.

Antoine Abt

Camptocamp France SAS
Savoie Technolac, BP 352
73377 Le Bourget du Lac, Cedex

Tel : 00 33 4 79 44 44 94
Mail : antoine.abt@camptocamp.com
http://www.camptocamp.com

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense.

When configuring a Select interaction with a custom condition that
includes mousemove, panning the map will not work any more. This is
because the return value of handleMapBrowserEvent is determined by
checking for a default condition function. By checking for the
underlying event type instead, we gain flexibility with custom condition
functions.
@tonio
Copy link
Member

tonio commented Dec 15, 2014

Don’t know what the Travis error is, otherwise looks good to me.

@tsauerwein
Copy link
Member

Your change looks good to me, thanks! I guess we still have the problem reported in #2755?

@ahocevar
Copy link
Member Author

I think #2755 is unrelated to this.

ahocevar added a commit that referenced this pull request Dec 15, 2014
Allow custom mousemove conditions for Select interaction
@ahocevar ahocevar merged commit e0f574b into openlayers:master Dec 15, 2014
@ahocevar ahocevar deleted the select-return branch December 15, 2014 12:26
@fredj
Copy link
Member

fredj commented Dec 15, 2014

Yes, #2755 is not related

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.

None yet

5 participants