Skip to content

Feature: vibrate for repeated inputs #381#2355

Open
glen-nicol wants to merge 3 commits intoHeliBorg:mainfrom
glen-nicol:vibrate_on_delete_hold
Open

Feature: vibrate for repeated inputs #381#2355
glen-nicol wants to merge 3 commits intoHeliBorg:mainfrom
glen-nicol:vibrate_on_delete_hold

Conversation

@glen-nicol
Copy link
Copy Markdown

This is primarily noticeable when holding the delete key to delete multiple characters or when using the keyboard to move the cursor.

This implements the desired behavior of #381. It does not implement the vibrate on paste behavior mentioned further down the issue.

The change is remarkably simple. Just perform haptics on repeat key presses. It works great for delete and swipe at low and high vibration durations on my test phone (Pixel 3a). And holding other keys doesn't trigger a repeat since they have a long press handler.

This is primarily noticeable when holding the delete key to delete multiple characters.
@Helium314
Copy link
Copy Markdown
Collaborator

@devycarol you did some work on haptic feedback, and iirc have plans to do more. Do you see any issues with this change?

@devycarol
Copy link
Copy Markdown
Contributor

See the HapticEvent enum I wrote.

I haven't thought much of it since then, but the idea is "as that TODO comment says, we should consider using a lighter vibration for repeat haptics."

Really the default "key press" vibration seems inoffensive to me, so I would just say to remove that commented line in the linked file before merging.

@glen-nicol
Copy link
Copy Markdown
Author

@devycarol was this the line you meant?

//    KEY_REPEAT(HapticFeedbackConstants.?, ?),

@devycarol
Copy link
Copy Markdown
Contributor

Yes.

@devycarol
Copy link
Copy Markdown
Contributor

OH—I remember why this is problematic.

For custom vibration durations that are long, this would be bad. So that enum constant should indeed be used, with "custom duration allowed" set to false.

…vent that is not affected by custom duration
@glen-nicol
Copy link
Copy Markdown
Author

Got it, I think I have hooked that up correctly. I didn't see any other obvious location where the KEY_PRESS event was used that needed repeat handling.

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