You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by AxelNennker <gi...@git.apache.org> on 2014/06/14 21:21:11 UTC
[GitHub] cordova-lib pull request: CB-3571: support for element in...
GitHub user AxelNennker opened a pull request:
https://github.com/apache/cordova-lib/pull/30
CB-3571: support for <splash> element in config.xml on Android
This PR implements CB-3571 Add support for <splash> elements in config.xml on Android
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/AxelNennker/cordova-lib master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/30.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 #30
----
commit 19d20cd7a8956032316a710a259f29cf5103ea3e
Author: ignisvulpis <ax...@nennker.de>
Date: 2014-06-14T19:17:14Z
CB-3571: support for <splash> element in config.xml on Android
----
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by perilousleigh <gi...@git.apache.org>.
Github user perilousleigh commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-48961753
After checking the code it doesn't look like it's expecting the "land" or "port" prefixes in the density attribute. So removing the orientation prefix and just using the density values ("hdpi", "ldpi", "mdpi", "xhdpi") worked for me.
---
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.
---
Re: [GitHub] cordova-lib pull request: CB-3571: support for
element in...
Posted by Axel Nennker <ig...@gmail.com>.
I ran jshint manually on the changed files and amended ConfigParser.js
which jshint complained about.
Axel
2014-06-20 22:06 GMT+02:00 Axel Nennker <ig...@gmail.com>:
> I ran npm test but get a bunch of other errors. No idea what is wrong.
> Axel
>
> > npm run jasmine && npm run jshint
>
>
> > cordova-lib@0.21.4-dev jasmine /host/Cordova/cordova-lib/cordova-lib
> > jasmine-node --color spec-plugman spec-cordova
>
> ...........................................................................................................................................................................................................................................cp:
> copyFileSync: could not write to dest file
> (code=ENOENT):/tmp/plugman-test/android_install
>
>
> ........FFFFF.................................................................................................................................................................................................................................................................................................................................................................................
>
> Failures:
>
> 1) install success should check version if plugin has engine tag
> Message:
> Expected spy satisfies to have been called with [ '2.5.0', '>=2.3.0'
> ] but actual calls were [ null, '>=2.3.0' ], [ '0.21.4-dev', '>=0.10.0' ],
> [ '2.5.0', '>=1.0.0' ], [ '2.5.0', '>=3.0.0' ]
> Stacktrace:
> Error: Expected spy satisfies to have been called with [ '2.5.0',
> '>=2.3.0' ] but actual calls were [ null, '>=2.3.0' ], [ '0.21.4-dev',
> '>=0.10.0' ], [ '2.5.0', '>=1.0.0' ], [ '2.5.0', '>=3.0.0' ]
> at null.<anonymous>
> (/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:218:35)
> at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
>
> 2) install success should check version and munge it a little if it has
> "rc" in it so it plays nice with semver (introduce a dash in it)
> Message:
> Expected spy satisfies to have been called with [ '3.0.0-rc1',
> '>=2.3.0' ] but actual calls were [ null, '>=2.3.0' ], [ '0.21.4-dev',
> '>=0.10.0' ], [ '3.0.0-rc1', '>=1.0.0' ], [ '3.0.0-rc1', '>=3.0.0' ]
> Stacktrace:
> Error: Expected spy satisfies to have been called with [ '3.0.0-rc1',
> '>=2.3.0' ] but actual calls were [ null, '>=2.3.0' ], [ '0.21.4-dev',
> '>=0.10.0' ], [ '3.0.0-rc1', '>=1.0.0' ], [ '3.0.0-rc1', '>=3.0.0' ]
> at null.<anonymous>
> (/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:232:35)
> at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
>
> 3) install success should check specific platform version over cordova
> version if specified
> Message:
> Expected spy satisfies to have been called with [ '3.1.0', '>=3.1.0'
> ] but actual calls were [ null, '>=3.0.0' ], [ null, '>=3.1.0' ], [ null,
> '>=18' ]
> Stacktrace:
> Error: Expected spy satisfies to have been called with [ '3.1.0',
> '>=3.1.0' ] but actual calls were [ null, '>=3.0.0' ], [ null, '>=3.1.0' ],
> [ null, '>=18' ]
> at null.<anonymous>
> (/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:247:29)
> at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
>
> 4) install success should check platform sdk version if specified
> Message:
> Expected [ null, '>=3.0.0' ] to equal [ '18.0.0', '>=3.0.0' ].
> Stacktrace:
> Error: Expected [ null, '>=3.0.0' ] to equal [ '18.0.0', '>=3.0.0' ].
> at null.<anonymous>
> (/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:267:43)
> at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
>
> 5) install success should check platform sdk version if specified
> Message:
> Expected [ null, '>=3.1.0' ] to equal [ '18.0.0', '>=3.1.0' ].
> Stacktrace:
> Error: Expected [ null, '>=3.1.0' ] to equal [ '18.0.0', '>=3.1.0' ].
> at null.<anonymous>
> (/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:268:43)
> at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
>
> 6) install success should check platform sdk version if specified
> Message:
> Expected [ null, '>=18' ] to equal [ '18.0.0', '>=18' ].
> Stacktrace:
> Error: Expected [ null, '>=18' ] to equal [ '18.0.0', '>=18' ].
> at null.<anonymous>
> (/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:269:43)
> at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
>
> 7) install success should check engine versions
> Message:
> Expected [ null, '>=2.3.0' ] to equal [ '', '>=2.3.0' ].
> Stacktrace:
> Error: Expected [ null, '>=2.3.0' ] to equal [ '', '>=2.3.0' ].
> at null.<anonymous>
> (/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:289:43)
> at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
>
> Finished in 11.773 seconds
> 617 tests, 872 assertions, 7 failures, 0 skipped
>
>
> npm ERR! weird error 1
> npm ERR! not ok code 0
> npm ERR! Test failed. See above for more details.
> npm ERR! not ok code 0
> ignisvulpis@ubuntu:/host/Cordova/cordova-lib/cordova-lib$
>
>
Re: [GitHub] cordova-lib pull request: CB-3571: support for
element in...
Posted by Axel Nennker <ig...@gmail.com>.
I ran npm test but get a bunch of other errors. No idea what is wrong.
Axel
> npm run jasmine && npm run jshint
> cordova-lib@0.21.4-dev jasmine /host/Cordova/cordova-lib/cordova-lib
> jasmine-node --color spec-plugman spec-cordova
...........................................................................................................................................................................................................................................cp:
copyFileSync: could not write to dest file
(code=ENOENT):/tmp/plugman-test/android_install
........FFFFF.................................................................................................................................................................................................................................................................................................................................................................................
Failures:
1) install success should check version if plugin has engine tag
Message:
Expected spy satisfies to have been called with [ '2.5.0', '>=2.3.0' ]
but actual calls were [ null, '>=2.3.0' ], [ '0.21.4-dev', '>=0.10.0' ], [
'2.5.0', '>=1.0.0' ], [ '2.5.0', '>=3.0.0' ]
Stacktrace:
Error: Expected spy satisfies to have been called with [ '2.5.0',
'>=2.3.0' ] but actual calls were [ null, '>=2.3.0' ], [ '0.21.4-dev',
'>=0.10.0' ], [ '2.5.0', '>=1.0.0' ], [ '2.5.0', '>=3.0.0' ]
at null.<anonymous>
(/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:218:35)
at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
2) install success should check version and munge it a little if it has
"rc" in it so it plays nice with semver (introduce a dash in it)
Message:
Expected spy satisfies to have been called with [ '3.0.0-rc1',
'>=2.3.0' ] but actual calls were [ null, '>=2.3.0' ], [ '0.21.4-dev',
'>=0.10.0' ], [ '3.0.0-rc1', '>=1.0.0' ], [ '3.0.0-rc1', '>=3.0.0' ]
Stacktrace:
Error: Expected spy satisfies to have been called with [ '3.0.0-rc1',
'>=2.3.0' ] but actual calls were [ null, '>=2.3.0' ], [ '0.21.4-dev',
'>=0.10.0' ], [ '3.0.0-rc1', '>=1.0.0' ], [ '3.0.0-rc1', '>=3.0.0' ]
at null.<anonymous>
(/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:232:35)
at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
3) install success should check specific platform version over cordova
version if specified
Message:
Expected spy satisfies to have been called with [ '3.1.0', '>=3.1.0' ]
but actual calls were [ null, '>=3.0.0' ], [ null, '>=3.1.0' ], [ null,
'>=18' ]
Stacktrace:
Error: Expected spy satisfies to have been called with [ '3.1.0',
'>=3.1.0' ] but actual calls were [ null, '>=3.0.0' ], [ null, '>=3.1.0' ],
[ null, '>=18' ]
at null.<anonymous>
(/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:247:29)
at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
4) install success should check platform sdk version if specified
Message:
Expected [ null, '>=3.0.0' ] to equal [ '18.0.0', '>=3.0.0' ].
Stacktrace:
Error: Expected [ null, '>=3.0.0' ] to equal [ '18.0.0', '>=3.0.0' ].
at null.<anonymous>
(/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:267:43)
at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
5) install success should check platform sdk version if specified
Message:
Expected [ null, '>=3.1.0' ] to equal [ '18.0.0', '>=3.1.0' ].
Stacktrace:
Error: Expected [ null, '>=3.1.0' ] to equal [ '18.0.0', '>=3.1.0' ].
at null.<anonymous>
(/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:268:43)
at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
6) install success should check platform sdk version if specified
Message:
Expected [ null, '>=18' ] to equal [ '18.0.0', '>=18' ].
Stacktrace:
Error: Expected [ null, '>=18' ] to equal [ '18.0.0', '>=18' ].
at null.<anonymous>
(/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:269:43)
at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
7) install success should check engine versions
Message:
Expected [ null, '>=2.3.0' ] to equal [ '', '>=2.3.0' ].
Stacktrace:
Error: Expected [ null, '>=2.3.0' ] to equal [ '', '>=2.3.0' ].
at null.<anonymous>
(/host/Cordova/cordova-lib/cordova-lib/spec-plugman/install.spec.js:289:43)
at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
Finished in 11.773 seconds
617 tests, 872 assertions, 7 failures, 0 skipped
npm ERR! weird error 1
npm ERR! not ok code 0
npm ERR! Test failed. See above for more details.
npm ERR! not ok code 0
ignisvulpis@ubuntu:/host/Cordova/cordova-lib/cordova-lib$
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46689209
For this particular PR. Thanks for fixing the indentation.
With JSHint added yesterday, "npm test" now runs JSHint on all the files. Please run it and fix the JSHint errors, they are minor, mostly about forgotten "var"s.
If you want to disable the constraint for consistent quotation marks, use quotmark:false in JSHint config line.
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by surajpindoria <gi...@git.apache.org>.
Github user surajpindoria commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46506676
Can you add some tests for this.
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46688510
The "Merge remote..." commits are not that horrible, we have plenty of those in our repos and they have some benefits. But if you want to avoid them, try some of the below
git checkout YourBranch
git rebase master
git checkout master
git merge YourBranch
OR
git checkout master
git cherry-pick <hash-or-your-change>
The disadvantage is that it creates a new commit with (nearly) identical content but different hash. But we often do it anyway when editing a commit message with git commit --amend to add stuff like
github: clonse PRnumber
A simple and somewhat brutal way of squashing a pull request into a single commit without preserving authorship or any other info from the original commits in the PR
curl https://github.com/apache/cordova-lib/pull/30.diff | git apply
A similar way to just apply all the commits from a PR onto you current HEAD while preserving all the commit info. Similar to first rebasing and then merging.
curl https://github.com/apache/cordova-lib/pull/30.patch | git am
In some cases, if the conflicts are annoying enough, I just use a graphical diffing/merging tool (like meld) between two independent clones of the repo to copy the changes as a new commit on top of master. Especially when I need to break one commit into several (the opposite of squashing).
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by jsoref <gi...@git.apache.org>.
Github user jsoref commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46631067
Could you please do a rebase so that there isn't an ugly merge (or multiple ugly merges) in the history? I've done a number for #9 -- it makes things much nicer.
A quick check shows that while there is a conflict, it's incredibly tiny, and trivial to resolve ("pick your change").
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46172077
@AxelNennker ok, i started with WP8, Windows8 and iOS implementations, based on your work here: https://github.com/MSOpenTech/cordova-lib/tree/CB-3571.
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/30
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by AxelNennker <gi...@git.apache.org>.
Github user AxelNennker commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-48963855
@perilousleigh
On Android the default splash screen are in the land/port directories.
The example above uses the same directories.
Please add output from "cordova platform add android -d" to describe what is not working for you.
There is a new issue that "default" resources do not work in 3.5.0-0.2.6.
https://issues.apache.org/jira/browse/CB-7132
Maybe that is causing problems for you too?
https://github.com/apache/cordova-lib/pull/58
I am always using the full monty; that is: specify icons and splashes for all densities for the best look
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46170707
Here is proposal for standartized splashscreen support across platforms: https://gist.github.com/vladimir-kotikov/875a5f61a88a508d6ca9
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by AxelNennker <gi...@git.apache.org>.
Github user AxelNennker commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46165459
The current code does not remove the default cordova splash screens if there are any but there is no <splash> in config.xml
I think the default splash screens should not be there in the first place if config.xml does not indicate splash screen usage.
So if there are <splash> elements then screen.png is deleted. If <splash> is not there then nothing happens to res/drawable-*/* files.
From: Sergey Grebnov [mailto:notifications@github.com]
Sent: Monday, June 16, 2014 11:35 AM
To: apache/cordova-lib
Cc: Axel Nennker
Subject: Re: [cordova-lib] CB-3571: support for <splash> element in config.xml on Android (#30)
The usage LGTM. But does current implementation remove all default cordova splash screen images if there is no splash element defined?
Also, on Android icons and splash images definitions are very similar, but we use two different code blocks for icons and splashIcons. There is probably a way to move common logic to special function and then call it for icons and splash screens.
—
Reply to this email directly or view it on GitHub<https://github.com/apache/cordova-lib/pull/30#issuecomment-46158371>.
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by perilousleigh <gi...@git.apache.org>.
Github user perilousleigh commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-49034021
Ok my problem was that when I generated my project I was on an earlier version of the CLI and it did not generate all the land/port drawable directories. So because the copying of the splash files is based on what directories are in existence it was never finding those elements specified with the "land" or "port" prefix in the density attribute.
I'm all set now, thanks!
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46166927
@AxelNennker, thx for the clarification!
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by rohitagg28 <gi...@git.apache.org>.
Github user rohitagg28 commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46936636
I have remove this line <preference name="SplashScreen" value="screen" /> from config.xml file and added these to it.
<platform name="android">
<splash src="res/screen/android/splash-land-hdpi.png" density="land-hdpi"/>
<splash src="res/screen/android/splash-land-ldpi.png" density="land-ldpi"/>
<splash src="res/screen/android/splash-land-mdpi.png" density="land-mdpi"/>
<splash src="res/screen/android/splash-land-xhdpi.png" density="land-xhdpi"/>
<splash src="res/screen/android/splash-port-hdpi.png" density="port-hdpi"/>
<splash src="res/screen/android/splash-port-ldpi.png" density="port-ldpi"/>
<splash src="res/screen/android/splash-port-mdpi.png" density="port-mdpi"/>
<splash src="res/screen/android/splash-port-xhdpi.png" density="port-xhdpi"/>
</platform>
So here I have res>screen>android folder with respect to project folder and into which images are placed. But after running this code I have still not able to see any of these images as splashscreen.
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46646255
Could we also incorporate the following PR to this one before merge? It adds splash images support for iOS, WP8 and Windows8.
https://github.com/AxelNennker/cordova-lib/pull/1
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/30#discussion_r13974255
--- Diff: cordova-lib/src/cordova/metadata/android_parser.js ---
@@ -88,13 +88,54 @@ module.exports.prototype = {
fs.writeFileSync(this.strings, strings.write({indent: 4}), 'utf-8');
events.emit('verbose', 'Wrote out Android application name to "' + name + '"');
+ var projectRoot = util.isCordova(this.path);
+
+ var splashIcons = config.getIcons('android', 'splash');
+ // if there are icon elements in config.xml
+ if (splashIcons) {
+ events.emit('verbose', "splash icons: " + JSON.stringify(splashIcons));
--- End diff --
This line seems to be indented with 3 spaces. Please use consistent 4 space indentation.
A suggestion: the whole splashScreen section seems to be long enough to live in a function of its own. Preferably in such a way so that it can be shared with Amazon FireOS parser, or whatever Android derivative wants to use it.
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by AxelNennker <gi...@git.apache.org>.
Github user AxelNennker commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46670164
@jsoref How do I do the rebase to get rid of "Merge remote-tracking branch 'upstream/master'" commits.
I wrecked my other repo (for cordova-cli) so I am cautious.
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by AxelNennker <gi...@git.apache.org>.
Github user AxelNennker commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46097179
Tested with this config.xml
<platform name="android">
<splash src="../img/splash-land-hdpi.png" density="land-hdpi"/>
<splash src="../img/splash-land-ldpi.png" density="land-ldpi"/>
<splash src="../img/splash-land-mdpi.png" density="land-mdpi"/>
<splash src="../img/splash-land-xhdpi.png" density="land-xhdpi"/>
<splash src="../img/splash-port-hdpi.png" density="port-hdpi"/>
<splash src="../img/splash-port-ldpi.png" density="port-ldpi"/>
<splash src="../img/splash-port-mdpi.png" density="port-mdpi"/>
<splash src="../img/splash-port-xhdpi.png" density="port-xhdpi"/>
</platform>
The paths to the PNGs are relative to the project_dir NOT relative to www.
Axel
---
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.
---
[GitHub] cordova-lib pull request: CB-3571: support for element in...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-lib/pull/30#issuecomment-46158371
The usage LGTM. But does current implementation remove all default cordova splash screen images if there is no splash element defined?
Also, on Android icons and splash images definitions are very similar, but we use two different code blocks for icons and splashIcons. There is probably a way to move common logic to special function and then call it for icons and splash screens.
---
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.
---