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

Remove touch based click tolerance #5922

Merged

Conversation

Muscot
Copy link
Contributor

@Muscot Muscot commented Nov 17, 2017

Fixes #5921


// @option tolerance: Number = 0
// How much to extend click tolerance round a path/object on the map
tolerance : 0
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the default of this be Browser.touch ? 10 : 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that we should try to build so little logic as possible round browser.touch in the library, because it's difficult to detect if it's a touch device or not. But still give the options for the users to be able to set tolerance. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Since I've supervised this PR, I think I will have to agree with @Muscot 😄

@IvanSanchez from a backwards compatibility point of view, you are right, but I think the old default does not make sense: SVG does not have the same (larger tolerance when touch is true), and actually setting the tolerance to anything other than 0 will cause problems with lines/polygons that are right next to each other (see #5144).

So, I'd argue actually defaulting to 0 is what we've always should have done.

Copy link
Member

Choose a reason for hiding this comment

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

If @perliedman says it's right to break a bit of backwards compatibility, then I have no problem with that.

@IvanSanchez IvanSanchez changed the title #5921 Remove touch based click tolerance Remove touch based click tolerance Nov 17, 2017
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.

3 participants