fix click firing after drag

[bob0005]

User description

Closes #1158

---

PR Type

Bug fix

---

Description

• Fixed an issue where a click event was firing after a drag operation.
• Removed the mouseup event listener and added drag detection logic within the handleMouseDown method.
• Introduced an isDragged flag to track whether a drag operation has occurred.

---

Changes walkthrough 📝

	Relevant files
Bug fix	InputManager.ts Fix click event firing after drag operation                            client/src/three/components/InputManager.ts Removed mouseup event listener. Added isDragged flag to track drag state. Implemented drag detection logic in handleMouseDown. +10/-5

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
eternum	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 9, 2024 7:04pm

[mentatbot]

You are out of MentatBot reviews. Your usage will refresh August 12 at 07:00 AM.

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Drag Threshold The drag detection logic uses a hard-coded threshold of 10 pixels. Consider making this a configurable parameter to allow easier adjustments based on different device resolutions or user preferences. Event Listener Removal The 'mousemove' event listener is removed as soon as the drag condition is met. This could lead to issues if the user stops and then starts moving the mouse again within the same mousedown session. Consider handling this scenario more gracefully.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Robustness	Add error handling to the drag detection function Add error handling for the checkDrag function to manage exceptions that may occur during the drag detection process, ensuring the application's robustness. client/src/three/components/InputManager.ts [79-83] const checkDrag = (e: MouseEvent) => { - if (Math.abs(this.mouseX - e.clientX) > 10 || Math.abs(this.mouseY - e.clientY) > 10) { - this.isDragged = true; - window.removeEventListener("mousemove", checkDrag); + try { + if (Math.abs(this.mouseX - e.clientX) > 10 || Math.abs(this.mouseY - e.clientY) > 10) { + this.isDragged = true; + window.removeEventListener("mousemove", checkDrag); + } + } catch (error) { + console.error("Failed to execute checkDrag:", error); } }; Suggestion importance[1-10]: 9 Why: Adding error handling to the checkDrag function increases the robustness of the application by managing potential exceptions during the drag detection process.	9
Maintainability	Replace hardcoded values with a constant for the drag threshold Replace the hardcoded drag threshold value with a constant to improve maintainability and readability. This allows easy adjustments and ensures consistency across the codebase if used in multiple places. client/src/three/components/InputManager.ts [80-82] -if (Math.abs(this.mouseX - e.clientX) > 10 || Math.abs(this.mouseY - e.clientY) > 10) { +const DRAG_THRESHOLD = 10; +if (Math.abs(this.mouseX - e.clientX) > DRAG_THRESHOLD || Math.abs(this.mouseY - e.clientY) > DRAG_THRESHOLD) { this.isDragged = true; window.removeEventListener("mousemove", checkDrag); } Suggestion importance[1-10]: 8 Why: Using a constant for the drag threshold improves maintainability and readability, making it easier to adjust the value and ensuring consistency across the codebase.	8
Best practice	Ensure proper cleanup of event listeners Ensure the removal of the "mousemove" event listener when the component is destroyed or when it is no longer needed to prevent potential memory leaks and performance issues. client/src/three/components/InputManager.ts [86] window.addEventListener("mousemove", checkDrag); +// Ensure to remove this event listener appropriately when not needed anymore Suggestion importance[1-10]: 7 Why: Adding a comment to remind developers to remove the event listener helps prevent potential memory leaks and performance issues, although it would be better to implement the actual cleanup code.	7
Readability	Use a more descriptive type name for mouse event types Consider using a more descriptive type name than ListenerTypes to reflect that it specifically relates to mouse events, enhancing code readability and maintainability. client/src/three/components/InputManager.ts [5] -type ListenerTypes = "click" | "mousemove" | "contextmenu" | "dblclick" | "mousedown"; +type MouseEventType = "click" | "mousemove" | "contextmenu" | "dblclick" | "mousedown"; Suggestion importance[1-10]: 6 Why: A more descriptive type name enhances code readability and maintainability, but the current name is not incorrect and the improvement is minor.	6