Haptics #2611
Haptics #2611
Conversation
|
@raineorshine I have managed to add the haptic changes in at each of the listed places. I have a list below of those mentioned in the ticket. The ones with Italics are completed, while the others are either not completed due to being unable to reproduce the functionality or because it is still being discussed (Like vibrations on scrolling.) @trevinhofmann If you have time to test some of these that would be great.
Changes and optimizations: |
|
Needs discussion: Q: I don't think we should add a continuous impact. We already have light impacts after each tap. Drag and Drop: Q: I am unsure about where and how this drag and drop is supposed to work. I ran into issues I linked it on the issue with a question for you @raineorshine |
Great, thanks!
Can you explain "all touches"? Is that all touchstart events, all touchend events, or both? My understanding was that we discussed adding impact(light) to
I'm not really sure what is possible with Capacitor haptics as I haven't experienced yet myself, but the idea here was to add a very tactile experience similar to the old iPod or Nest wheel where there is continuous haptic feedback that is proportional to velocity. We may have to experiment a bit to get it right. Since it is in scope for the original issue, we should at least prototype something, even if we end up deciding on a different approach. I'm open to any ideas you have based on your initial experience with the Haptics API.
Yes, unfortunately we'll have to hold off on drag-and-drop haptics as discussed until #2474 and other multicursor mobile bugs related to drag-and-drop are fixed. |
|
@raineorshine Pretty much everywhere at that point. As for the tactile feedback, Maybe we could add an impact when a user scrolls an interval distance. Therefore, the faster they scroll the faster the interval of the vibration. I don't recommend adding this until after further testing is done on how it feels to use in day-to-day. I figure that adding too many vibrations and impacts when touching and scrolling will cause the user to feel overwhelmed. If you are not worried bout this, please ignore my recommendation and I can implement the scroll feature as I just described, based on interval impacts. |
Great, sounds good. I look forward to trying it out!
Sure, I'm happy to pause that feature until I get my hands on the first iteration. Let's keep it on the table though as I have gotten a lot of interest in high tactile feedback from my beta testers, especially neurodivergent users. -- I'm noticing some merge conflicts, so I recommend rebasing on |
|
@raineorshine Looks good to go. |
|
Thanks! I will let @trevinhofmann do an initial review of code + behavior, then I will review and get a feel for how the haptics feel. |
There was a problem hiding this comment.
Thank you for the PR, @msdewitt!
Overall, the code looks very reasonable to me.
- Gesture
- start:
selectionStart- swipe:
selectionChanged- end:
selectionEnd
- except Back/Forward:
impact(light)- except Delete:
notification(warning)- except Delete permanently:
notification(warning)- cancel:
notification(warning)
- I am not noticing any feedback when starting a gesture (except, generally, when the gesture menu appears).
- I am noticing the feedback for "selection changed", but only when waiting long enough for the gesture menu to appear. If I quickly make a gesture, it does not provide feedback throughout the gesture.
- For the "Back" gesture, there is feedback at the end (and only the end).
- For the "Forward" gesture, there is feedback when the gesture menu appears and when the gesture ends.
Most of these can be grouped as "gesture feedback only enabled when the gesture menu is opened". @raineorshine -- Is that the intended functionality?
- Long tap
- start:
selectionStart
✅ This appears to be working as expected but is difficult for me to test, because I believe it conflicts with / is redundant with native haptics.
- Drag-and-drop
- start:
selectionStart- drop target changes:
selectionChanged- end:
selectionEnd- cancel:
notification(warning)
As previously discussed, drag-and-drop functionality is not working on main, and partly captured in #2474. When trying to drag a thought, it switches from drag-and-drop to starting a new gesture. When trying to drag a button on the Customize Toolbar page, it just scrolls the toolbar.
I'll skip testing this 👍
- Hamburger menu
- toggle:
impact(light)
✅ This appears to be working as expected.
- Link
- click:
impact(light)
✅ This appears to be working as expected.
- Button
- click:
impact(light)
✅ This appears to be working as expected.
Currently having some difficulty getting my Android test device connected, but I'll resolve that soon on my own time and be ready to test again next time with that device.
| if (Capacitor.isNativePlatform()) { | ||
| Haptics.notification({ type: NotificationType.Warning }) | ||
| } |
There was a problem hiding this comment.
One suggestion might be creating our own haptics utility which checks for Capacitor.isNativePlatform() internally so that we don't need to include so many new if blocks throughout the rest of the code.
e.g.:
| if (Capacitor.isNativePlatform()) { | |
| Haptics.notification({ type: NotificationType.Warning }) | |
| } | |
| haptics.notification({ type: NotificationType.Warning }) |
A reasonable counterargument is that we want to expressly show within the hooks/actions/components that the haptic event is only triggered on native.
I would be okay with leaving this as it is, but slightly prefer creating a utility.
There was a problem hiding this comment.
We definitely want to abstract this out since the condition is present with every Haptics call. It should always NOOP on other platforms.
There was a problem hiding this comment.
Thanks, that is a good observation. In general the haptics should not be connected to the gesture menu (command palette). There is a delay before the gesture menu is opened, but there should be no delay before the haptics. The haptic feedback needs to correspond directly to the user's swipes on the screen when entering a gesture. This is triggered in the Another way to think about it is that the gesture menu is visual feedback for gestures, while haptics is tactile feedback. Both need to be directly and meaningfully connected to the user's finger on the screen (MultiGesture) and/or the execution of the commands (Shortcut.exec). |
|
@raineorshine Alright, it sounds like most of the feedback is good on this first round of testing. Did you also want to get the app in your hands to test and changes? I will go back and make changes to the gesture portion now. I guess that I can move the Haptics SelectionStart and SelectionEnd portions to the MultiGesture class. Is there a faster place to detect a new selectionChanged for Gestures other than the GestureMenu? |
|
Thanks! I would like to wait till the gesture haptics timing is fixed, since that is a key part of the experience. The |
|
@raineorshine Alright, I added the changes suggested to the MultiGesture and kept the 'selectionChanged' part in the GestureMenu for now. Secondly, I added a small impact light at the start of any and all Gestures to see if the experience improves at all. @trevinhofmann I know you mentioned that you didn't feel anything at the start of most Gestures, so I wanted to see if this makes it feel better. Please give it another review when you have time. |
|
Thanks! I'll take a look asap. |
There was a problem hiding this comment.
I had a chance to try it myself, and it works great! Really exciting to have this feature in em 😁
Here are my thoughts on each of the haptics we have now:
- Gesture/start
selectionStart - Gesture/onGesture
selectionChangedimpact(light), asselectionChangeddoesn't feel strong enough. We may go impact(medium) but let's test impact(light) first. - Gesture/end
selectionEnd - Gesture/cancel
notification(warning) - - I'm not feeling this one, but it might be overlapping with the universal touchend. I'll test again after the next round. - Scroll/Continuous
impactselectionChangedLike old iPod or Nest wheel. - TheselectionChangedhaptic is very light and I think would make great scrolling feedback. Let's move forward with this. You can generateselectionChangedhaptics proportional to the velocity (i.e. every X pixels scrolled). - Long tap/start
selectionStart- This one seems to be missing. It should trigger as soon as the long tap fires, i.e. when the bullet turns the blue highlight. See useLongPress. - Drag-and-drop - HOLD
- Hamburger menu - toggle:
impact(light) ✓ - Link - click:
impact(light) ✓ - Button - click:
impact(light) - Active buttons work well. Disabled buttons should not trigger haptics. For example, if the cursor is null then the Bold button is disabled.
Tapping the blank area below the thoughts should not result in any haptics. This may be triggered by the fastClick in Content.tsx#L69. Adding a preventHaptics option to fastClick might solve this.
| if (Capacitor.isNativePlatform()) { | ||
| Haptics.notification({ type: NotificationType.Warning }) | ||
| } |
There was a problem hiding this comment.
src/util/fastClick.ts
Outdated
| if (Capacitor.isNativePlatform()) { | ||
| Haptics.impact({ style: ImpactStyle.Light }) | ||
| } |
There was a problem hiding this comment.
I believe this should go in the else statement below with tapUp, as that is where a successful click occurs. Cancelled clicks and move events are filtered out on purpose.
Awesome review. Thank you for this through review. Its very detailed. I can make those changes you requested now. |
|
@raineorshine I've finished adjusting the changes you had in code review for testing tonight. Please take a look at it this morning to see how it feels after the adjustments. Specifically, see how the toolbar at the top feels with the 'scroll' feature enabled. If you can, please give some feedback for any further adjustments. |
There was a problem hiding this comment.
Thanks! It's looking good.
Here's are the changes I think should happen.
Gestures
Gestures are better now. I'm usually getting one haptic per swipe.
I'm still getting two impacts sometimes. Try slowly activating Back (→) and Forward (←) between a, b, c:
- a
- b
- c
Scroll
This is a good start! I can see a couple things in need of change to get a more satisfying feel.
1. Rate
To create the most realistic feedback, the rate of haptic feedback should ideally be proportional to the velocity of the scroll, i.e. the number of haptics should be proportional to the number of pixels scrolled. However, the scroll event does not fire every pixel when the user scrolls quickly. This means that currently we're getting the opposite of what we want: the rate of haptic feedback is inversely proportional to the scroll velocity.
So basically you have to measure how many pixels have been scrolled each scroll event (and probably since the start of the scroll) and determine whether haptics should fire or not. I would aim for about one haptic every 10 pixels.
We could also try firing multiple haptics when a large scroll is detected in a single event, though that might create a weird delay.
Note: we are limited to 32 Hz (31.25 haptics per second) so we should keep that in mind. Additional haptics above that are dropped, so I don't think we have to do anything special with this rate limiting though unless it affects performance.
2. Touchmove
Since haptics parallel touch events to create a tactile experience, it's clear that we don't want them to fire when the user's finger it not on the screen. So just firing haptics on scroll is a problem because of momentum scrolling (when it keeps scrolling after the user has released their finger from the screen).
It's not pretty, but we actually have a global touching variable that you should be able to easily check:
Lines 9 to 11 in efcc745
Long press
This one is still missing. It should trigger as soon as the long tap fires, i.e. when the bullet turns the blue highlight.
Button
I'm still getting a false positive on disabled toolbar buttons:
- Set the cursor to null.
- Tap
boldtoolbar button.
|
|
||
| isHaptics: boolean = false, | ||
|
|
There was a problem hiding this comment.
The existing fastClick interface is not very conducive to adding new options unfortunately. Sticking isHaptics in between tapUp and tapDown is not the most intuitive.
I can think of two approaches that would be a bit cleaner:
- Convert all of the options to named parameters.
- Leave the existing parameters as-is and add
isHapticsto a new options object at the end:
..., { isHaptics?: boolean } = {})| @@ -41,6 +42,7 @@ const useLongPress = ( | |||
| clientCoords.current = { x: e.touches?.[0]?.clientX, y: e.touches?.[0]?.clientY } | |||
| } | |||
| onTouchStart?.() | |||
| Haptics.selectionStart() | |||
There was a problem hiding this comment.
This should go next to onLongPressStart, where the long press actually starts.
start only starts the timer.
So I have some news then regarding haptics and the behavior you want for quick consecutive triggers of haptics, right now it is already firing the haptics response as fast as possible and it might not be built to handle what you want with the fast triggers based on scrolling velocity. Right now, it is set to trigger every 15 pixels in the code. But because each Haptics vibration needs to contact the native device, it runs in an async thread. When we try and trigger them many times consecutively, there is a buffer delay that cannot be overcome when translating to the native code and back along with the asyn behavior of many threads spawning. @raineorshine If you have ideas to overcome this it would be great. |
Yes, that makes sense. I'm actually not suggesting we try to speed it up. I was thinking we could slow down the haptics when the user only scrolls a tiny amount. Then, relatively speaking, we would get a more proportional haptics rate.
Can you link to the line of code for this? I can't seem to find it. Also just to confirm, you have only added scroll haptics to the Toolbar so far, right?
Yes, I understand that is a constraint we have to work with. Can scrolling the body be detected in the Capacitor plugin? If so, we could possibly control the scroll haptics from the WebView whether there is no latency from the js bridge. |
| @@ -137,6 +138,7 @@ const Toolbar: FC<ToolbarProps> = ({ customize, onSelect, selected }) => { | |||
|
|
|||
| if (scrollDifference >= 5) { | |||
There was a problem hiding this comment.
Sorry the difference isn't 15px but 5 pixels right now.
|
@raineorshine Yes, its only added to the toolbar right now, just to work out these issues. Adding these changes to the IOS Webview scroll bar will be even more troublesome since it's not in the javascript code. |
Great, just wanted to make sure I was looking at the same commit as you.
I suggest that we consider all our options, estimate the work effort, and then determine what is in scope for the current contract. There is far more harm in dismissing a possible course of action prematurely. I'm sure that between the two of us we can make a good judgment about the best way to solve this. I respect your time and I'm not going to make you do anything you don't want to do.
Got it, I see. It will only trigger the haptics when a scroll event has exceeded 5px. Stepping back from the implementation, I want to make sure that you understand and are able to reproduce the behavior I am describing. Try scrolling very slowly, then medium speed, then very quickly. When scrolling very slowly, the haptics fire very frequently. When scrolling at medium speed, the haptics fire less frequently. And when scrolling very quickly, the haptics don't fire very much at all. Due to the latency I understand that there may not be anything we can do about the high rate of scroll. Therefore, let's focus on the slow scroll. At the moment there are too many haptics. If we can bring that rate down, I think it will feel a lot more natural. While I'm not able to guide the exact implementation, I am happy to further clarify the behavior. I bet with a little experimentation you can get it closer to a more proportional and natural feeling rate of scroll, at least at the slower speeds. |
|
@raineorshine Gotcha, while I was testing here I was able to see each trigger happen more frequently when scrolling faster, but the device itself might not be able to handle so many vibrations and the native components might prevent it, causing the faster speeds to seem like there are less impacts. And thank you for respecting my time. We can talk more in Upwork chat about that when needed. |
Yes, that could be. Let's focus on the slower scrolling for now. I don't think the 5px threshold is working the way you are expecting. Probably because |
There was a problem hiding this comment.
Secondly, I added a small impact light at the start of any and all Gestures to see if the experience improves at all. @trevinhofmann I know you mentioned that you didn't feel anything at the start of most Gestures, so I wanted to see if this makes it feel better.
I see additional changes have been made since, but still wanted to confirm my current experience with gestures on the latest commit (aa23971).
The haptics for gestures are no longer dependent on the gesture menu's appearance, and I am usually observing the haptic feedback when starting a gesture now.
I have also noticed what @raineorshine pointed out:
I'm still getting two impacts sometimes. Try slowly activating Back (→) and Forward (←) between a, b, c:
There are usually two rather than one whenever making the gesture slowly.
I also noticed that there is feedback for every direction change when gesturing, including when cancelling the gesture. For example, if you just draw multiple circles for the gesture, the haptic feedback will continue (4 per circle) even after recognizing that the gesture is cancelled.
That may be the intended/desired functionality, but #2392 only mentions "cancel: notification (warning)", so I would probably expect the haptic notification feedback only once when the gesture is cancelled. @raineorshine may be able to clarify what is expected.
|
Good point. Subsequent swipes after a gesture has been cancelled should not trigger haptics. |
This PR is one which will change as we test to further perfect the acceptance criteria mentioned in issue-2392.