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 window size functionality #74

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mqtran01
Copy link

Added convenience method for mouse to find the size of the screen.

Useful to use to do calculations to of relative position.

@moses-palmer
Copy link
Owner

Thank you for your contribution!

I have a few questions about your implementation:

  1. You have added the display dimensions as a method, whereas other device attributes are generally accessed through properties. Perhaps a more similar interface would be better?
  2. But you have added the information to pynput.mouse.Controller. I think that this information does not necessarily belong there; you may want access to the dimensions from a listener as well. Perhaps it should be moved to a general helper function instead?
  3. The functions you use to retrieve the dimensions appear to focus mainly on the primary display. What happens when several monitors are connected?

I apologise for all questions :-) I have contemplated adding this functionality in the past, but found a simple yet non-limiting API to be a bit difficult to define.

@mqtran01
Copy link
Author

To address those comments:

  1. That can probably be done.
  2. I did not want to change much of your current implementation, and simply wanted to inject a bit of code to do that job. Unless we have something else we can something general, pynput.screen or pynput.window. I think if you do look into window handling functionality (moving active window and such), it would be worth it to create to create another module focusing on windows and screens.
  3. Not a clue. Did not think of that. Shame I only work on one screen because I am just a uni student.

Good job though. Love your package!

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