Skip to content

Conversation

@tido64
Copy link
Member

@tido64 tido64 commented Sep 19, 2022

Description

Implements the QR code scanner feature that has existed on iOS for some time.

Resolves #1146.

Platforms affected

  • Android
  • iOS
  • macOS
  • Windows

Test plan

device-2022-09-19-201856

CameraX + ML Kit adds about 4-5 MBs to the installed app size. Make sure that if single app mode is enabled, we don't get that size bump.

@tido64 tido64 requested review from afoxman and kelset September 19, 2022 18:25
@github-actions github-actions bot added the platform: Android This affects Android label Sep 19, 2022
@tido64 tido64 mentioned this pull request Sep 20, 2022
4 tasks
@tido64 tido64 force-pushed the tido/android-qr-code-scanner branch from dffdf92 to 370b72c Compare September 22, 2022 19:10
Comment on lines +260 to +266
if (project.ext.react.enableCamera) {
main.java.srcDirs += "src/camera/java"
} else {
main.java.srcDirs += "src/no-camera/java"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is pretty cool tbh :D

Copy link
Contributor

@kelset kelset left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

managed to test it via the virtual camera in Android emulator, and overall it works well 👍

one small thing, that probably needs to be tested separately for both iOS and Android is to handle gracefully when the url is not a metro bundler one. In my first test I generated a random url and the app just crashes. Would be nice to error out, maybe with a "hey this url is not a valid metro instance, try again" or something.

But it's probably something that both iOS and Android need to be coded for, so it can be done as a follow up PR

@tido64 tido64 force-pushed the tido/android-qr-code-scanner branch from 370b72c to 5357a81 Compare October 4, 2022 17:14
@tido64
Copy link
Member Author

tido64 commented Oct 5, 2022

managed to test it via the virtual camera in Android emulator, and overall it works well 👍

one small thing, that probably needs to be tested separately for both iOS and Android is to handle gracefully when the url is not a metro bundler one. In my first test I generated a random url and the app just crashes. Would be nice to error out, maybe with a "hey this url is not a valid metro instance, try again" or something.

But it's probably something that both iOS and Android need to be coded for, so it can be done as a follow up PR

#1146

@tido64 tido64 merged commit d0b8f6d into trunk Oct 6, 2022
@tido64 tido64 deleted the tido/android-qr-code-scanner branch October 6, 2022 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform: Android This affects Android

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle random URLs more gracefully

4 participants