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

fix: improve ios launch screen on notch devices #2810

Merged

Conversation

simonbengtsson
Copy link
Contributor

What

Fixes position of the launch screen logo on notch devices.

Screenshot

Screen Shot 2022-08-19 at 22 18 13

@codecov-commenter
Copy link

codecov-commenter commented Aug 19, 2022

Codecov Report

Merging #2810 (eda84c3) into develop (85f491b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           develop   #2810   +/-   ##
=======================================
  Coverage     7.05%   7.05%           
=======================================
  Files          219     219           
  Lines        10864   10864           
=======================================
  Hits           766     766           
  Misses       10098   10098           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@teolemon
Copy link
Member

Nice. Fyi, it's also happening on non notch devices

@monsieurtanuki
Copy link
Contributor

monsieurtanuki commented Aug 20, 2022 via email

@simonbengtsson
Copy link
Contributor Author

simonbengtsson commented Aug 20, 2022

Oh! Didn't see that. Tried running the flutter_native_splash:create command now, but it resulted in 50+ file changes and a different launch screen on iOS than in app today (see below). Should we add an issue about fixing the splash screen generation and in the meantime merge this PR?

Launch screen after flutter_native_splash:create

@M123-dev
Copy link
Member

Thats right @monsieurtanuki but I'm not sure if we are currently using the files of the generator, @g123k did some changes to use svgs instead of pngs which was not supported by the plugin but this could also just have affected the app icon. @g123k what do you say?

@M123-dev
Copy link
Member

Fixes: #2787

@g123k
Copy link
Collaborator

g123k commented Aug 20, 2022

Thats right @monsieurtanuki but I'm not sure if we are currently using the files of the generator, @g123k did some changes to use svgs instead of pngs which was not supported by the plugin but this could also just have affected the app icon. @g123k what do you say?

Yes indeed, I did it manually because the plugin generated broken things.
We should remove it and do everything manually.

@M123-dev M123-dev requested a review from g123k August 20, 2022 12:25
@monsieurtanuki
Copy link
Contributor

Honestly I would prefer us to use flutter_native_splash, which is a "Flutter Favorite" with 100% popularity and only 9 open issues. I don't think we can be much smarter than them, and that's not where we're supposed to be good at anyway.
And if the problem is just how to generate png files from svg files, I'm sure we can manage that.

That said, if there's a consensus about getting rid of that package, that's not my project so why not. But if you guys decide remove flutter_native_splash, do remove it.

Btw that is the answer to the current PR: jonbhanson/flutter_native_splash#325

@g123k
Copy link
Collaborator

g123k commented Aug 20, 2022

Honestly I would prefer us to use flutter_native_splash, which is a "Flutter Favorite" with 100% popularity and only 9 open issues. I don't think we can be much smarter than them, and that's not where we're supposed to be good at anyway. And if the problem is just how to generate png files from svg files, I'm sure we can manage that.

That said, if there's a consensus about getting rid of that package, that's not my project so why not. But if you guys decide remove flutter_native_splash, do remove it.

Btw that is the answer to the current PR: jonbhanson/flutter_native_splash#325

I’m still convinced to to do it manually since our custom Android implementation supports dark mode and themed icons.

@M123-dev
Copy link
Member

I am open to this, but we can of course keep the splash generator and remove the icon generator if one of them is missing features.

@monsieurtanuki
Copy link
Contributor

Please do something that works and that flutter devs can maintain.

@monsieurtanuki
Copy link
Contributor

I've just downloaded the app from the google play store.
That didn't go well:
Screenshot_2022-08-22-12-28-06

@monsieurtanuki
Copy link
Contributor

Then the camera froze, then switched to "always black" mode.
I tried to restart the app, but now it crashes systematically (I just have the splash screen).
But that's another story.

@teolemon
Copy link
Member

@simonbengtsson
Copy link
Contributor Author

Great! Rebased against develop now in case we want to merge this.

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

Hi @simonbengtsson!
As far as I understand iOS LaunchScreen storyboard, looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍎 iOS iOS specific issues or PRs splashscreen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants