-
Notifications
You must be signed in to change notification settings - Fork 6k
[ios] view controller based status bar #42643
[ios] view controller based status bar #42643
Conversation
fiox rename test
9076a22
to
9ae7979
Compare
shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm
Outdated
Show resolved
Hide resolved
@@ -175,26 +181,46 @@ - (void)setSystemChromeEnabledSystemUIOverlays:(NSArray*)overlays { | |||
postNotificationName:FlutterViewControllerHideHomeIndicator | |||
object:nil]; | |||
} | |||
if (self.enableViewControllerBasedStatusBarAppearance) { | |||
// This notification is respected by the iOS embedder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: comments are missing periods.
@@ -175,26 +181,46 @@ - (void)setSystemChromeEnabledSystemUIOverlays:(NSArray*)overlays { | |||
postNotificationName:FlutterViewControllerHideHomeIndicator | |||
object:nil]; | |||
} | |||
if (self.enableViewControllerBasedStatusBarAppearance) { | |||
// This notification is respected by the iOS embedder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we communicating within the embedder via notifications, rather than an explicit delegate or callback relationship? My previous experience with notification for communication within a project is that it's significant headache for understanding how components interact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copy pasted the old code and didn't really dig into the reason behind it. Most of the notification code are 5+ year old. And looking at the https://github.com/flutter/engine/blob/8c3eeb572f91b86c4fc6a5b90e85d6d008de3dc7/shell/platform/darwin/ios/BUILD.gn at the time, both FlutterPlatformPlugin and FlutterViewController are under the same build target. So I think it was probably just overlooked.
I will remove this notification and also create an issue to move away from the notifications.
I'm not sure if there are apps out there using these notifications directly. We should probably keep these notification for now and slowly deprecate them.
// The |UIViewController:prefersStatusBarHidden| of this ViewController is overriden and returns | ||
// `flutterPreferesStatusBarHidden`. Only works when `UIViewControllerBasedStatusBarAppearance` in | ||
// info.plist of the app project is `true`. | ||
@property(nonatomic, assign) BOOL flutterPreferesStatusBarHidden; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: prefers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a few comments.
/** | ||
* @brief Whether the status bar appearance is based on the style preferred for this ViewController. | ||
* | ||
* The default value is true. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: "YES", not "true".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm
Outdated
Show resolved
Hide resolved
@@ -47,6 +60,9 @@ - (instancetype)initWithEngine:(fml::WeakPtr<FlutterEngine>)engine { | |||
|
|||
if (self) { | |||
_engine = engine; | |||
NSNumber* infoValue = [[NSBundle mainBundle] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code (the next line, specifically) will crash if someone puts the wrong key type in their Info.plist. Since that's a developer error that's probably okay, but it might be nice to check the type and log (in debug only) a clear error message if it's wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
* returns `flutterPrefersStatusBarHidden`. This is ignored when | ||
* `UIViewControllerBasedStatusBarAppearance` in info.plist of the app project is `false`. | ||
*/ | ||
@property(nonatomic, assign) BOOL flutterPrefersStatusBarHidden; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the structure again here... couldn't you just redeclare prefersStatusBarHidden
as read-write, to avoid having two getters that do exactly the same thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I still kept the local var's name to be _flutterPrefersStatusBarHidden so it is clearer that the getter and setter are overridden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* @brief Whether the status bar is preferred hidden. | ||
* | ||
* This overrides the |UIViewController:prefersStatusBarHidden| of this | ||
* ViewController is overriden. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove redundant "is overriden".
…128959) flutter/engine@9934c0d...48e0b4e 2023-06-15 [email protected] Roll Skia from 0b66c6928dcf to 2d531d020c26 (3 revisions) (flutter/engine#42883) 2023-06-15 [email protected] Raster cache should preserve RTree for overlay layers (flutter/engine#42552) 2023-06-15 [email protected] Roll Skia from c0c74b433117 to 0b66c6928dcf (1 revision) (flutter/engine#42879) 2023-06-15 [email protected] [Linux] Allow BasicMessageChannel sending and responding to null message (flutter/engine#42808) 2023-06-15 [email protected] Roll Skia from 12375fb6f3c8 to c0c74b433117 (1 revision) (flutter/engine#42876) 2023-06-15 [email protected] Roll Skia from d62221bd33a6 to 12375fb6f3c8 (1 revision) (flutter/engine#42873) 2023-06-15 [email protected] Roll Skia from e2e0256d4c6a to d62221bd33a6 (1 revision) (flutter/engine#42871) 2023-06-15 [email protected] Roll Skia from c3abd540c7f9 to e2e0256d4c6a (1 revision) (flutter/engine#42869) 2023-06-15 [email protected] Roll Fuchsia Linux SDK from uvmDF7KM34dWGdsuK... to 53EjCyuRu91oFTBf2... (flutter/engine#42868) 2023-06-15 [email protected] Roll Fuchsia Mac SDK from h3-8RUVrC889UXou7... to P7QA6bfO_Ij5dre7B... (flutter/engine#42867) 2023-06-15 [email protected] Roll Skia from 2718866006d2 to c3abd540c7f9 (1 revision) (flutter/engine#42866) 2023-06-15 [email protected] Roll Skia from 19051bc5fc90 to 2718866006d2 (33 revisions) (flutter/engine#42864) 2023-06-15 [email protected] Roll Dart SDK from 922c315b2c34 to 8eaed3382237 (1 revision) (flutter/engine#42862) 2023-06-15 [email protected] Add missing artifact to the android_arm64_profile config. (flutter/engine#42858) 2023-06-14 [email protected] Build skia with expat (flutter/engine#42859) 2023-06-14 [email protected] [ios] use interfaceOrientation orientation on iOS 13 and above (flutter/engine#42846) 2023-06-14 [email protected] Manual roll Dart SDK from f1387834bfd9 to 922c315b2c34 (4 revisions) (flutter/engine#42855) 2023-06-14 [email protected] [Impeller] Make interleaved layout (more) explicit in generated headers. (flutter/engine#42628) 2023-06-14 [email protected] Renamed validation layers build (flutter/engine#42826) 2023-06-14 [email protected] [ios] view controller based status bar (flutter/engine#42643) 2023-06-14 [email protected] Roll Skia from 6d5dc31d88e2 to 19051bc5fc90 (25 revisions) (flutter/engine#42828) 2023-06-14 [email protected] Roll Fuchsia Linux SDK from Xi3c5nti2LKnEOqYt... to uvmDF7KM34dWGdsuK... (flutter/engine#42842) 2023-06-14 [email protected] Fix generateLockfiles running directory for documentation (flutter/engine#42734) 2023-06-14 [email protected] Roll Fuchsia Mac SDK from Cld7-rm6ZmCOO8j-K... to h3-8RUVrC889UXou7... (flutter/engine#42839) 2023-06-14 [email protected] Roll ANGLE from 7e075469ff02 to 3a3a3c655a96 (8 revisions) (flutter/engine#42834) 2023-06-14 [email protected] Roll Dart SDK from c4e9794df8af to f1387834bfd9 (1 revision) (flutter/engine#42833) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from Xi3c5nti2LKn to 53EjCyuRu91o fuchsia/sdk/core/mac-amd64 from Cld7-rm6ZmCO to P7QA6bfO_Ij5 If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
UIApplication based status bar API was deprecated.
This PR makes the ViewController based status bar appearance the default.
Still keep UIApplication based status bar for apps that explicitly sets
UIViewControllerBasedStatusBarAppearance
to NO in info.plist.Part of flutter/flutter#128969
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.