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

Changed mouseScroll to use X and Y as direction. #194

Merged
merged 11 commits into from
Feb 23, 2017

Conversation

BHamrick1
Copy link
Contributor

For my project I require side to side scrolling and have found that robotjs does not have any way of doing this. This change will make it so that you can specify a x and y coordinate like a vector for scrolling. Also instead of using 'up' and 'down' we would use negative and positive values.

I know that this removes how we previously did things so I am not sure if this needs to be a separate method or if we need to mark something as deprecated. I did it this way because robotjs is still relatively in alpha or pre.

@octalmage
Copy link
Owner

This is awesome, thanks for the PR! I'll have to think a bit about this. I like using "up" and "down", we do it in a few other places as well. Would it be possible to use "left" and "right"?

@BHamrick1
Copy link
Contributor Author

I just don't see the need for using "up" or "down". It seems odd to use both a number and a direction instead of using something like a vector.

We could add "left" and "right", but I am in the need of doing diagonal scrolling as well. I think using numbers would also make smooth scroll implementation easier.

@octalmage
Copy link
Owner

You're right that using just the numbers makes more sense. I'm fine with this, and breaking backwards compatibility is fine. Give me a bit to test this and I'll get it merged!

Thanks again!

}

int xi;
int yi;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind fixing the whitespace here?

@BHamrick1
Copy link
Contributor Author

Did I do it correctly? Not sure what you mean...

int x;
int dir = 4; /* Button 4 is up, 5 is down. */
Display *display = XGetMainDisplay();
CFRelease(event);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I meant that there are a few places where spaces are used instead of tabs. If you select the characters before CFRelease release you'll see what I mean. I can fix them after this gets merged though, if you'd like!

@AnjaP7
Copy link

AnjaP7 commented Dec 4, 2016

I'd really like to get this function. Is it merged now?

@BHamrick1
Copy link
Contributor Author

I just fixed the merge problem. Not sure what formating I need to fix. Could you link me to the style guide or the specific line number you have a problem with?

@BHamrick1
Copy link
Contributor Author

Think I fixed all the spacing.

@BHamrick1
Copy link
Contributor Author

How do I go about making sure that both parameters are numbers and then printing an error if they are not for the new scroll method?

@BHamrick1
Copy link
Contributor Author

Is the repo dead?

@octalmage
Copy link
Owner

Hi @BHamrick1, it's not dead, I'm just busy. I'll try to take a look this week.

Thanks!

@zz85
Copy link
Collaborator

zz85 commented Feb 23, 2017

@BHamrick1 thanks for the PR. I was also thinking of extending robotjs for horizontal scrolling too but I tried this and it works.

@octalmage octalmage merged commit 01f3464 into octalmage:master Feb 23, 2017
@BHamrick1
Copy link
Contributor Author

Do I need to change the documentation?

@BHamrick1
Copy link
Contributor Author

Made the change in the docs octalmage/robotjs.io#3

@zz85
Copy link
Collaborator

zz85 commented Mar 10, 2017

@octalmage just to check when will this change be pushed to npm?

@octalmage
Copy link
Owner

0.4.6 has been released to npm! Thanks again!

@andrewjaykeller
Copy link
Contributor

Why was the type script file not updated? https://github.com/octalmage/robotjs/blob/master/index.d.ts#L26

andrewjaykeller pushed a commit to andrewjaykeller/robotjs that referenced this pull request Apr 27, 2017
octalmage#194 never updated type script file
Jachobsen pushed a commit to Jachobsen/robotjs that referenced this pull request May 1, 2017
octalmage#194 never updated type script file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants