Switched to (imo) better error handling #4
Conversation
| @@ -38,7 +38,8 @@ - (void)viewDidAppear:(BOOL)animated { | |||
| if (success) { | |||
| [self startScan]; | |||
| } else { | |||
| [self showNoCameraAccessAlert]; | |||
| [self.delegate barcodeScannerViewController:self didScanBarcodeWithResult:@"PERMISSION_NOT_GRANTED"]; | |||
There was a problem hiding this comment.
I would prefer this be implemented as a separate delegate method. Something like barcodeScannerViewController:didFailWithError:
There was a problem hiding this comment.
good catch, makes the code way cleaner!
example/lib/main.dart
Outdated
| String barcode = await BarcodeScanner.scan(); | ||
| setState(() => this.barcode = barcode); | ||
| } on PlatformException catch (e) { | ||
| if (e.code == 'PERMISSION_NOT_GRANTED') { |
There was a problem hiding this comment.
Lets turn PERMISSION_NOT_GRANTED into a constant so we can use if (e.code == BarcodeScanner.CameraAccessDenied or something similar.
|
@BenSower Thanks for the PR! I agree that letting the user handle this exception in flutter is much better. Ideally, we would request the camera permission first from flutter with something like I've been thinking about writing another plugin purely for managing permissions across iOS and Android. IMO there should just be a general way in flutter for testing and requesting permissions. I did see this issue flutter/flutter#685 which where the last comment contains someones work in progress for a generic permissions mechanism: https://github.com/goposse/permit |
|
@matthewtsmith I implemented the requested changes, hoping they are what you requested? Also: Yeah, I though about creating such a plugin, too. Especially since I had to implement this error handling now multiple times, since other plugins also did not handle the permissions very well, yet. Currently I don't have a lot of capacity to start something like this, so I will probably only be able to start at the end of january, but I'd definitely support it though. |
|
@BenSower Thanks for the updates. Everything looks good so I'll merge in and release a new version soon. RE: Bandwidth, I'm in the same boat. I'm hoping to start something like that after the DartConf. I'll try and drum up some interest in it so maybe we can get a few contributors. |
|
@matthewtsmith @BenSower Let me know if you guys need help. I'm looking for a generic permissions plugin as well. |
|
@matthewtsmith Perfect, thanks! |
Hey,
I recently started using your project since the other BarcodeScannerProject doesn't seem to be maintained anymore and I am still waiting for a PR to be merged (https://github.com/BenSower/BarcodeScannerPlugin/tree/scanlineFix) ;-).
Still,
I removed the internal iOs error messages if the user does not grant the permission the necessary permissions. This way, a dev can not only properly catch (no pun intended, but you can now actually catch it as a Platform exception with error code
'PERMISSION_NOT_GRANTED') when that happens, but also react in whatever way and language he or she wants. This also somewhat solves the problem of the customizable error texts, since you can now define whatever message you like.
For more info, take a look into the example.
I am still not a fan of the "cancel" als well as the "flash-on/off" Button in terms of internationalization and would recommend to replace both with the appropriate icons. Also, I do like the Android implementation of Zxing, which displays a frame for the scanner, which allows for easier aiming and would love something similar in iOs.
I did implement both those things for the other project and will try to implement this, too...If there will be some time in the near future...