-
Notifications
You must be signed in to change notification settings - Fork 0
adding active target mode #11
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
Conversation
Summary of ChangesHello @jsonify, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the ClickIt application by introducing an 'active target mode'. This new functionality allows users to control click automation dynamically, where the application will click at the current mouse cursor's position. It involves comprehensive changes across core logic, event handling, UI, and settings to provide a more interactive and flexible clicking experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces an 'active target mode', allowing clicks to follow the cursor's position. The implementation is solid, adding a CursorManager to handle cursor state, extending HotkeyManager to monitor mouse clicks, and updating the view model and settings to support the new mode. The changes are well-structured and handle aspects like backward compatibility and re-entrancy correctly. I have one suggestion to refactor some duplicated code in the new CursorManager for better maintainability.
| func restoreNormalCursor() { | ||
| DispatchQueue.main.async { [weak self] in | ||
| guard let self = self else { return } | ||
|
|
||
| if self.isTargetCursorActive { | ||
| self.isTargetCursorActive = false | ||
|
|
||
| // Stop the cursor update timer | ||
| self.cursorUpdateTimer?.invalidate() | ||
| self.cursorUpdateTimer = nil | ||
|
|
||
| // Restore arrow cursor | ||
| NSCursor.arrow.set() | ||
|
|
||
| self.logger.info("Normal cursor restored") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Force restore cursor (useful for cleanup on app termination) | ||
| func forceRestoreNormalCursor() { | ||
| DispatchQueue.main.async { [weak self] in | ||
| guard let self = self else { return } | ||
|
|
||
| self.isTargetCursorActive = false | ||
| self.cursorUpdateTimer?.invalidate() | ||
| self.cursorUpdateTimer = nil | ||
| NSCursor.arrow.set() | ||
|
|
||
| self.logger.info("Cursor forcefully restored") | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To improve maintainability and reduce code duplication, the common logic for resetting the cursor state in restoreNormalCursor() and forceRestoreNormalCursor() can be extracted into a private helper method. This makes the code cleaner and easier to manage.
/// Restores the normal system cursor
func restoreNormalCursor() {
DispatchQueue.main.async { [weak self] in
guard let self, self.isTargetCursorActive else { return }
self.performCursorReset()
self.logger.info("Normal cursor restored")
}
}
/// Force restore cursor (useful for cleanup on app termination)
func forceRestoreNormalCursor() {
DispatchQueue.main.async { [weak self] in
guard let self else { return }
self.performCursorReset()
self.logger.info("Cursor forcefully restored")
}
}
private func performCursorReset() {
isTargetCursorActive = false
cursorUpdateTimer?.invalidate()
cursorUpdateTimer = nil
NSCursor.arrow.set()
}
No description provided.