Haptic feedback on long press, Fixes: #4649 #4688
Conversation
This doesn't do that, right? We should consider using a library like react-native-haptic-feedback instead of the expo thing, since that actually supports the long-press specific haptic feedback API. |
should we favour this "if Haptic is disabled in the Android system settings, allow ignoring the setting and trigger haptic feedback"? |
vaibhavs2
force-pushed
the
haptic-feedback
branch
from
1cfed5b to
80dddfa
Compare
| android: 'longPress', | ||
| }); | ||
| ReactNativeHapticFeedback.trigger(hapticTriggerType, { | ||
| ignoreAndroidSystemSettings: true, |
There was a problem hiding this comment.
We shouldn't ignore the system settings here.
There was a problem hiding this comment.
should we favour this "if Haptic is disabled in the Android system settings, allow ignoring the setting and trigger haptic feedback"?
I asked about that before :)
well, I will remove that, should i remove and push -f or wait for the request that we did on upstream?
There was a problem hiding this comment.
Go ahead and push with the fix. I don't see a reason to wait on upstream, since the helper function that you wrote gets around the problem.
| */ | ||
| export const longPressHapticFeedback = () => { | ||
| const hapticTriggerType = Platform.select({ | ||
| ios: 'selection', |
There was a problem hiding this comment.
Just FYI, this is the default that react-native-haptic-feedback will do on iOS if we do longPress — you can see that in the upstream code.
I think it's still reasonable to have this, just so if that changes, we can change this ourselves, but I don't think we really need this, strictly speaking.
There was a problem hiding this comment.
Look at the next line upstream code there it do not have default for supportsHapticFor6SAnd6SPlus, that's why I explicitly provide the selection type
if we intend not to provide haptics in Iphone 6 then this would definetly of one line implementation.
As this module providing us that feature then Its good to implement
There was a problem hiding this comment.
Ah, cool, that makes sense. Looking at this table, it seems like the 6s/6s Plus support iOS 14, which means we still support them.
It's probably worth filing a upstream bug about that, I'll go do that now.
There was a problem hiding this comment.
Looks like this is mkuczera/react-native-haptic-feedback#47
|
I posted some questions about this in the chat thread. |
Fixes: #4649