-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add mouse button bindings #792
Conversation
|
7e325b0
to
716bf25
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #792 +/- ##
==========================================
- Coverage 41.74% 41.60% -0.14%
==========================================
Files 108 108
Lines 6998 7020 +22
==========================================
Hits 2921 2921
- Misses 4077 4099 +22 ☔ View full report in Codecov by Sentry. |
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.
Thanks for working on this @tomas7770! LGTM, just needs some changes to the input sample's page before being merged :)
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.
Looks great! Ty! Just one thing before merging: some of your commit messages don't follow the format - you can use git rebase -i HEAD~7
to edit your previous 7 commits, and change each pick
to reword
(or just r
) to change their messages. Most of your commits would fall under the feat(input)
scope - for formatting one, you should use style: fix formatting
. The commit you have which touches both sides, I would use feat(window)
since its mostly window related code that's being changed. Usually when we can't choose a scope we try to split the commits into multiple ones or just omit the scope entirely.
Also, some tips for future PRs:
- you can press the 'auto merge' button to merge this automatically when all conversations are resolved and when the PR is approved.
- feel free to press the resolve button on reviews after pushing fixes for them.
Something like this?
|
LGTM! |
a4a4f31
to
4a17331
Compare
Implement mouse buttons without support for binding them through assets
Mouse buttons can be binded through assets
so it doesn't interfere with mouse showcase
4a17331
to
dcc5cc0
Compare
Description
Adds support for binding mouse buttons to input actions, similar to existing implementation of keyboard/gamepad. Serialization functions are also implemented in order to support adding bindings in an asset.
Adds a new showcase to the input sample to test the bindings.
Checklist