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

Improve docs for interaction.Select #2269

Merged
merged 1 commit into from Jul 3, 2014
Merged

Improve docs for interaction.Select #2269

merged 1 commit into from Jul 3, 2014

Conversation

probins
Copy link
Contributor

@probins probins commented Jun 29, 2014

Attempts to make the options clearer.

@@ -1928,7 +1928,8 @@ olx.interaction.SelectOptions.prototype.addCondition;
/**
* A function that takes an {@link ol.MapBrowserEvent} and returns a boolean
* to indicate whether that event should be handled.
* By default, {@link ol.events.condition.singleClick} toggles the selection.
* By default, {@link ol.events.condition.singleClick} makes this feature
* the selected one. Additional features can be added with the `toggle` option.
Copy link
Member

Choose a reason for hiding this comment

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

I do not understand "By default, {@link ol.events.condition.singleClick} makes this feature the selected one." Is it just me?

@probins probins closed this Jun 30, 2014
@probins probins reopened this Jun 30, 2014
@probins
Copy link
Contributor Author

probins commented Jun 30, 2014

rewritten. Hope this is now clear

@marcjansen
Copy link
Member

I like it. @elemoine, do you agree?

@@ -1928,7 +1928,12 @@ olx.interaction.SelectOptions.prototype.addCondition;
/**
* A function that takes an {@link ol.MapBrowserEvent} and returns a boolean
* to indicate whether that event should be handled.
* By default, {@link ol.events.condition.singleClick} toggles the selection.
* This is the toggle for the selected features as a whole. By default, this is
Copy link
Member

Choose a reason for hiding this comment

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

I find the word "toggle" a bit confusing here. But again this may be just me. Leave it if you think it makes sense.

@elemoine
Copy link
Member

elemoine commented Jul 3, 2014

It looks good to me too. I just added two new comments. We can merge this when my second comment is addressed. Thanks!

@probins
Copy link
Contributor Author

probins commented Jul 3, 2014

revised version

elemoine pushed a commit that referenced this pull request Jul 3, 2014
Improve docs for interaction.Select
@elemoine elemoine merged commit 76e1ca6 into openlayers:master Jul 3, 2014
@probins probins deleted the select branch July 3, 2014 19:00
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

3 participants