search

OneZoom / OZtree

OneZoom Tree of Life Explorer
Other
91 stars 20 forks source link

No image panning #906

Open jaredkhan opened 6 days ago

jaredkhan commented 6 days ago

Fix #900

Just turns off image-based panning on mobile as suggested by @lentinj, a performance optimisation that's not really necessary anymore and was causing this blank space problem

jaredkhan commented 6 days ago

We had no such optimisation in place for pinch-to-zoom anyway, so users whose mobiles can't keep up with the render would already be struggling to it. Panning still works well here on the iPhone 13 Pro that I'm using but admittedly that's only a few years old. It's not flawless, there are occasional frame drops when panning into un-developed areas of the tree, but I think it's better than not being able to see where you're going. We should try it on some lower-powered devices.

davidebbo commented 6 days ago

I pulled the change, but I'm not sure quite what to look for. Is the difference only observable on mobile? I have a touchscreen laptop, but can't really tell the difference between old and new when I pinch zoom and pan.

lentinj commented 5 days ago

@davidebbo Yeah, we don't use image-based panning on desktops, based on useragent:

https://github.com/OneZoom/OZtree/blob/7453fdbf7596337da1a04bbe961446beb5c85637/OZprivate/rawJS/OZTreeModule/src/util/general.js#L13-L20

@jaredkhan with this pull I'm fairly sure that the is_on_mobile definition above is also useless, so the definition could also be removed whilst we're at it.

As for testing, I'll let @jrosindell weigh in but without a target minimum mobile I think we try it and revert if we get complaints.

jaredkhan commented 4 hours ago

@jaredkhan with this pull I'm fairly sure that the is_on_mobile definition above is also useless, so the definition could also be removed whilst we're at it.

@lentinj good point. Now that you mention it, we don't use the 'last_draw_by_redraw' flag anymore either. I've removed both now.