View / apply constraints when an interaction starts #9404
Conversation
There was a problem hiding this comment.
Thanks @jahow. This looks good. Would be great if you could comment on the two questions I added. The reason why I'm asking is because it strikes me as a flaw that interactions and controls are now responsible for updating the constraints for animations, but the view can handle that itself for interactions.
src/ol/control/Zoom.js
Outdated
| // upon it | ||
| return; | ||
| } | ||
| view.resolveConstraints(0); |
There was a problem hiding this comment.
I'm wondering if a better place for this new line this would be somewhere in ol/View#animate().
There was a problem hiding this comment.
I've thought about that as well but didn't manage to get it to work: it was either breaking the "smoothly return view in allowed extent/resolution" effect at the end of an interaction, or causing regressions in other places, depending on where I'd put it.
I agree this does not feel right. To have a cleaner system would mean some rework of the animate method IMO, which is a tricky task. The ideal solution would be to get rid of all these applyTargetState_ and resolveConstraints calls and only translate target values into actual view parameters on the main render loop (ans as such apply constraints only once per frame).
src/ol/interaction/Interaction.js
Outdated
| const newZoom = view.getConstrainedZoom(currentZoom + delta); | ||
| const newResolution = view.getResolutionForZoom(newZoom); | ||
|
|
||
| view.resolveConstraints(0); |
There was a problem hiding this comment.
Is this needed? Or will it still be needed if ol/View#animate() handles it (like I suggested in the previous comment)?
test/spec/ol/view.test.js
Outdated
| disposeMap(map); | ||
| }); | ||
|
|
||
| it.only('works when initialized with #setCenter() and #setZoom()', function(done) { |
test/spec/ol/view.test.js
Outdated
| }, 500); | ||
| }); | ||
|
|
||
| it.only('works when initialized with #animate()', function(done) { |
There was a problem hiding this comment.
.only also should be removed here.
Previously, an interaction could begin while target values (center/resolution) were out of the allowed range, causing a glitch where the view zoom/position would jump suddenly.
jahow
force-pushed
the
fix-view-jump-glitch
branch
from
3b893fb to
0f73f16
Compare
|
TL;DR: could not find a better way with the time & means available to me, sorry! |
|
Works fine! well done |
2 participants
Previously, an interaction could begin while target values (center/resolution) were out of the allowed range, causing a glitch where the view zoom/position would jump suddenly.
@ahocevar I've reused your test from #9399 & expanded on it.
Fixes #9396.