fix(orientation): refactor orientation detection and fix related issues #1552
Conversation
| } | ||
|
|
||
| return videoOrientation | ||
| return UIDevice.current.orientation.videoOrientation |
There was a problem hiding this comment.
UIDeviceOrientation doesn't have a videoOrientation member?
https://developer.apple.com/documentation/uikit/uidevice/orientation
Or am I looking in the wrong place?
Isn't this the old switch case above? And what with the old comment:
// Get the orientation from the window scene if available
// When the app's orientation is fixed and the app orientation is actually different from the device orientation, it malfunctions.
does that still apply?
There was a problem hiding this comment.
Thanks for the thorough review and the LGTM—much appreciated!
Regarding your question on the iOS orientation handling: I've reverted those changes in this commit because I don't have access to physical iOS devices to fully test the UIDeviceOrientation logic and ensure it aligns with the window scene fallback (or avoids the issues noted in the old comment). I was only able to verify the fixes on Android, so I'd prefer to hold off on iOS until I can test properly or get some guidance/eyes on it from the team.
If you'd like to take a stab at refining that part (or point me to the right approach for the videoOrientation mapping), I'm all ears—happy to iterate! Otherwise, let me know if this works for merging the Android-focused fixes as-is.
(Added a changelog entry for the Android refactors)
|
Nice one! Any estimate on when this will be merged? |
|
@FelipeABastos I have a very busy week this week, so i'm aiming for next week. |
This reverts commit 4a3008a.
- Add flutter/services.dart import for DeviceOrientation enum - Update all BarcodePainter constructor calls to include required deviceOrientation parameter - Set deviceOrientation to DeviceOrientation.portraitUp for all test cases - Fixes compilation errors after BarcodePainter constructor was updated to require deviceOrientation All 6 tests now pass successfully.
This reverts commit 80d2195.
- Refactored Android orientation detection to use `OrientationEventListener`, improving accuracy during `MobileScanner` initialization. - Updated `BarcodePainter` to dynamically adjust `cameraPreviewSize` based on `deviceOrientation`, resolving overlay misalignment during device rotations.
As with official 1P plugins, these also don't use a header for the changelog, so I removed that here, too. For example: https://github.com/flutter/packages/blob/main/packages/camera/camera/CHANGELOG.md
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1552 +/- ##
===========================================
+ Coverage 38.14% 38.45% +0.31%
===========================================
Files 42 42
Lines 991 996 +5
===========================================
+ Hits 378 383 +5
Misses 613 613
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
One more thing, it seems that one of the files was not formatted, using the 80 character line length limit:
info • The line length exceeds the 80-character limit • lib/src/overlay/barcode_painter.dart:61:81 • lines_longer_than_80_chars
info • The line length exceeds the 80-character limit • lib/src/overlay/barcode_painter.dart:73:81 • lines_longer_than_80_chars
|
Also fixed a couple of long-line lints in the painter for good measure. |
5 participants
This PR fixes several orientation-related issues by refactoring the orientation detection
on Android.
Changes:
OrientationEventListener to get the physical device orientation instead of the UI
orientation.
cameraPreviewSize accordingly, fixing the barcode overlay alignment on orientation
change.
Related Issues:
Fixes #1486
Fixes #1474
Fixes #1462