Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added event listeners to TwistyPlayer #345

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Dave2ooo
Copy link

Added event listeners that trigger when the user interacts with visualizationWrapperElem of TwistyPlayer.

  • canvas-mousedown
  • canvas-mouseup
  • canvas-touchstart
  • canvas-touchend
  • canvas-click

I implemented the click event for Feature Request #344.
Unfortunately, attaching a click event listener to visualizationWrapperElem would also trigger the event when the mouse is released at a different point than where it was pressed. Therefore, I added all these event listeners to check if the click and release point is the same to trigger the custom click event.

This is my stub.ts for testing:

import { TwistyPlayer } from "cubing/twisty";
const twistyPlayer = document.body.appendChild(new TwistyPlayer());
twistyPlayer.addEventListener("canvas-click", (e: MouseEvent) => { console.log("Click", e); });
twistyPlayer.addEventListener("canvas-mousedown" , (e: MouseEvent) => { console.log("Mousedown", e); });
twistyPlayer.addEventListener("canvas-mouseup", (e: MouseEvent) => { console.log("Mouseup", e); });
twistyPlayer.addEventListener("canvas-touchstart", (e: TouchEvent) => { console.log("Touchstart", e); });
twistyPlayer.addEventListener("canvas-touchend", (e: TouchEvent) => { console.log("Touchend", e); });

During testing, I encountered an issue with the Inspection tool in Google Chrome. While simulating a handheld device, clicking with the mouse (same press and release point) triggers touch and mouse events. This results in triggering the canvas-click event twice. This might be an issue for developers implementing TwistyPlayer. Should this be tackled?
In Firefox, simulation a handheld device only triggers mouse events.

Are the names of the events okay like that or is there a better idea?

Please tell me if the code is unclear or can be written in a better way.

@lgarron
Copy link
Member

lgarron commented Dec 14, 2024

Hmm, I'm amenable to exposing enough to get the events, but I don't want to be in the business of re-implementing parts of the DOM API. Do you think there could be something which bypasses the need for cubing.js to do anything that could get out of sync with the DOM API?

@Dave2ooo
Copy link
Author

I didn't think about this possibility. This new implementation calls a callback function for each event and passes the event as argument.

This is my stub.ts for testing:

import { TwistyPlayer } from "cubing/twisty";
const twistyPlayer = document.body.appendChild(new TwistyPlayer());
twistyPlayer.onMouseDown = (event) => { console.log("Mouse down detected:", event); };
twistyPlayer.onMouseUp = (event) => { console.log("Mouse up detected:", event); };
twistyPlayer.onTouchStart = (event) => { console.log("Touch start detected:", event); };
twistyPlayer.onTouchEnd = (event) => { console.log("Touch end detected:", event); };
twistyPlayer.onClick = (event) => { console.log("Click detected!", event); };

Is it okay to have the this.addEventListeners(); in the constructor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants