You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Panajev <gi...@git.apache.org> on 2016/03/25 16:51:13 UTC
[GitHub] cordova-plugin-splashscreen pull request: Added new spinner style,...
GitHub user Panajev opened a pull request:
https://github.com/apache/cordova-plugin-splashscreen/pull/95
Added new spinner style, fix for iOS 8 bugs, and improved documentation.
* Improved documentation.
* Added references to the new material spinner and included its BSD like license.
* Small Xcode suggested indentation changes.
* Listen to main frame URL reloads.
* When the URL reloads: Put up the splash screen, but add a configurable timeout in case the page loaded event takes a long while to come through.
* Added iPad Pro image for the splashscreen.
* Fixed small issue rotation iPhone 4 and iPhone 5 devices on iOS 8.
* Fix for portrait rotation issue 8.0 - 8.4.
* Adding window.nativeWrapper to clobber.
Changes Signed-off-by: Goff Marocchi, Gabriela Pittari, James Clement
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Panajev/cordova-plugin-splashscreen master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugin-splashscreen/pull/95.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #95
----
commit eba7b5af7da8987f89a372ef923b6d712e8af72d
Author: Goffredo Marocchi <go...@gamesys.co.uk>
Date: 2016-03-25T15:46:42Z
* Improved documentation.
** Added references to the new material spinner and included its BSD like license.
* Small Xcode suggested indentation changes.
* Listen to main frame URL reloads.
** When the URL reloads: Put up the splash screen, but add a configurable timeout in case the page loaded event takes a long while to come through.
* Added iPad Pro image for the splashscreen.
* Fixed small issue rotation iPhone 4 and iPhone 5 devices on iOS 8.
* Fix for portrait rotation issue 8.0 - 8.4
* Adding window.nativeWrapper to clobber.
Changes Signed-off-by: Goff Marocchi, Gabriela Pittari, James Clement
Signed-off-by: Goffredo Marocchi <go...@gamesys.co.uk>
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-splashscreen issue #95: Show splashscreen everytime a new pag...
Posted by infil00p <gi...@git.apache.org>.
Github user infil00p commented on the issue:
https://github.com/apache/cordova-plugin-splashscreen/pull/95
Noticing that this PR still has the HTML5 Boilerplate licence, and a bunch of conflicts. I'm going to consider this PR abandoned and I'm going to close it. Please issue a new PR if you still wish to contribute.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] cordova-plugin-splashscreen pull request: Added new spinner style,...
Posted by jcesarmobile <gi...@git.apache.org>.
Github user jcesarmobile commented on the pull request:
https://github.com/apache/cordova-plugin-splashscreen/pull/95#issuecomment-203421921
Thanks for this PB, but I think we should keep plugins simple and this PR add too many features and they are iOS only.
Why does it need the HTML5 Boilerplate license?
I think the material spinner and the listener for the frame URL reloads should be separated plugins, not part of the splashscreen plugin.
For the iPad pro and other bug fixes. Do they have a JIRA issue? if not, it should be created and use the id on the title of the PR. (http://issues.cordova.io/)
For the new iPad pro splash you add, can you add it to the README.md Example Configuration section too? We should create an iPad pro default splash too and add it to the base project.
What is the window.nativeWrapper clobber for?
Before we can merge it though, you need to sign Apache's Contributor License Agreement (can be done online): http://www.apache.org/licenses/#clas
Not sure if Goff Marocchi, Gabriela Pittari and James Clement have to sign theirs too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-splashscreen pull request: Show splashscreen everyt...
Posted by Panajev <gi...@git.apache.org>.
Github user Panajev commented on the pull request:
https://github.com/apache/cordova-plugin-splashscreen/pull/95#issuecomment-203582719
@jcesarmobile Sorry for the early PR and for not really following all the steps of the process in the right order :/.
**Signing**
Goff Marocchi is me (actually Goffredo Marocchi) and yes I do still need to sign it, but I will ask James and Gabriela to sign it too.
**Multiplatform Concerns**
* The changes apply to both Android and iOS and we did pull out anything referring to custom UI for the splashscreen when the main URL is changed. The splashscreen being shown is always the same.
* A point could be made about the Material Design like spinner. We would like to uniform the spinner design where it is possible and make it look more modern, but for example on iOS 6 this is not a trivial change for a platform which is less and less the target of developers worldwide. Also we may still need to commit work for the Android version of the plugin for the spinner too.
* I understand your point about separating the spinner from the current PR, possibly breaking it into plugins this very plugin would be able to optonally use. Maybe broadcasting a message and expecting a new UI Element back? Will address this concern.
**iPad Pro**
I will look into a JIRA ticket for it. I will look for a default image for the cordova-ios project. We have forked it off for internal reasons so we are using a slightly modified version of cordova-ios 3.8.0. Still, I take your point and will try to address them.
**iOS 8 bugfixes**
I will look into a JIRA ticket for it. Should I still split it into a different PR here or should I include both JIRA ticket numbers in the PR?
**HTML5 boilerplate license**
Sorry, cruft I forgot to delete. Will remove it.
**Clobber change**
The thought behind the clobber was to namespace the cordova plugins, like we do for all other plugins we use internally, to a window.nativeWrapper.<pluginName> space to avoid collisions with other JavaScript libraries.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-splashscreen pull request: Added new spinner style,...
Posted by Panajev <gi...@git.apache.org>.
Github user Panajev commented on the pull request:
https://github.com/apache/cordova-plugin-splashscreen/pull/95#issuecomment-201347880
Weird. Only one of the two tests targets and for a line I have not touched:
```
In file included from /Users/travis/build/apache/cordova-plugin-splashscreen/src/ios/CDVSplashScreen.m:20:
/Users/travis/build/apache/cordova-plugin-splashscreen/src/ios/CDVSplashScreen.h:21:9: fatal error: 'Cordova/CDVPlugin.h' file not found
#import <Cordova/CDVPlugin.h>
^
1 error generated.
Testing failed:
'Cordova/CDVPlugin.h' file not found
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-splashscreen pull request: Added new spinner style,...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on the pull request:
https://github.com/apache/cordova-plugin-splashscreen/pull/95#issuecomment-203286589
@Panajev, could you please rebase and force-push your PR to check the test/build status?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org