[WIP] Device orientation #1716
[WIP] Device orientation #1716
Conversation
| MediaQueryCapability, | ||
| LocationCapability, | ||
| DeviceInfoCapability { | ||
| class Device with Invokable, MediaQueryCapability, LocationCapability, DeviceInfoCapability, WidgetsBindingObserver { |
There was a problem hiding this comment.
Setup your formatter to property format code so it is consistent with everyone else. Your review reformats a lot of the code and it shows more changes than needed
There was a problem hiding this comment.
Yeah, i do keep the necesarry changes, it was a draft that's why there comments in it as well, Thanks for pointing out.
| @@ -98,6 +98,20 @@ class DataContext implements Context { | |||
| if (_contextMap['device'] == null) { | |||
| _contextMap['device'] = Device(); | |||
| } | |||
|
|
|||
| // Always set up the listener | |||
| print('👂 Setting up device update listener...'); | |||
There was a problem hiding this comment.
This doesn't make sense. What is it for? DataContext already points to Device class which handle listening for changes.
| static Stream<MediaQueryData> get onDeviceUpdate => _deviceUpdateController.stream; | ||
|
|
||
| Device._internal() { | ||
| if (!_isInitialized) { |
There was a problem hiding this comment.
Device is a singleton, meaning there's only 1 instance of it. You don't need to check if it's initialized or not.
| void didChangeMetrics() { | ||
| print('📏 Metrics changed - handling update...'); | ||
| if (!_isHandlingChange) { | ||
| WidgetsBinding.instance.addPostFrameCallback((_) => _handleMediaQueryChange()); |
There was a problem hiding this comment.
Why do addPostFrameCallback() here for? Just call handleMediaqueryChange() directly.
There was a problem hiding this comment.
This was to make sure that values gets updated properly, wot was happening before:
- when I move from portrait to landscape, value was still portrait.
- and when I again moved back to portrait from landscape, then value updated to portrait and so on.
it was fixed using addPostFrameCallback()
| @override | ||
| void didChangeMetrics() { | ||
| print('📏 Metrics changed - handling update...'); | ||
| if (!_isHandlingChange) { |
There was a problem hiding this comment.
what is this flag for, preventing multiple concurrent call? There is no async code here so that's not going to happen.
|
|
||
| // Broadcast the change | ||
| print('📢 Broadcasting device update...'); | ||
| _deviceUpdateController.add(newData); |
There was a problem hiding this comment.
Why? Why don't you set the orientation/width/height directly?
| } | ||
| } | ||
|
|
||
| bool _hasSignificantChanges(MediaQueryData? oldData, MediaQueryData newData) { |
There was a problem hiding this comment.
Maybe you use this for testing but why not just remove this function and change it to "orienttation != newData.orientation || size != newData.size". Simpler and less code to look through
TheNoumanDev
requested review from
vusters and
snehmehta
and removed request for
vusters
|
@snehmehta please review this |
No description provided.