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

Revert latest PR & Add "Invert Direction" setting in the extension settings. #10

Merged
merged 9 commits into from
Apr 1, 2024

Conversation

salihefee
Copy link

I apologize again for the latest PR. This reverts the changes made in the latest PR and adds an option to invert the direction.

[email protected]/extension.js Outdated Show resolved Hide resolved
[email protected]/extension.js Outdated Show resolved Hide resolved
salihefee and others added 2 commits April 1, 2024 20:06
@francislavoie
Copy link
Owner

I think this'll still invert for both input methods since it doesn't do any input detection on the scroll event. So users might need to toggle the setting by hand if they plug in a mouse or w/e. Not perfect, but probably good enough for 99.9% of users 😅

@salihefee
Copy link
Author

Yes it will invert for both methods but I don't think there is an easy way of getting the source device, so this is the best I can do. Yes, it is not perfect, but at least now users have an option to invert whereas before, they would have to edit the extension source everytime they change the device :)

@francislavoie
Copy link
Owner

francislavoie commented Apr 1, 2024

FYI, for next time, you shouldn't commit on your main branch because it causes weird conflicts. You should instead make a new branch on your own repo based on upstream's main and then make your commits on that. Your main is behind my main so this PR has repeated commits which isn't ideal. But it's fine, I can resolve it 👍

Also, best practice is to put a reference to the relevant issue in the PR description (i.e. #7) to link them together

@francislavoie francislavoie merged commit d789e3e into francislavoie:main Apr 1, 2024
@salihefee
Copy link
Author

I'm sorry, these two are my first pull requests and I am not experienced with git + development in general. I will keep that in mind the next time I create a PR. Thank you.

@francislavoie
Copy link
Owner

No need to be sorry :) you're doing great!

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