search

juliansteenbakker / mobile_scanner

A universal scanner for Flutter based on MLKit. Uses CameraX on Android and AVFoundation on iOS.
BSD 3-Clause "New" or "Revised" License
745 stars 444 forks source link

feat: change platform os detect pattern #1032

Closed penkzhou closed 3 weeks ago

penkzhou commented 3 weeks ago

current code in lib/src/method_channel/mobile_scanner_method_channel.dart cannot change the platform in the test code when need to change the platform. according to this thread, maybe we should change the platform os detect pattern into defaultTargetPlatform. ref:https://stackoverflow.com/questions/55233853/flutter-widget-tests-with-platform-branching

penkzhou commented 3 weeks ago

cc @navaronbracke .

navaronbracke commented 3 weeks ago

@penkzhou Just to be sure, this is primarily for people that write tests, right?

I had a look at the implementation of defaultTargetPlatform and that uses the Platform.is getters under the hood on non-web platforms.

penkzhou commented 3 weeks ago

@penkzhou Just to be sure, this is primarily for people that write tests, right?

I had a look at the implementation of defaultTargetPlatform and that uses the Platform.is getters under the hood on non-web platforms.

IMO, it is the best practise. According to the official doc, The [dart:io.Platform](https://api.flutter.dev/flutter/dart-io/Platform-class.html) object should only be used directly when it's critical to actually know the current platform, without any overrides possible (for example, when a system API is about to be called).

navaronbracke commented 3 weeks ago

You are correct, yes. In these cases, we already called the native side and we just parse the result. There is no "system level API" that we needed to call here.

penkzhou commented 3 weeks ago

yeah. with defaultTargetPlatform, we can change the platform in test case with debugDefaultTargetPlatformOverride api. the ref is here.

penkzhou commented 3 weeks ago

@navaronbracke I just add the additional change on the example/lib/barcode_scanner_window.dart,Could you please help review this?

penkzhou commented 3 weeks ago

If you want to update the version to 5.0.1 and update the changelog to include a mention of this fix, that would be helpful.

ok.

penkzhou commented 3 weeks ago

If you want to update the version to 5.0.1 and update the changelog to include a mention of this fix, that would be helpful.

I have already submitted all files related to the version changes. Please take a look and let me know if any other changes are needed.