Fix - Reduce the haptic feedback vibration to a short bip #5960

rushatgabhane · 2021-10-20T12:16:17Z

cc: @Jag96 Creating a draft PR. Looking forward for your feedback!

Details

This PR removes react-native-unimodules and expo-haptics as they aren't required anymore.

Now that my PR is merged into react-native-haptic-feedback, we can use it directly instead of Expensify's fork.

Use the lib junina-de/react-native-haptic-feedback for haptic feedback.

Android

Create a new file: components/PressableWithSecondaryInteraction/index.android.js
Call ReactNativeHapticFeedback.trigger('effectHeavyClick'); for a long press.

iOS

Create a new file: components/PressableWithSecondaryInteraction/index.ios.js
Call ReactNativeHapticFeedback.trigger('selection'); for a long press.

Fixed Issues

$ #4355

Tests / QA

Navigate to a conversation on mobile (iOS, Android).
Long press on a message.
Verify that the haptic feeback is same as slack's long press on a message.

Tested On

iOS
Android

Screenshots

Not Applicable

This reverts commit fee4bec.

Jag96 · 2021-10-21T17:54:29Z

@rushatgabhane we've fixed the e2e tests on main, please merge main into your branch to confirm they pass again!

rushatgabhane · 2021-10-21T23:01:45Z

All checks have passed!

parasharrajat · 2021-10-22T23:21:59Z

I believe that we should be good to drop react-native-unimodules modules and expo-haptic packages.

rushatgabhane · 2021-10-22T23:37:17Z

Yup, from Rory's comment #4355 (comment)
dropping unimodules and expo-haptic would be the next step.

If this draft PR is given a go ahead, I'll remove those libs as cleanup commits.

Jag96 · 2021-10-26T01:06:04Z

cc @Julesssss @Beamanator since they volunteered to test this previously, could you confirm the feedback pattern is the same as slack on your physical Android devices on this PR?

Julesssss · 2021-10-26T10:15:51Z

src/components/PressableWithSecondaryInteraction/index.ios.js

+        ref={props.forwardedRef}
+        onPress={props.onPress}
+        onPressIn={props.onPressIn}
+        delayLongPress={200}


Why not use the default value of 500? If this is to precisely match Slack, then does Slack not match the native delay length on iOS?

Didn't notice it.
Changing long press delay to 500ms for iOS.

This was intentional. Menu itself takes 300 to show so adding 500 here means 800ms delay before menu is completely visible and interactive.

That's a reasonable choice, but I would prefer to keep longPress time at the iOS default. If the menu is slow to appear we should optimise that instead.

Julesssss

I'm getting a crash on Android 8.1:

Error when loading lib: dlopen failed: library "libjsc.so" not found lib hash: eae0e884bac6a338b8c53618b45e2515 search path is /data/app/com.expensify.chat-4zdOjJAUwAHOSjq0TXDQ8Q==/lib/arm64
2021-10-26 11:36:08.075 16636-16636/com.expensify.chat W/System.err: java.lang.UnsatisfiedLinkError: dlopen failed: library "libjsc.so" not found
2021-10-26 11:36:08.076 16636-16636/com.expensify.chat W/System.err:     at com.facebook.soloader.SoLoader$1.load(SoLoader.java:384)
2021-10-26 11:36:08.077 16636-16636/com.expensify.chat W/System.err:     at com.facebook.soloader.DirectorySoSource.loadLibraryFrom(DirectorySoSource.java:77)
2021-10-26 11:36:08.077 16636-16636/com.expensify.chat W/System.err:     at com.facebook.soloader.DirectorySoSource.loadLibrary(DirectorySoSource.java:50)
2021-10-26 11:36:08.077 16636-16636/com.expensify.chat W/System.err:     at com.facebook.soloader.ApplicationSoSource.loadLibrary(ApplicationSoSource.java:89)

rushatgabhane · 2021-10-26T15:58:10Z

@Julesssss Thanks for testing it out!
I've fixed the crash on versions below Android 10.

Tested it on Samsung S8 (android 9), Pixel 3 (android 11), and the haptics match Slack.

Also tested on android 5, 6, 7, 8 on emulator to check for crashes.

Jag96

Tested on my physical iPhone and the 500ms matches slack exactly 👍

src/components/PressableWithSecondaryInteraction/index.ios.js

Julesssss

Tests well on Android, the vibration pattern matches slack.

I requested one minor change, but once that's done I think we're good to merge 👍

Julesssss · 2021-10-28T10:38:10Z

@rushatgabhane is there any reason for this to still be a draft?

rushatgabhane · 2021-10-28T11:10:02Z

@Julesssss from this comment, I think we're good to remove react-native-unimodules and expo-haptics.

Should I remove it in this PR, or create a new one?

src/components/PressableWithSecondaryInteraction/index.android.js

src/components/PressableWithSecondaryInteraction/index.ios.js

Julesssss

Nnoticed a few small things to change

Julesssss · 2021-10-28T11:30:14Z

@rushatgabhane nice spot, it would be great if you remove it in this PR so we can run the vibration tests against it.

rushatgabhane · 2021-10-28T13:43:25Z

Status: PR ready for review.

ToDo

remove iOS config for react-native-unimodules

src/components/PressableWithSecondaryInteraction/index.android.js

Julesssss

Confirmed the vibration pattern matches on Android.

Jag96

LGTM, great work on this!

OSBotify · 2021-10-29T18:06:37Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-10-29T23:28:27Z

🚀 Deployed to staging by @Jag96 in version: 1.1.11-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2021-11-02T23:36:57Z

🚀 Deployed to production by @yuwenmemon in version: 1.1.12-3 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

rushatgabhane added 2 commits October 20, 2021 15:02

install react-native-haptic-feedback

fee4bec

add android, ios haptic on long press

4b24b6a

This comment has been minimized.

Sign in to view

rushatgabhane added 2 commits October 21, 2021 02:59

Revert "install react-native-haptic-feedback"

7e13b36

This reverts commit fee4bec.

install react-native-haptic-feedback

ed0d3ef

Merge branch 'Expensify:main' into rushat-fix-haptic

75ae12c

Julesssss reviewed Oct 26, 2021

View reviewed changes

change long press delay to 500 ms for iOS

3d2b094

Julesssss requested changes Oct 26, 2021

View reviewed changes

rushatgabhane added 2 commits October 26, 2021 18:46

fix crash below android 10

5d6375a

early return

555097e

Jag96 reviewed Oct 27, 2021

View reviewed changes

Julesssss reviewed Oct 28, 2021

View reviewed changes

src/components/PressableWithSecondaryInteraction/index.ios.js Outdated Show resolved Hide resolved

Julesssss approved these changes Oct 28, 2021

View reviewed changes

rm prop as it's default

defe7bd

Julesssss reviewed Oct 28, 2021

View reviewed changes

src/components/PressableWithSecondaryInteraction/index.android.js Show resolved Hide resolved

Julesssss reviewed Oct 28, 2021

View reviewed changes

src/components/PressableWithSecondaryInteraction/index.android.js Outdated Show resolved Hide resolved

Julesssss reviewed Oct 28, 2021

View reviewed changes

src/components/PressableWithSecondaryInteraction/index.ios.js Show resolved Hide resolved

Julesssss requested changes Oct 28, 2021

View reviewed changes

rushatgabhane added 2 commits October 28, 2021 14:30

rm delay time, use the default value for android

5b8cadc

add comment explaining android version

0129d55

rushatgabhane added 5 commits October 28, 2021 15:24

uninstall expo-haptics

b37a84a

eslint

d70bd3d

npm uninstall react-native-unimodules

b245186

undo unimodules android config

ad9de2a

fix android config

1709c5d

revert ios config for unimodules

1bb3d17

rushatgabhane marked this pull request as ready for review October 29, 2021 00:49

rushatgabhane requested a review from a team as a code owner October 29, 2021 00:49

botify requested a review from Jag96 October 29, 2021 00:49

MelvinBot removed the request for review from a team October 29, 2021 00:49

rushatgabhane requested a review from Julesssss October 29, 2021 00:52

Julesssss reviewed Oct 29, 2021

View reviewed changes

src/components/PressableWithSecondaryInteraction/index.android.js Outdated Show resolved Hide resolved

fix docs param type

31624cb

Julesssss approved these changes Oct 29, 2021

View reviewed changes

Jag96 approved these changes Oct 29, 2021

View reviewed changes

Jag96 merged commit 17aab27 into Expensify:main Oct 29, 2021

OSBotify mentioned this pull request Oct 29, 2021

Deploy Checklist: New Expensify 2021-10-29 #6127

Closed

23 tasks

botify mentioned this pull request Nov 2, 2021

[HOLD for payment 2021-11-09] Reduce the haptic feedback vibration to a short bip #4355

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix - Reduce the haptic feedback vibration to a short bip #5960

Fix - Reduce the haptic feedback vibration to a short bip #5960

rushatgabhane commented Oct 20, 2021 •

edited

Loading

This comment has been minimized.

Jag96 commented Oct 21, 2021

rushatgabhane commented Oct 21, 2021

parasharrajat commented Oct 22, 2021

rushatgabhane commented Oct 22, 2021 •

edited

Loading

Jag96 commented Oct 26, 2021

Julesssss Oct 26, 2021 •

edited

Loading

rushatgabhane Oct 26, 2021

parasharrajat Oct 27, 2021

Julesssss Oct 28, 2021

Julesssss left a comment •

edited

Loading

rushatgabhane commented Oct 26, 2021 •

edited

Loading

Jag96 left a comment

Julesssss left a comment

Julesssss commented Oct 28, 2021

rushatgabhane commented Oct 28, 2021

Julesssss left a comment

Julesssss commented Oct 28, 2021

rushatgabhane commented Oct 28, 2021 •

edited

Loading

Julesssss left a comment

Jag96 left a comment

OSBotify commented Oct 29, 2021

OSBotify commented Oct 29, 2021

OSBotify commented Nov 2, 2021

Fix - Reduce the haptic feedback vibration to a short bip #5960

Fix - Reduce the haptic feedback vibration to a short bip #5960

Conversation

rushatgabhane commented Oct 20, 2021 • edited Loading

Details

Android

iOS

Fixed Issues

Tests / QA

Tested On

Screenshots

This comment has been minimized.

Jag96 commented Oct 21, 2021

rushatgabhane commented Oct 21, 2021

parasharrajat commented Oct 22, 2021

rushatgabhane commented Oct 22, 2021 • edited Loading

Jag96 commented Oct 26, 2021

Julesssss Oct 26, 2021 • edited Loading

Choose a reason for hiding this comment

rushatgabhane Oct 26, 2021

Choose a reason for hiding this comment

parasharrajat Oct 27, 2021

Choose a reason for hiding this comment

Julesssss Oct 28, 2021

Choose a reason for hiding this comment

Julesssss left a comment • edited Loading

Choose a reason for hiding this comment

rushatgabhane commented Oct 26, 2021 • edited Loading

Jag96 left a comment

Choose a reason for hiding this comment

Julesssss left a comment

Choose a reason for hiding this comment

Julesssss commented Oct 28, 2021

rushatgabhane commented Oct 28, 2021

Julesssss left a comment

Choose a reason for hiding this comment

Julesssss commented Oct 28, 2021

rushatgabhane commented Oct 28, 2021 • edited Loading

Julesssss left a comment

Choose a reason for hiding this comment

Jag96 left a comment

Choose a reason for hiding this comment

OSBotify commented Oct 29, 2021

OSBotify commented Oct 29, 2021

OSBotify commented Nov 2, 2021

rushatgabhane commented Oct 20, 2021 •

edited

Loading

rushatgabhane commented Oct 22, 2021 •

edited

Loading

Julesssss Oct 26, 2021 •

edited

Loading

Julesssss left a comment •

edited

Loading

rushatgabhane commented Oct 26, 2021 •

edited

Loading

rushatgabhane commented Oct 28, 2021 •

edited

Loading