Splash screen images for iPhone XR, iPhone XS, and iPhone XSMAX #158
Comments
project-bot
bot
added this to new/unassigned/reopened issues
in Apache Cordova: project-bot automation testing
project-bot
bot
moved this from new/unassigned/reopened issues
to edited issue
in Apache Cordova: project-bot automation testing
|
Awesome. So what you want to do is create a Pull Request (PR) from a branch in a fork of this repository where you changed those files. Small glossary:
Now some links with instructions what you have to do:
This seems like a lot now, but it is just a few simple steps once you understood it. Let me know if you hit any stumbling blocks, I'll be happy to help you along the way. |
|
I might need some help:
I performed a fork, made changes to the files in the fork that showed up in my GitHub account and from within my account at the completion of the edits created Pull Request, but if I go into https://github.com/apache/cordova-plugin-splashscreen/pulls <https://github.com/apache/cordova-plugin-splashscreen/pulls> I’m not seeing the Pull Request show up??
… On Sep 22, 2018, at 2:16 AM, Jan Piotrowski ***@***.***> wrote:
Awesome.
So what you want to do is create a Pull Request (PR) from a branch in a fork of this repository where you changed those files.
Small glossary:
Pull Request = Like an issue, but has code attached to it that can simply be merged into the original repository
Branch = A "branch" in a repository with some changes. There mostly is a master branch that has the main code, and many other branches with slight modifications. That way you can have a branch for each bug, feature etc. and it is easy to see only the changes for that
Fork = Copy of a repository under your username. At first a fork is just a copy, but then you can make changes in there (you can't do that here because you don't have access) and send a PR with those changes later.
Now some links with instructions what you have to do:
Fork the repo: https://help.github.com/articles/fork-a-repo/ <https://help.github.com/articles/fork-a-repo/> Really relevant is just this bit: https://help.github.com/articles/fork-a-repo/#fork-an-example-repository <https://help.github.com/articles/fork-a-repo/#fork-an-example-repository>
Make changes in your repository: Now you can check out your fork with Git to your computer, change the files and commit and push the changes: https://help.github.com/articles/cloning-a-repository/ <https://help.github.com/articles/cloning-a-repository/> + https://help.github.com/articles/adding-a-file-to-a-repository-using-the-command-line/ <https://help.github.com/articles/adding-a-file-to-a-repository-using-the-command-line/>
Or you can do it online on Github.com in the UI: https://help.github.com/articles/editing-files-in-your-repository/ <https://help.github.com/articles/editing-files-in-your-repository/>
When you have edited both files in your fork, you can create a pull request on this repository to get your changes back here: https://help.github.com/articles/creating-a-pull-request/ <https://help.github.com/articles/creating-a-pull-request/>
Then we have a Pull Request here with your changes that we can look at.
This seems like a lot now, but it is just a few simple steps once you understood it. Let me know if you hit any stumbling blocks, I'll be happy to help you along the way.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#158 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Apejd120ameQZA78kIN18etqndBZyNZcks5udf_wgaJpZM4W1Lu1>.
|
|
Common misunderstanding: You are creating the pull requests from the branch in your fork to As I saw your two PRs, I noticed you edited the files in two different branches - sdgpalm2-patch-1 and sdgpalm2-patch-2 - which means you also need two PRs to get all the changes in. This should not be the case, you should try to edit both files in the same branch. You can do that by either going to one of the branches in your repository and also edit the other file, or create a Pull Request from one branch to the other, merge it and then merge that branch into the Cordova repository. Let me know if this works. |
|
It’s now showing up as a Pull request, is there anything else to do?
… On Sep 22, 2018, at 10:44 AM, Jan Piotrowski ***@***.***> wrote:
Common misunderstanding: You are creating the pull requests from the branch in your fork to master in your fork. They show up here: https://github.com/sdgpalm2/cordova-plugin-splashscreen/pulls <https://github.com/sdgpalm2/cordova-plugin-splashscreen/pulls> You have to create the Pull Request to this repository, for example by creating the PR here: https://github.com/apache/cordova-plugin-splashscreen/compare <https://github.com/apache/cordova-plugin-splashscreen/compare>
As I saw your two PRs, I noticed you edited the files in two different branches - sdgpalm2-patch-1 and sdgpalm2-patch-2 - which means you also need two PRs to get all the changes in. This should not be the case, you should try to edit both files in the same branch. You can do that by either going to one of the branches in your repository and also edit the other file, or create a Pull Request from one branch to the other, merge it and then merge that branch into the Cordova repository.
Let me know if this works.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#158 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Apejd_QaTcdlUvQfWFyAcDONmPpBcvqkks5udnbqgaJpZM4W1Lu1>.
|
|
Not really, congratulations on your first PR! 🥇 💯 (The tests are failing, but that is because they are failing for all commits because of a infrastructure problem, the actual code tests seem to be fine) You can add Now someone from the Apache Cordova team will have to look at, review and then merge it. |
janpio
moved this from edited issue
to new/unassigned/reopened issues
in Apache Cordova: project-bot automation testing
|
Hi, iPhone XsMax and iPhone Xr have the same height in points (896) but not in pixels (2688, 1792 pixels), the launch images are not the same size in xcassets, there are distinct images for XsMax and Xr launch screen, you have to go with native screen bounds to detect the right iPhone model... |
|
@Stefano1964, can you follow the same steps I posted above to create a PR please? |
|
This is my own version, i do not fork @sdgpalm2... |
|
Exactly, a new PR for the changes you made so they can possibly be integrated back into this plugin. |
|
Pull #159 has been updated with code provided from @Stefano1964 |
|
@sdgpalm2 thanks, i'm really busy in this days. |
|
@Stefano1964, I've been using Asset Catalog Creator and did remove and replaced the xcassets, I even cleared the apps from the simulators and all the graphics did show up for me, but anyways, after reading your code, thought your process was correct, part of testing of your code yesterday was to edit the three images and place the works "X XS", "XR", "XSMax" into their respected selfs and did verified that they did show up correctly at launch based on the selected Xcode simulator, Thanks again! |
|
@sdgpalm2 it's working fine if the app is launched in portrait mode but not in landscape because i made an error, i was testing only height, we have to test both height and width as the original code for points does. Attached the last version. |
|
@Stefano1964, code has beed code applied. |
Not Working in cordova 9.0 and ios5.0 version . |
|
The plugin has been integrated into cordova-ios 6, so no more iOS work will be done here. |
project-bot
bot
moved this from new/unassigned/reopened issues
to closed issues
in Apache Cordova: project-bot automation testing
Code updated to support "Splash screen images for iPhone XR, iPhone XS and iPhone XSMAX", files attached, just expanded on the above code. iPhone XS uses iPhone X code, iPhone XR and iPhone XSMAX uses same limit of limit == 896. New to all this GitHub stuff, contributing openly.
Yes I would be interested in learning to fork and submit this in a proper PR, thanks!
CDVSplashScreen_h.txt
CDVSplashScreen_m.txt
See pull #159
The text was updated successfully, but these errors were encountered: