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

CB-13737 (iOS): fix Splash screen images for iPhone X #146

Merged
merged 1 commit into from Jan 9, 2018

Conversation

jcesarmobile
Copy link
Member

Platforms affected

iOS

What does this PR do?

Takes into account iPhone X sizes for splash screen

What testing has been done on this change?

Tested on iPhone X

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

@idoodler
Copy link

idoodler commented Jan 8, 2018

I can't wait for this PR to be merged!

@jcesarmobile
Copy link
Member Author

@idoodler have you tried? does it work fine for you?

@idoodler
Copy link

idoodler commented Jan 8, 2018

@jcesarmobile Yes, I can confirm, it is working on my iPhone X (Real device)

@lobo87
Copy link

lobo87 commented Jan 8, 2018

@jcesarmobile was thinking about creating a PR on my own, then saw your solution. We have nearly the same implementation, with the only difference that i split up iphone6Plus and iPhoneX conditions for more readability.

However +1 for your solution!

@jcesarmobile
Copy link
Member Author

Yeah, I did it this way to avoid duplicating the landscape/portrait part, we could probably improve that part entirely.

I will merge it later

@idoodler
Copy link

idoodler commented Jan 9, 2018

Whats up with the continuous integration test? It looks like the error has nothing to to with this fix.

@jcesarmobile
Copy link
Member Author

@idoodler yeah, the test failure has nothing to do with the PR, they fail for Android. But that wasn't stopping me from merging, I just didn't have time yesterday.
Merging it now and will try to fix the tests later

@jcesarmobile jcesarmobile merged commit be8be33 into apache:master Jan 9, 2018
@jcesarmobile jcesarmobile deleted the CB-13737 branch January 9, 2018 19:00
@iisdan
Copy link

iisdan commented Jan 10, 2018

Thank you!!!

@DuBistKomisch
Copy link

This fixed it for us as well, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants