Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added cameratype (front/back) to properties of BarcodeScanner Native #34

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

magnusgreennl
Copy link

On request of a customer we had to add an option to use the front camera for the BarcodeScanner (hanging tablet on a wall, where barcodes were presented to the tablet). This is a standard option on react-native-camera (see doc).

Note that the front camera often is of less quality, so barcodes will be harder to scan.

Checklist

  • Contains unit tests ✅ ❌
  • Contains breaking changes ✅ ❌
  • Compatible with: MX 7️⃣, 8️⃣, 9️⃣
  • Did you update version and changelog? ❌
  • PR title properly formatted ([XX-000]: description)? ✅ ❌
  • Works in Android ✅
  • Works in iOS ✅
  • Works in Tablet ✅

Feature specific

  • Comply with designs ✅ ❌
  • Comply with PM's requirements ✅ ❌

Please remove unnecessary emojis and sections and this comment before proceeding

This PR contains

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Other (describe)

What is the purpose of this PR?

...

Relevant changes

Please add a high level explanation of what was changed and how the initial problem was solved

What should be covered while testing?

...

Extra comments (optional)

Please add extra comments or delete the section if not required

@magnusgreennl magnusgreennl requested a review from a team as a code owner March 15, 2023 17:43
@diego-antonelli
Copy link
Contributor

@jdiehl Can you check if this feature is something we should include in the widget?

@@ -21,6 +21,7 @@ export class BarcodeScanner extends Component<Props> {
<RNCamera
testID={this.props.name}
style={{ flex: 1, justifyContent: "center", alignItems: "center" }}
type={this.props.cameraType == 'back' ? RNCamera.Constants.Type.back : RNCamera.Constants.Type.front}

Choose a reason for hiding this comment

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

Could you invert the condition like this:

type={this.props.cameraType === 'front' ? RNCamera.Constants.Type.front : RNCamera.Constants.Type.back}

This is for the sake of being future proof. If the names change or have other types of cameras, this way it can fallback to back instead of front.

Also, please use three equals :)

Choose a reason for hiding this comment

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

Good points, fixed both!

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me - @furkanarabaci what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants