fix: wheel zoom normalization #5165
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
From playing around a little more, I actually think that if anything, me may actually want to reduce the zoom amount when very zoomed out. |
are we talking about trackapad or in general? |
Ok, playing with it, it's definitely wild. On macbook trackpad, it's high, but manageable — but we should still lower it across all levels of zoom, not just when zoomed out. But mouse wheel is not not very usable after the change — previously zooming with mouse wheel was very solid, so I'd try to match the same. |
johtso
force-pushed
the
wheel-zoom-normalization
branch
from
4a05794
to
0e8accf
Compare
|
After taking another look, I think the whole "normalization" thing was a bit of a red herring. The real issue was that we were rounding every zoom change to the nearest "zoom step", and because trackpads produce lots of tiny wheel events, they never hit the next zoom step until you are swiping way too fast. Removing that rounding, things are a fair bit better. The speed of zooming is still way too fast when over 100%, but I'm not sure how to improve that without affecting the mechanical scroll wheel behaviour which is okay at the moment. I don't know of any way to distinguish between a mechanical scroll wheel and a trackpad. Possibly we could do something as simple as changing behaviour if the delta is over a certain amount.. |
|
With the "reduced amplification for small deltas" tweak things are working quite nicely! |
src/components/App.tsx
Outdated
| // reduced amplification for small deltas (small movements on a trackpad) | ||
| const smallDeltaAdjustment = Math.min(1, absDelta / 20); | ||
| newZoom += | ||
| Math.log10(Math.max(1, this.state.zoom.value)) * | ||
| -sign * | ||
| smallDeltaAdjustment; |
There was a problem hiding this comment.
| // reduced amplification for small deltas (small movements on a trackpad) | |
| const smallDeltaAdjustment = Math.min(1, absDelta / 20); | |
| newZoom += | |
| Math.log10(Math.max(1, this.state.zoom.value)) * | |
| -sign * | |
| smallDeltaAdjustment; | |
| newZoom += | |
| Math.log10(Math.max(1, this.state.zoom.value)) * | |
| -sign * | |
| // reduced amplification for small deltas (small movements on a trackpad) | |
| Math.min(1, absDelta / 20) |
I think it's better to inline this.
Btw, doesn't that expression amplifies (clamped to 1) instead of reducing the amplification?
There was a problem hiding this comment.
It's going to result in multiplying by a number between 0 and 1, so it will reduce the amplification.
There was a problem hiding this comment.
nvm. Even though I myself said we're clamping to 1, my mind was for some reason in >1 numbers :).
Co-authored-by: David Luzar <luzar.david@gmail.com>
Playing around with the wheel zoom normalization to try and get it to behave nicely on trackpads.
It seems the go-to logic for getting some kinds of normalization between different browsers / devices is some hand-me-down facebook code. https://github.com/basilfx/normalize-wheel
I've tried this in firefox and chrome with both a mouse wheel and trackpad, and it seems to behave fine.
The only thing that could be slightly better is the velocity when very zoomed in, currently it moves at bit fast. Would just be a question of tweaking the maths a little..
https://www.loom.com/share/dff08cfe53d04719bcf7da624b4d6149
Fixes #4385
I think the normalize-wheel repo is just a copy of what's on npm with types added? In that case it might be better to use the NPM version and just add the types locally?