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

Trigger long-press as soon as the button is held long enough, instead… #65

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

Conversation

float32
Copy link

@float32 float32 commented Oct 18, 2017

… of at release

Hopefully this doesn't have any side effects I haven't thought of. I don't know the codebase well enough to say for sure.

Line 85 is to stop the button being flagged to ignore by Ui::AppSettings(). Kinda hacky.

The +7 in line 89 is compensation for the 7-bit button debouncing.

@timchurches
Copy link
Collaborator

I haven't looked at the pull request code, but functionally, is this an improvement. By actioning the long-press when the button is released, the action can be triggered at a precise moment (eg on-the-beat), whereas triggering after minimum long-press time has elapsed makes it very hard to manually synchronise the action with other events.

@patrickdowling
Copy link
Collaborator

Off the top of my head I suspect this might break at least one thing, the hold + turn in the scale editor. I vaguely remember thinking about handling all button presses this way when the UI code was re-written, but suspect I didn't to maintain existing behaviours (or some other forgotten reason).

@patrickdowling
Copy link
Collaborator

So looking at the source seems to confirm that generating the long-press this way would conflict with the scale editor: The L long press is used, and also there's a case where you hold L pressed and turn R where the long press (at release) gets ignored. It might still work if you manage to turn R within kLongPressTicks but that seems a bit too fiddly... Not sure if there's a way to resolve that, at least nothing comes to mind spontaneously.

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.

3 participants