Horizontal scroll and additional mouse buttons #242
Conversation
src/lib/barrier/mouse_types.h
Outdated
|
|
||
| static const ButtonID kMacButtonRight = 2; | ||
| static const ButtonID kMacButtonMiddle = 3; | ||
| //@} | ||
|
|
||
| static const UInt8 NumButtonIDs = 5; | ||
| // XXX -- some osx stuff uses this and might expect it to be 5 |
There was a problem hiding this comment.
Can we figure out whether this the case or not and explain? For now this seems to have high regression potential.
|
I've tested on OSX now and it doesn't work. I don't know the codebase nor the required APIs so porting @yupi2's patch is all I can contribute. |
|
I have tested this PR with a Debian 9 server and MacOS Mojave 10.14.2 client. The extra mouse buttons are working, but I have not tested horizontal scrolling. |
|
Thanks! :D |
|
An update on Mojave testing. The original test was performed using a game (League of Legends), which does recognize the button input. However, testing with a browser produces no result for the back/forward buttons. On Windows 10 the inverse is true. The browser back/forwards works, but the input is not recognized in a game. Edit: I tested the behavior with a mouse physically plugged into the machine running MacOS Mojave and the behavior appears the same. Safari does not support the forward and back buttons and FireFox on MacOS brings up the scroll indicator. |
|
NumButtonIDs being 6 shouldn't cause any issues, as for handling the button events it seems it's a client OS issue since the behavior is the same when plugging the device in directly. |
|
@vorph1 You still interested in this PR? |
|
Sure, I'd like to see this merged. Do you have any other suggestions since as @rolandoislas said the behavior seems to be the same with barrier and mouse plugged in directly? |
|
Actually, looking back.
I feel like if I were to merge this, people might complain about having a "half-baked" experience. |
|
I'd argue that given the amount of time the development team has, we should only merge well tested code that don't introduce regressions. It's a pity we don't have unit tests which could ensure a level of quality. |
|
Forward/Back working fine for me with SensibleSideButtons. Is this needed within this PR? I'm looking for horizontal scroll support, can the two PRs be split? |
|
I think I'll close this PR due to inactivity. If anyone wants me to re-open it, I'll re-open it under a new PR so it will be built against the new CI system, and just reference this closed PR in the new PR. |
|
I think it would be really nice to get this in. Would it be better to use this PR, or form a new one? |
|
I'd say new one, so it can be ran against the new CI. |
This re-implements the original patch from debauchee#242 on the more up to date codebase.
ghost
mentioned this pull request
Patch talked about in issue #51. Going to mention @vorph1, as I don't know whether they consider this change finished or not.