feat: updated camera to react-native-vision 3 #1046

wadeking98 · 2023-12-12T23:33:50Z

Summary of Changes

Replaced the depricated RNCamera module with react-native-vision-camera. Updated tests to be compatible. I had to make some modifications to the android permissions text... but it looks like they weren't being used anyway for some reason. This should be a transparent change, I've tested in the android emulator API version 30 and 32, as well as an iphone physical device. I was running into an issue on API version 30, but I wasn't sure if that was just a problem with the emulator itself. Other than android API 30 it works great on all versions. I'd appreciate some help testing the changes on different versions especially on android.

Additionally I've made sure the torch functionality works well.

Other advantages of this library:

It provides support for non QR code barcodes out of the box so we can provide more specific error messages if the user tries to scan their driver's license, etc.

Related Issues

Fixes bcgov/bc-wallet-mobile#1653

Pull Request Checklist

Tick all boxes below to demonstrate that you have completed the respective task. If the item does not apply to your this PR check it anyway to make it apparent that there's nothing to do.

All commits contain a DCO Signed-off-by line (we use the DCO GitHub app to enforce this);
Updated LICENSE-3RD-PARTY.md for any added dependencies or vendored components;
Updated documentation as needed for changed code and new or modified features;
Added sufficient tests so that overall code coverage is not reduced.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

Pro Tip 🤓

Read our contribution guide at least once; it will save you a few review cycles!
Your PR will likely not be reviewed until all the above boxes are checked and all automated tests have passed.

PR template adapted from the Python attrs project.

Signed-off-by: wadeking98 <[email protected]>

codecov · 2023-12-12T23:37:34Z

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (73a2246) 61.77% compared to head (5f7f49b) 62.09%.

Files	Patch %	Lines
...ges/legacy/core/App/components/misc/ScanCamera.tsx	50.00%	14 Missing ⚠️
packages/legacy/core/App/navigators/TabStack.tsx	0.00%	7 Missing ⚠️
...ages/legacy/core/App/components/misc/QRScanner.tsx	55.55%	3 Missing and 1 partial ⚠️
packages/legacy/core/App/screens/Scan.tsx	55.55%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1046      +/-   ##
==========================================
+ Coverage   61.77%   62.09%   +0.32%     
==========================================
  Files         170      171       +1     
  Lines        5467     5466       -1     
  Branches     1574     1571       -3     
==========================================
+ Hits         3377     3394      +17     
+ Misses       2066     2048      -18     
  Partials       24       24

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bryce-mcmath · 2023-12-12T23:43:10Z

What was the issue on API 30? Just so we can look out for it

bryce-mcmath · 2023-12-12T23:45:40Z

Looks like you may need to update the gradle lockfile in Bifold too

wadeking98 · 2023-12-13T00:06:41Z

What was the issue on API 30? Just so we can look out for it

I'd get a repetitive error saying failed to load depricated vision dynamite module

Signed-off-by: wadeking98 <[email protected]>

packages/legacy/app/package.json

packages/legacy/core/package.json

jleach · 2023-12-13T00:40:38Z

@wadeking98 We go back pretty far for Android API compatibility - no? Do you think we'll loose anything with this change?

Signed-off-by: wadeking98 <[email protected]>

wadeking98 · 2023-12-13T22:08:24Z

I've fixed the issue for api level 29 but I'm still running into issues with API level 30: mrousavy/react-native-vision-camera#2287

Signed-off-by: wadeking98 <[email protected]>

…ent-react-native into feat-update-camera

Signed-off-by: wadeking98 <[email protected]>

wadeking98 · 2023-12-23T00:16:27Z

I've confirmed this is working with API level 29, 30, 31 now

bryce-mcmath · 2023-12-27T21:02:16Z

packages/legacy/core/App/constants.ts

+export const SCREEN_WIDTH = Dimensions.get('window').width
+export const SCREEN_HEIGHT = Platform.select<number>({
+  android: Dimensions.get('screen').height - StaticSafeAreaInsets.safeAreaInsetsBottom,
+  ios: Dimensions.get('window').height,
+}) as number
+


How does this behave on tablets when they switch between portrait and landscape on the camera screen?

good question, BC Wallet is locked in portrait mode so it shouldn't be an issue

I don't think it's locked in portrait mode on tablets, only on phones

I tested it on the pixel tablet with auto rotate turned on and the BC Wallet app was locked in portrait mode on that device

Hmm. Maybe it's just unlocked on iPads. We should ask @jleach

Seems like this is a known issue with the library:

🐛 [iPadOS] If app is set to landscape only, vision camera gets preview orientation wrong mrousavy/react-native-vision-camera#1455

✨ Implement Orientation ($3,000) mrousavy/react-native-vision-camera#1891

I could downgrade to version 2 or switch to another library but that may introduce it's own dependency issues and IMHO that seems like a lot of work to address what would be a minor inconvenience to only a handful of users, especially when it looks like this issue is going to be fixed in the library eventually. What are your thoughts @bryce-mcmath @jleach ?

I would tend to agree with that but what happened with testing older Android versions that we still support? I remember you saying something like API 24 wasn't scanning

Actually I just tested it again on my mac environment and it worked fine on API 24. I think maybe I just didn't try moving the camera around last time

Well then I'm ok to move forward with this. Might want to wait until Clecio gets back and takes a look at it but I think you've done a great job

Signed-off-by: wadeking98 <[email protected]>

…ent-react-native into feat-update-camera

Signed-off-by: wadeking98 <[email protected]>

jleach · 2024-01-16T21:58:14Z

packages/legacy/app/App.tsx

@@ -42,6 +44,10 @@ const App = () => {
    SplashScreen.hide()
  }, [])

+  if (!isTablet()) {
+    Orientation.lockToPortrait()


I think we want to support landscape but just lock the camera screen to portrait.

I thought we decided against landscape mode in mobile view?

jleach · 2024-01-16T22:00:11Z

packages/legacy/app/App.tsx

@@ -42,6 +44,10 @@ const App = () => {
    SplashScreen.hide()
  }, [])

+  if (!isTablet()) {


There are some helper functions that use the screen dimensions for this. I think its better if we don't use both. If not in this PR can you create a technical debt ticket to clean this up at a later point?

packages/legacy/core/App/constants.ts

Signed-off-by: wadeking98 <[email protected]>

bryce-mcmath

Can we only use the official package isTablet function and get rid of our helper version of it?

Signed-off-by: wadeking98 <[email protected]>

packages/legacy/core/App/components/misc/ScanCamera.tsx

Signed-off-by: wadeking98 <[email protected]>

wadeking98 requested a review from a team as a code owner December 12, 2023 23:33

wadeking98 added 2 commits December 12, 2023 15:35

updated to rn vision camera

7db948c

Signed-off-by: wadeking98 <[email protected]>

updated tests and linting

456d7f7

Signed-off-by: wadeking98 <[email protected]>

wadeking98 force-pushed the feat-update-camera branch from c3c470f to 456d7f7 Compare December 12, 2023 23:35

wadeking98 added 2 commits December 12, 2023 16:17

added dependancy to app package

4a43cb9

Signed-off-by: wadeking98 <[email protected]>

updated grandle lock

1111a02

Signed-off-by: wadeking98 <[email protected]>

jleach requested changes Dec 13, 2023

View reviewed changes

packages/legacy/app/package.json Show resolved Hide resolved

packages/legacy/core/package.json Show resolved Hide resolved

wadeking98 added 2 commits December 13, 2023 09:53

removed old cam module from deps

f2e38f2

Signed-off-by: wadeking98 <[email protected]>

updated gradle lock

37796e4

Signed-off-by: wadeking98 <[email protected]>

wadeking98 requested a review from jleach December 13, 2023 19:57

wadeking98 added 3 commits December 13, 2023 14:10

fixed error on api 29 android

c2856eb

Signed-off-by: wadeking98 <[email protected]>

updated tests

0d9a4b8

Signed-off-by: wadeking98 <[email protected]>

fixed inting

24b2ca3

Signed-off-by: wadeking98 <[email protected]>

jleach marked this pull request as draft December 14, 2023 00:15

jleach added the do not merge Don't merge when label present label Dec 19, 2023

wadeking98 added 3 commits December 22, 2023 12:59

Merge branch 'main' of https://github.com/hyperledger/aries-mobile-ag…

03cddec

…ent-react-native into feat-update-camera

updated gradle dependency for camera

413f11a

Signed-off-by: wadeking98 <[email protected]>

added gradle lock

db96515

Signed-off-by: wadeking98 <[email protected]>

wadeking98 marked this pull request as ready for review December 23, 2023 00:15

bryce-mcmath reviewed Dec 27, 2023

View reviewed changes

wadeking98 requested a review from bryce-mcmath December 27, 2023 22:42

removed defined orientation

39647b2

Signed-off-by: wadeking98 <[email protected]>

wadeking98 removed the do not merge Don't merge when label present label Jan 5, 2024

wadeking98 added 7 commits January 9, 2024 14:58

Merge branch 'main' of https://github.com/hyperledger/aries-mobile-ag…

5ab32aa

…ent-react-native into feat-update-camera

orientation partially working

0f67217

Signed-off-by: wadeking98 <[email protected]>

updated orientation detection

3a14dc1

Signed-off-by: wadeking98 <[email protected]>

fixed mobile device oriantation lock

d6e7717

Signed-off-by: wadeking98 <[email protected]>

resolved merge conflicts

91d0f5b

Signed-off-by: wadeking98 <[email protected]>

updated linting and tests

df72c21

Signed-off-by: wadeking98 <[email protected]>

Merge branch 'main' into feat-update-camera

7009558

jleach previously approved these changes Jan 16, 2024

View reviewed changes

jleach self-requested a review January 16, 2024 22:06

fixed aspect ratio

46244f0

Signed-off-by: wadeking98 <[email protected]>

wadeking98 dismissed jleach’s stale review via 46244f0 January 17, 2024 18:45

remove safe-area-insets since not used

3a5a71b

Signed-off-by: wadeking98 <[email protected]>

bryce-mcmath requested changes Jan 19, 2024

View reviewed changes

jleach previously approved these changes Jan 19, 2024

View reviewed changes

removed unneeded helpers

9ef96c4

Signed-off-by: wadeking98 <[email protected]>

wadeking98 dismissed jleach’s stale review via 9ef96c4 January 19, 2024 23:10

bryce-mcmath requested changes Jan 19, 2024

View reviewed changes

packages/legacy/core/App/components/misc/ScanCamera.tsx Show resolved Hide resolved

fixed scan orientation

402f70e

Signed-off-by: wadeking98 <[email protected]>

wadeking98 requested review from bryce-mcmath and jleach January 19, 2024 23:26

bryce-mcmath approved these changes Jan 19, 2024

View reviewed changes

jleach approved these changes Jan 19, 2024

View reviewed changes

Merge branch 'main' into feat-update-camera

5f7f49b

bryce-mcmath approved these changes Jan 19, 2024

View reviewed changes

wadeking98 merged commit 94e231e into openwallet-foundation:main Jan 20, 2024
6 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: updated camera to react-native-vision 3 #1046

feat: updated camera to react-native-vision 3 #1046

wadeking98 commented Dec 12, 2023 •

edited by jleach

Loading

codecov bot commented Dec 12, 2023 •

edited

Loading

bryce-mcmath commented Dec 12, 2023

bryce-mcmath commented Dec 12, 2023

wadeking98 commented Dec 13, 2023

jleach commented Dec 13, 2023

wadeking98 commented Dec 13, 2023

wadeking98 commented Dec 23, 2023

bryce-mcmath Dec 27, 2023

wadeking98 Dec 27, 2023

bryce-mcmath Dec 27, 2023

wadeking98 Dec 27, 2023

bryce-mcmath Dec 27, 2023

wadeking98 Jan 2, 2024

wadeking98 Jan 2, 2024

bryce-mcmath Jan 2, 2024

wadeking98 Jan 3, 2024

bryce-mcmath Jan 3, 2024

jleach Jan 16, 2024

wadeking98 Jan 16, 2024

jleach Jan 16, 2024

bryce-mcmath left a comment

feat: updated camera to react-native-vision 3 #1046

feat: updated camera to react-native-vision 3 #1046

Conversation

wadeking98 commented Dec 12, 2023 • edited by jleach Loading

Summary of Changes

Other advantages of this library:

Related Issues

Pull Request Checklist

codecov bot commented Dec 12, 2023 • edited Loading

Codecov Report

bryce-mcmath commented Dec 12, 2023

bryce-mcmath commented Dec 12, 2023

wadeking98 commented Dec 13, 2023

jleach commented Dec 13, 2023

wadeking98 commented Dec 13, 2023

wadeking98 commented Dec 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryce-mcmath left a comment

Choose a reason for hiding this comment

wadeking98 commented Dec 12, 2023 •

edited by jleach

Loading

codecov bot commented Dec 12, 2023 •

edited

Loading