fix: mobile on-screen buttons, reduce spacing, add pause button#4
Conversation
Agent-Logs-Url: https://github.com/armin-l/Snake/sessions/d6ee5583-15f2-43a2-a8f9-eb8784b246b0 Co-authored-by: armin-l <107995326+armin-l@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves the mobile control experience for the Snake game by adding a dedicated pause button, tightening mobile UI spacing, and adjusting touch handling so on-screen controls don’t interfere with swipe input.
Changes:
- Added a pause button to the on-screen controls and wired it into the game’s pause state.
- Improved touch behavior to avoid treating control-button touches as swipe gestures.
- Reduced vertical spacing between the canvas, effects bar, and controls on mobile.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
snake.js |
Adds pause toggle function, pause button wiring, and touch filtering for control buttons. |
snake.css |
Positions the new pause button in the control grid and reduces mobile margins above effects/controls. |
index.html |
Adds the pause button element to the control cluster. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -982,6 +992,16 @@ function wireEvents() { | |||
| DOM.btnDown.addEventListener('click', () => tryDir('DOWN')); | |||
| DOM.btnLeft.addEventListener('click', () => tryDir('LEFT')); | |||
| DOM.btnRight.addEventListener('click', () => tryDir('RIGHT')); | |||
| [ | |||
There was a problem hiding this comment.
The touch+click wiring for movement is duplicated: the four direction buttons get individual click listeners and then the same mapping is repeated again in the array for touchstart. This makes the control bindings harder to maintain and increases the risk of double-firing on some devices/browsers. Consider defining the button→handler mapping once and registering both click and touchstart from that single list (or remove the per-button click listeners and attach click in the loop too).
There was a problem hiding this comment.
Done in 645bc58. The button→handler mapping is now defined once and both click and touchstart are registered from that single list, removing the duplication. Also extracted an isCtrlBtn(target) helper that guards closest() with an instanceof Element check, preventing a potential throw when the touch target is a Text node (e.g. tapping the button icon).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tanceof check Agent-Logs-Url: https://github.com/armin-l/Snake/sessions/04827a4b-9a8e-40f5-a406-ecacee22e6bc Co-authored-by: armin-l <107995326+armin-l@users.noreply.github.com>
Agent-Logs-Url: https://github.com/armin-l/Snake/sessions/d6ee5583-15f2-43a2-a8f9-eb8784b246b0