Pinch zoom: allow fractional zoom #6224

Merged
merged 3 commits into from Dec 9, 2016

Projects

None yet

4 participants

@aAXEe
Contributor
aAXEe commented Dec 8, 2016

This adda a parameter keepFractionalZoomLevel to the ol.interaction.PinchZoom constructor so that the user can choose whether to keep the fractional zoom or not.

See #6223

@ahocevar
Member
ahocevar commented Dec 8, 2016

Nice! In the spirit of #6113 I'd even say that unconstrained zooming should become the default, and the option to constrain resolutions should be named constrainResolution.

@tschaub
Member
tschaub commented Dec 9, 2016

I agree with @ahocevar on the name and the default behavior.

@marcjansen
Member

And maybe add a note in the upgrade notes about the changed behaviour and how to reestablish the old one?

@aAXEe
Contributor
aAXEe commented Dec 9, 2016

I changed the parameter to constrainResolution and the default behavior to keep fractional zooms.

The example now shows how to establish the old behavior with a constrained zoom.

Can I already add a note to the upgrade notes?

@ahocevar
Member
ahocevar commented Dec 9, 2016

Thanks @aAXEe. Yes, you can add a note to changelog/upgrade-notes.md, under the ### Next release heading. The release manager will take these notes and copy them to their own file when the release is being cut.

externs/olx.js
+/**
+ * `true` to zoom to next whole-number zoom after the pinch completes;
+ * `false` to keep the zoom at an fractional number.
+ * Default is `false`.
@tschaub
tschaub Dec 9, 2016 Member

When documenting booleans, just describe the behavior when true (and then identify the default). For example, this could be:

Zoom to the closest integer zoom level after the pinch gesture ends. Default is false.

src/ol/interaction/pinchzoom.js
+ * @type {boolean}
+ */
+ this.constrainResolution_ = options.constrainResolution ?
+options.constrainResolution : false;
@tschaub
tschaub Dec 9, 2016 Member

I'll see about updating our linter config to catch this. There should be two indents in a multi-line conditional expression. E.g.

var foo = bar ?
    bam : baz;
@tschaub
tschaub Dec 9, 2016 Member

Alternatively, to avoid repeating the variable as the conditional and consequent, this could be:

this.constrainResolution_ = options.constrainResolution || false;
@aAXEe
aAXEe Dec 9, 2016 Contributor

The second one is not only simpler and easier to read .. it also solves the indention problem ;)
Thanks for the notes!

aAXEe added some commits Dec 9, 2016
@aAXEe aAXEe add an option to make the pinchZoom constrain zoom levels to integers
Changes the default behavior of ol.interaction.PinchZoom to keep
the fractional zoom level after the gesture.
The old behavior of zooming to the next whole-number level can be
enforced with the new parameter `constrainResolution`.
321b1f7
@aAXEe aAXEe add a pinchZoom example demonstrating the old behavior with constrain…
…ed zooms
f5a27d2
@aAXEe aAXEe add an upgrade note about the new pinchZoom behavior
61ac0c4
@tschaub tschaub merged commit 4ff87a0 into openlayers:master Dec 9, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.005%) to 86.678%
Details
@tschaub
Member
tschaub commented Dec 9, 2016

Thanks for the great contribution @aAXEe!

@aAXEe aAXEe deleted the aAXEe:pinZoom-allowFractionalZoom branch Dec 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment