Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes iOS and Android Splash Screens #159

Merged
merged 2 commits into from
Jul 27, 2021
Merged

Fixes iOS and Android Splash Screens #159

merged 2 commits into from
Jul 27, 2021

Conversation

ggirotto
Copy link
Contributor

Fixes #130

@ggirotto ggirotto added enhancement New feature or changes to an existing one target: flutter Relates to Flutter and its sub-dependencies labels Jul 23, 2021
@ggirotto ggirotto self-assigned this Jul 23, 2021
@ggirotto ggirotto requested a review from matuella July 23, 2021 18:39
Copy link
Contributor

@matuella matuella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a temporary solution? It doesn't seems to me that it's an assumed solution, given how there's no associated Flutter issue with this (and it's clearly an issue with the framework then).

Also, does this occurs on Android devices as well for you to update the Android solution?

@matuella
Copy link
Contributor

Maybe a regression issue? Loosely related: flutter/flutter#37155

@ggirotto
Copy link
Contributor Author

Why do you believe this is not an assumed solution? Using the native splash screens avoids any possible Flutter problem and it's definitely the optimal solution, since it uses the native way of handling with splash screens instead relying on Flutter framework lifecycle.

I also updated Android because now the splash screen appears faster.

I don't think that #37155 is the same issue that we're facing, since the OP reports that the app freeze at the black screen, and we just have an unexpected delay (maybe Flutter framework initialize delay, not sure)

@ggirotto ggirotto requested a review from matuella July 24, 2021 01:00
Copy link
Contributor

@matuella matuella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you believe this is not an assumed solution? Using the native splash screens avoids any possible Flutter problem and it's definitely the optimal solution, since it uses the native way of handling with splash screens instead relying on Flutter framework lifecycle.

Well, if you're going that way, I could say the same for "implementing native UI screens is the optimal solution". If it's out of Flutter's scope of handling stuff, I would totally agree with you, such as handling camera usage, audio-related operations, etcetera. - we could even possibly create a plugin for such thing -, but again, I don't believe this case. A splash screen is something that's not required, but it's a standard that all mobile applications, and it's definitely something that we should expect from Flutter's framework - and after all, it's just a screen.

What I feel from this is simply a limitation - or some bug - with Flutter, thus why it doesn't feel to me like a solution, more like a workaround.

@ggirotto
Copy link
Contributor Author

I disagree this is a workaround, since it's suggested even in the Flutter docs. They state the same that I believe:

They (SplashScreen) set the stage for your application, while allowing time for the app engine to load and your app to initialize.

If there is the possibility of Flutter engine delays it initialization, we cannot rely on it Flutter to provide a solution to the problem. They also state:

Each Flutter experience in an app requires a few moments to initialize the Dart isolate that runs the code. This means that a user momentarily sees a blank screen until Flutter renders its first frame.

Which is the exactly description about this issue. They propose a "Flutter" solution that uses a Android View, which I personally believe that it's worst from the platform-related solution proposed by this PR

@ggirotto ggirotto requested a review from matuella July 26, 2021 16:59
matuella
matuella previously approved these changes Jul 26, 2021
Copy link
Contributor

@matuella matuella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree this is a workaround, since it's suggested even in the Flutter docs.

Indeed, this is what the Flutter's team recommends to do - I was completely wrong. Interesting article, haven't seen this one when I read the docs last time!

Nonetheless, I still feel like this is something that we could think about improving, although I think it won't be trivial. My only argument to why this can lead to future problems is that a future change to the SplashPage is not guaranteed to be equal in both native platforms, that's my only concern with 3 different ecosystems requiring the exact same interface, that may change over time. Even though this is the ideal solution, given the cited article, I suggest it to have a little succinct section in our ARCHITECTURE, as a reference for future changes. This sections is a nitpick, it's up to you to add it.

@ggirotto ggirotto merged commit 303b8c8 into main Jul 27, 2021
@ggirotto ggirotto deleted the fix-splash branch August 2, 2021 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or changes to an existing one target: flutter Relates to Flutter and its sub-dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

White screen when opening/loading the app
2 participants