Pinch zoom: allow fractional zoom #6224
Conversation
|
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 |
|
I agree with @ahocevar on the name and the default behavior. |
|
And maybe add a note in the upgrade notes about the changed behaviour and how to reestablish the old one? |
aAXEe
force-pushed
the
pinZoom-allowFractionalZoom
branch
from
42094f6
to
34fb89d
Compare
|
I changed the parameter to The example now shows how to establish the old behavior with a constrained zoom. Can I already add a note to the upgrade notes? |
|
Thanks @aAXEe. Yes, you can add a note to |
| /** | ||
| * `true` to zoom to next whole-number zoom after the pinch completes; | ||
| * `false` to keep the zoom at an fractional number. | ||
| * Default is `false`. |
There was a problem hiding this comment.
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.
| * @type {boolean} | ||
| */ | ||
| this.constrainResolution_ = options.constrainResolution ? | ||
| options.constrainResolution : false; |
There was a problem hiding this comment.
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;There was a problem hiding this comment.
Alternatively, to avoid repeating the variable as the conditional and consequent, this could be:
this.constrainResolution_ = options.constrainResolution || false;There was a problem hiding this comment.
The second one is not only simpler and easier to read .. it also solves the indention problem ;)
Thanks for the notes!
aAXEe
force-pushed
the
pinZoom-allowFractionalZoom
branch
from
f2ec4a3
to
24c7e8f
Compare
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`.
aAXEe
force-pushed
the
pinZoom-allowFractionalZoom
branch
from
24c7e8f
to
61ac0c4
Compare
|
Thanks for the great contribution @aAXEe! |
|
This really made a mess out of all my zoom level dependent functions and layer changes. I had to add the new PinchZoom code with constrainResolution set to true to fix it. My comment is that it would probably be better to leave old functionality the as the default on new additions like this. It would be better to let everyone choose to add new code to get new functionality rather than requiring everyone to add code to fix things that get broken if they don't add the new code. |
This adda a parameter
keepFractionalZoomLevelto theol.interaction.PinchZoomconstructor so that the user can choose whether to keep the fractional zoom or not.See #6223