downforacross / downforacross.com

Web frontend for downforacross.com -- continuation of stevenhao/crosswordsio
https://downforacrosscom.downforacross1.now.sh
MIT License
233 stars 91 forks source link

Fix: Can't toggle selected direction on tablet #182

Closed ajhyndman closed 3 years ago

ajhyndman commented 3 years ago

Fixes #181

After a simple touch event, modern mobile browsers fire a simulated "click" event. I believe what's happening in this issue is that both the click and touchend event handlers are calling onClick for each tap on the screen. This causes the state to flip from row to column mode and back.

There are more than one approach to fixing this issue:

  1. At smaller screen sizes, when this app's extra "mobile" features kick in, this issue is no longer present. Activate that mode earlier, or port whatever logic changed between modes back to the tablet.
  2. In the touchend event handler, just before calling onClick, call e.preventDefault(). This prevents the touch event from generating a simulated click event.

However, it strikes me that the "click" event generated by browsers is exactly what these touch handlers are attempting to emulate. Unless I'm missing some context, they appear to be redundant.

This diff just removes the touch handlers from Cell.tsx. I tested that this fixes the perceived issue on iPad. I also tested on a Samsung S9 to confirm that touch events can still toggle column/row and there were no regressions to gesture-based pan and zoom.

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

šŸ” Inspect: https://vercel.com/downforacross/downforacross.com/EPV1RAfyEsM2BH7zdXGtPaUcc7Ts
āœ… Preview: https://downforacrossc-git-fork-ajhyndman-181-toggle-direction-i-31e915.vercel.app

ajhyndman commented 3 years ago

If I recall correctly, the start/end logic is there in case the click event doesn't fire for some reason (e.g. if the user drags their finger a bit on accident while doing the click?).

My point was that browsers already implement their own heuristics for disambiguating between a short drag and a "click". At this point, those heuristics are pretty sophisticated, so I'd suggest that re-implementing that heuristic in this codebase is probably redundant.

That said, I'm happy to update this diff with a preventDefault approach instead.

stevenhao commented 3 years ago

Thanks for the update. I don't know enough to comment on the heuristics that most devices use for detecting click events and supporting short drags -- that's an area I'd need to do more research and experimentation in. For now, I'm happy with listening to touchend instead of click, and calling e.preventDefault() to suppress the click event.

Thanks for the contribution!