Nightly build #241 for cordova has succeeded!

[Apache Jenkins Server]

Nightly build #241 for cordova has succeeded!
The latest nightly has been published and you can try it out with 'npm i -g 
cordova@nightly'

For details check build console at 
https://builds.apache.org/job/cordova-nightly/241/consoleFull

-
Jenkins for Apache Cordova

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-android issue #349: CB-12169: Check for build directory before runni...

[codecov-io]

Github user codecov-io commented on the issue:

https://github.com/apache/cordova-android/pull/349
  
## [Current 
coverage](https://codecov.io/gh/apache/cordova-android/pull/349?src=pr) is 
35.58% (diff: 50.00%)
> Merging 
[#349](https://codecov.io/gh/apache/cordova-android/pull/349?src=pr) into 
[master](https://codecov.io/gh/apache/cordova-android/branch/master?src=pr) 
will decrease coverage by **<.01%**

```diff
@@ master   #349   diff @@
==
  Files12 12  
  Lines  1034   1037 +3   
  Methods 204205 +1   
  Messages  0  0  
  Branches173173  
==
+ Hits368369 +1   
- Misses  666668 +2   
  Partials  0  0  
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update 
[3bfeda4...073d554](https://codecov.io/gh/apache/cordova-android/compare/3bfeda4a3b9e323c5f329b304fa1480d4fcb58e4...073d554f7f5928a14c7a51deec8b9fd345a1a671?src=pr)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib issue #468: CB-8978 Add resource-file parsing to config.xml

[dpogue]

Github user dpogue commented on the issue:

https://github.com/apache/cordova-lib/pull/468
  
Any other input here? Are we in agreement to remove the top-level file 
copying?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Re: Hello again!

[Jesse]

Welcome back!


@purplecabbage
risingj.com

On Mon, Nov 28, 2016 at 11:50 AM, Simon MacDonald  wrote:

> Never heard of this guy.
> Simon Mac Donald
> http://simonmacdonald.com
>
>
> On Mon, Nov 28, 2016 at 2:48 PM, Filip Maj  wrote:
> > Hi everyone!
> >
> > Just wanted to (re)introduce myself after a 3 year or so hiatus :)
> >
> > I used to be an active member of the group between 2011 and 2013 when
> > I was on the Adobe PhoneGap team. I took a 3 year detour focusing on
> > mobile testing infrastructure at Sauce Labs, but recently rejoined the
> > Adobe PhoneGap team. I have been lurking more intently on this list
> > for the past month or so and aim to be more involved these days.
> >
> > I've been poking around and getting my bearings around the testing
> > suites, infrastructure and CI in Cordova the past week or so. I think
> > I will try to contribute in that area initially. In particular, I am
> > interested in enabling functional end-to-end testing for all repos in
> > cordova that could benefit from that sort of testing, and seamlessly
> > integrating running the tests and reporting their results back into
> > the standard Cordova dev workflow (I assume that is focussed around
> > GitHub?). I see there are different kinds of test coverage and CI
> > systems at play (cloudapp, travis, appveyor, plus unit and functional
> > tests), so initially just wrapping my head around all that.
> >
> > If anyone here has suggestions on areas that need work, have
> > grievances around how they are frustrated by manually needing to test
> > something, or having any other helpful tips on what needs work or what
> > could be improved, feel free to reply to this thread!
> >
> > My generic notes on this topic so far, in case that is helpful:
> >
> > cordova testing overview
> > ———
> > notes / weird things:
> >  - cordova-paramedic configs are pulled from cordova-medic repo. (?)
> > requires an extra pull in CI.
> >  - paramedic setup for individual plugins install latest HEAD of
> > master of platform code (at least, cordova-android + device plugin)
> >  - there are plugins tests that run via a jenkins instance on
> > cloudapp.net, and there are travis tests too. travis is pull-req
> > triggered, cloud app runs nightly. why?
> >
> > road to testing utopia:
> >  - how do platforms get tested? integration tests with what: tooling?
> plugins?
> >- unit tests run on travis/appveyor?
> >- understand what needs to be tested for a release. work backwards
> > to automate that from there. Steve sent some helpful links my way:
> >  - platform:
> > https://github.com/apache/cordova-coho/blob/master/docs/
> platforms-release-process.md#testing
> >  - plugins:
> > https://github.com/apache/cordova-coho/blob/master/docs/
> plugins-release-process.md#test
> >  - tools: https://github.com/apache/cordova-coho/blob/master/docs/
> tools-release-process.md#test
> >  - how do plugins get tested?
> >- make sure dependencies / artifact versions are locked down.
> >- what is the difference between "local" vs appium tests
> >
> > Looking forward to collaborating with y'all in here once more :)
> >
> > Cheers,
> > Fil
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
> > For additional commands, e-mail: dev-h...@cordova.apache.org
> >
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
> For additional commands, e-mail: dev-h...@cordova.apache.org
>
>

Re: Hello again!

[Simon MacDonald]

Never heard of this guy.
Simon Mac Donald
http://simonmacdonald.com


On Mon, Nov 28, 2016 at 2:48 PM, Filip Maj  wrote:
> Hi everyone!
>
> Just wanted to (re)introduce myself after a 3 year or so hiatus :)
>
> I used to be an active member of the group between 2011 and 2013 when
> I was on the Adobe PhoneGap team. I took a 3 year detour focusing on
> mobile testing infrastructure at Sauce Labs, but recently rejoined the
> Adobe PhoneGap team. I have been lurking more intently on this list
> for the past month or so and aim to be more involved these days.
>
> I've been poking around and getting my bearings around the testing
> suites, infrastructure and CI in Cordova the past week or so. I think
> I will try to contribute in that area initially. In particular, I am
> interested in enabling functional end-to-end testing for all repos in
> cordova that could benefit from that sort of testing, and seamlessly
> integrating running the tests and reporting their results back into
> the standard Cordova dev workflow (I assume that is focussed around
> GitHub?). I see there are different kinds of test coverage and CI
> systems at play (cloudapp, travis, appveyor, plus unit and functional
> tests), so initially just wrapping my head around all that.
>
> If anyone here has suggestions on areas that need work, have
> grievances around how they are frustrated by manually needing to test
> something, or having any other helpful tips on what needs work or what
> could be improved, feel free to reply to this thread!
>
> My generic notes on this topic so far, in case that is helpful:
>
> cordova testing overview
> ———
> notes / weird things:
>  - cordova-paramedic configs are pulled from cordova-medic repo. (?)
> requires an extra pull in CI.
>  - paramedic setup for individual plugins install latest HEAD of
> master of platform code (at least, cordova-android + device plugin)
>  - there are plugins tests that run via a jenkins instance on
> cloudapp.net, and there are travis tests too. travis is pull-req
> triggered, cloud app runs nightly. why?
>
> road to testing utopia:
>  - how do platforms get tested? integration tests with what: tooling? plugins?
>- unit tests run on travis/appveyor?
>- understand what needs to be tested for a release. work backwards
> to automate that from there. Steve sent some helpful links my way:
>  - platform:
> https://github.com/apache/cordova-coho/blob/master/docs/platforms-release-process.md#testing
>  - plugins:
> https://github.com/apache/cordova-coho/blob/master/docs/plugins-release-process.md#test
>  - tools: 
> https://github.com/apache/cordova-coho/blob/master/docs/tools-release-process.md#test
>  - how do plugins get tested?
>- make sure dependencies / artifact versions are locked down.
>- what is the difference between "local" vs appium tests
>
> Looking forward to collaborating with y'all in here once more :)
>
> Cheers,
> Fil
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
> For additional commands, e-mail: dev-h...@cordova.apache.org
>

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Hello again!

[Filip Maj]

Hi everyone!

Just wanted to (re)introduce myself after a 3 year or so hiatus :)

I used to be an active member of the group between 2011 and 2013 when
I was on the Adobe PhoneGap team. I took a 3 year detour focusing on
mobile testing infrastructure at Sauce Labs, but recently rejoined the
Adobe PhoneGap team. I have been lurking more intently on this list
for the past month or so and aim to be more involved these days.

I've been poking around and getting my bearings around the testing
suites, infrastructure and CI in Cordova the past week or so. I think
I will try to contribute in that area initially. In particular, I am
interested in enabling functional end-to-end testing for all repos in
cordova that could benefit from that sort of testing, and seamlessly
integrating running the tests and reporting their results back into
the standard Cordova dev workflow (I assume that is focussed around
GitHub?). I see there are different kinds of test coverage and CI
systems at play (cloudapp, travis, appveyor, plus unit and functional
tests), so initially just wrapping my head around all that.

If anyone here has suggestions on areas that need work, have
grievances around how they are frustrated by manually needing to test
something, or having any other helpful tips on what needs work or what
could be improved, feel free to reply to this thread!

My generic notes on this topic so far, in case that is helpful:

cordova testing overview
———
notes / weird things:
 - cordova-paramedic configs are pulled from cordova-medic repo. (?)
requires an extra pull in CI.
 - paramedic setup for individual plugins install latest HEAD of
master of platform code (at least, cordova-android + device plugin)
 - there are plugins tests that run via a jenkins instance on
cloudapp.net, and there are travis tests too. travis is pull-req
triggered, cloud app runs nightly. why?

road to testing utopia:
 - how do platforms get tested? integration tests with what: tooling? plugins?
   - unit tests run on travis/appveyor?
   - understand what needs to be tested for a release. work backwards
to automate that from there. Steve sent some helpful links my way:
 - platform:
https://github.com/apache/cordova-coho/blob/master/docs/platforms-release-process.md#testing
 - plugins:
https://github.com/apache/cordova-coho/blob/master/docs/plugins-release-process.md#test
 - tools: 
https://github.com/apache/cordova-coho/blob/master/docs/tools-release-process.md#test
 - how do plugins get tested?
   - make sure dependencies / artifact versions are locked down.
   - what is the difference between "local" vs appium tests

Looking forward to collaborating with y'all in here once more :)

Cheers,
Fil

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request #:

[audreyso]

Github user audreyso commented on the pull request:


https://github.com/apache/cordova-lib/commit/fbedd9e67f5a631404079b28e3def489cd07b129#commitcomment-19987410
  
In cordova-lib/src/cordova/platform.js:
In cordova-lib/src/cordova/platform.js on line 121:
hey @stevengill ... can you leave feedback for this line? not sure if my 
"if" statement logic is right. wanted to check!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request #:

[audreyso]

Github user audreyso commented on the pull request:


https://github.com/apache/cordova-lib/commit/fbedd9e67f5a631404079b28e3def489cd07b129#commitcomment-19987344
  
In cordova-lib/spec-cordova/pkgJson.spec.js:
In cordova-lib/spec-cordova/pkgJson.spec.js on line 28:
fixed this typo


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request #:

[audreyso]

Github user audreyso commented on the pull request:


https://github.com/apache/cordova-lib/commit/fbedd9e67f5a631404079b28e3def489cd07b129#commitcomment-19987342
  
In cordova-lib/spec-cordova/pkgJson.spec.js:
In cordova-lib/spec-cordova/pkgJson.spec.js on line 228:
fixed this typo


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #212: CB-9287 Not enough Icons and Splashscreens for W...

[daserge]

Github user daserge commented on the issue:

https://github.com/apache/cordova-windows/pull/212
  
@vladimir-kotikov thank you for the review!
I've addressed the notes.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[daserge]

Github user daserge commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89792140
  
--- Diff: cordova-js-src/splashscreen.js ---
@@ -58,8 +58,45 @@ function readBoolFromCfg(preferenceName, defaultValue, 
cfg) {
 }
 }
 
+// Refine splashscreen file extension to match what is in config.xml
+function refineSplashScreenExtension(cfg) {
--- End diff --

We could go that way and add another method in confighelper like 
`readManifest` but:
1. I'm not sure how it will affect performance (confighelper is based on 
async xhr),
2. Then we'll need to manipulate the Image.src anyway as it is in different 
format.

The benefit of this approach is more confidence that we are using the 
correct (base)name - but it is not an issue as it is hardcoded now.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[daserge]

Github user daserge commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89789288
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -353,6 +391,13 @@ function mapImageResources(images, imagesDir) {
 events.emit('warn', 'No images found for target: ' + 
img.target);
 } else {
 candidates.forEach(function(mrtImage) {
+if (img.target === SPLASH_SCREEN_DESKTOP_TARGET_NAME &&
+mrtSplashScreenToTargetProject(mrtImage) === 
TARGET_PROJECT_10 &&
+exceedsSizeLimit(mrtImage.path)) {
--- End diff --

This does not result in a build failure and the issue could only be seen in 
Visual Studio manifest designer so I believe the warning is useful here.
Do you mean we should fail the build rather than showing this warning?
It currently just skips oversized images.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[daserge]

Github user daserge commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89770853
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
+ * @param targetName 'SplashScreen'|'SplashScreenPhone'
+ * @returns boolean */ 
+function splashScreenTargetIs(splash, targetName) {
+var matchesTarget, targetImg;
+
+if (splash.target) {
+// MRT syntax:
+matchesTarget = splash.target === targetName;
+} else {
+// Fall back on find by size for old non-MRT syntax:
+targetImg = findPlatformImage(splash.width, splash.height);
+matchesTarget = 
splashScreenTargetProjectMatchesTargetName(targetImg.targetProject, targetName);
+}
+
+return matchesTarget;
+}
+
+// Updates manifests to match the app splash screen image types 
(PNG/JPG/JPEG)
+function updateSplashScreenImageExtensions(cordovaProject, locations) {
+
+// Saving all extensions used for targets to verify them later
+var extensionsUsed = {};
+
+function checkThatExtensionsAreNotMixed() {
+for (var target in extensionsUsed) {
+/*jshint loopfunc: true */
+if (extensionsUsed.hasOwnProperty(target)) {
+var extensionsUsedForTarget = extensionsUsed[target];
+
+// Check that extensions are not mixed:
+if (extensionsUsedForTarget.length > 1 && 
extensionsUsedForTarget.some(function(item) {
+return item !== extensionsUsedForTarget[0];
+})) {
+events.emit('warn', '"' + target + '" splash screens 
have mixed file extensions which is not supported. Some of the images will not 
be used.');
+}
+}
+}
+}
+
+function checkTargetMatchAndUpdateUsedExtensions(img, target) {
+var matchesTarget = splashScreenTargetIs(img, target);
+
+if (matchesTarget === true) {
+extensionsUsed[target] = extensionsUsed[target] || [];
+
extensionsUsed[target].push(path.extname(img.src.toLowerCase()));
+}
+
+return matchesTarget;
+}
+
+function updateSplashExtensionInManifest(manifestFileName, 
splashScreen) {
+var manifest = AppxManifest.get(path.join(locations.root, 
manifestFileName));
+var newExtension = path.extname(splashScreen.src);
+
+if (manifest.getVisualElements().getSplashScreenExtension() !== 
newExtension) {
+events.emit('verbose', 'Set ' + manifestFileName + ' 
SplashScreen image extension to "' + newExtension + '"');
+
manifest.getVisualElements().setSplashScreenExtension(newExtension);
+manifest.write();
+}
+}
+
+var splashScreens = 
cordovaProject.projectConfig.getSplashScreens('windows');
+
+var desktopSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_DESKTOP_TARGET_NAME);
+})[0];
+
+var phoneSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_PHONE_TARGET_NAME);
+})[0];
+
+checkThatExtensionsAreNotMixed();
--- End diff --

```xml


```

You should not mix image extensions for non-MRT syntax inside one project 
group - in this case ^ the second JPG image will not be used as AppxManifest 
will have PNG for SplashScreen.Image' extension.
There is also a [test for this 
case](https://github.com/apache/cordova-windows/pull/212/files#diff-954e5996601fc5ce41f4107af4f31525R620).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[daserge]

Github user daserge commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89769817
  
--- Diff: template/CordovaApp.Phone.jsproj ---
@@ -79,7 +79,9 @@
 
   Designer
 
-
+
--- End diff --

We are excluding images named like `SplashScreen.jpg` or 
`SplashScreen.scale-100.png` here for phone project (we can do this taking into 
account that phone and desktop projects have different hardcoded splashscreen 
names).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89729105
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -353,6 +391,13 @@ function mapImageResources(images, imagesDir) {
 events.emit('warn', 'No images found for target: ' + 
img.target);
 } else {
 candidates.forEach(function(mrtImage) {
+if (img.target === SPLASH_SCREEN_DESKTOP_TARGET_NAME &&
+mrtSplashScreenToTargetProject(mrtImage) === 
TARGET_PROJECT_10 &&
+exceedsSizeLimit(mrtImage.path)) {
--- End diff --

Are you sure that we need to handle this case for the user? I'd rather the 
build fail to clearly indicate to user that spash image must be updated.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89726640
  
--- Diff: spec/unit/Prepare.Win10.spec.js ---
@@ -541,4 +541,165 @@ describe('copyIcons method', function () {
 
expect(FileUpdater.updatePaths).toHaveBeenCalledWith(expectedPathMap, { 
rootDir: PROJECT }, logFileOp);
 });
 });
+
+it('should ignore splashScreens for Windows 10 project with size >200K 
and emit a warning', function () {
+var size300K = 300 * 1024;
+var warnSpy = jasmine.createSpy('warn');
+events.on('warn', warnSpy);
+
+var splashScreensFiles = [
--- End diff --

Nit: this could be calculated from `splashScreens`: 
`splashScreens.map(splash => path.basename(splash.src)`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89726245
  
--- Diff: cordova-js-src/splashscreen.js ---
@@ -58,8 +58,45 @@ function readBoolFromCfg(preferenceName, defaultValue, 
cfg) {
 }
 }
 
+// Refine splashscreen file extension to match what is in config.xml
+function refineSplashScreenExtension(cfg) {
--- End diff --

Just a suggestion - is it possible to read splashscreen file name and 
extension directly from appxmanifest? This way you could avoid all this tricky 
logic for getting the right filename.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89729119
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -362,7 +407,13 @@ function mapImageResources(images, imagesDir) {
 // find target image by size
 var targetImg = findPlatformImage(img.width, img.height);
 if (targetImg) {
-var targetPath = path.join(imagesDir, targetImg.dest);
+if (targetImg.targetProject === TARGET_PROJECT_10 &&
--- End diff --

Same as above


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89729302
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
--- End diff --

Nit: this is not a valid syntax for multiline JSDoc comments, please update


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89727836
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -297,48 +305,78 @@ function applyNavigationWhitelist(config, manifest) {
 manifest.getApplication().setAccessRules(whitelistRules);
 }
 
-function mapImageResources(images, imagesDir) {
-var pathMap = {};
+// Platform default images
+var platformImages = [
+{dest: 'Square150x150Logo.scale-100', width: 150, height: 150},
+{dest: 'Square30x30Logo.scale-100', width: 30, height: 30},
+{dest: 'StoreLogo.scale-100', width: 50, height: 50},
+{dest: 'SplashScreen.scale-100', width: 620, height: 300, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-125', width: 775, height: 375, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-140', width: 868, height: 420, 
targetProject: TARGET_PROJECT_81},
+{dest: 'SplashScreen.scale-150', width: 930, height: 450, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-180', width: 1116, height: 540, 
targetProject: TARGET_PROJECT_81},
+{dest: 'SplashScreen.scale-200', width: 1240, height: 600, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-400', width: 2480, height: 1200, 
targetProject: TARGET_PROJECT_10},
+// scaled images are specified here for backward compatibility only so 
we can find them by size
+{dest: 'StoreLogo.scale-240', width: 120, height: 120},
+{dest: 'Square44x44Logo.scale-100', width: 44, height: 44},
+{dest: 'Square44x44Logo.scale-240', width: 106, height: 106},
+{dest: 'Square70x70Logo.scale-100', width: 70, height: 70},
+{dest: 'Square71x71Logo.scale-100', width: 71, height: 71},
+{dest: 'Square71x71Logo.scale-240', width: 170, height: 170},
+{dest: 'Square150x150Logo.scale-240', width: 360, height: 360},
+{dest: 'Square310x310Logo.scale-100', width: 310, height: 310},
+{dest: 'Wide310x150Logo.scale-100', width: 310, height: 150},
+{dest: 'Wide310x150Logo.scale-240', width: 744, height: 360},
+{dest: 'SplashScreenPhone.scale-100', width: 480, height: 800, 
targetProject: TARGET_PROJECT_WP81},
+{dest: 'SplashScreenPhone.scale-140', width: 672, height: 1120, 
targetProject: TARGET_PROJECT_WP81},
+{dest: 'SplashScreenPhone.scale-240', width: 1152, height: 1920, 
targetProject: TARGET_PROJECT_WP81}
+];
+
+function findPlatformImage(width, height) {
+if (!width && !height){
+// this could be default image,
+// Windows requires specific image dimension so we can't apply it
+return null;
+}
+for (var idx in platformImages){
+var res = platformImages[idx];
+// If only one of width or height is not specified, use another 
parameter for comparation
+// If both specified, compare both.
+if ((!width || (width == res.width)) &&
+(!height || (height == res.height))){
+return res;
+}
+}
+return null;
+}
 
-// Platform default images
-var platformImages = [
-{dest: 'Square150x150Logo.scale-100.png', width: 150, height: 150},
-{dest: 'Square30x30Logo.scale-100.png', width: 30, height: 30},
-{dest: 'StoreLogo.scale-100.png', width: 50, height: 50},
-{dest: 'SplashScreen.scale-100.png', width: 620, height: 300},
-// scaled images are specified here for backward compatibility 
only so we can find them by size
-{dest: 'StoreLogo.scale-240.png', width: 120, height: 120},
-{dest: 'Square44x44Logo.scale-100.png', width: 44, height: 44},
-{dest: 'Square44x44Logo.scale-240.png', width: 106, height: 106},
-{dest: 'Square70x70Logo.scale-100.png', width: 70, height: 70},
-{dest: 'Square71x71Logo.scale-100.png', width: 71, height: 71},
-{dest: 'Square71x71Logo.scale-240.png', width: 170, height: 170},
-{dest: 'Square150x150Logo.scale-240.png', width: 360, height: 360},
-{dest: 'Square310x310Logo.scale-100.png', width: 310, height: 310},
-{dest: 'Wide310x150Logo.scale-100.png', width: 310, height: 150},
-{dest: 'Wide310x150Logo.scale-240.png', width: 744, height: 360},
-{dest: 'SplashScreenPhone.scale-240.png', width: 1152, height: 
1920}
-];
+/** Maps MRT splashscreen image to its target project defined in 
platformImages -> 8.1|WP8.1|10
+ * This assumes we have different 
--- End diff --

Nit: incomplete comment


---
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, 

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89726251
  
--- Diff: cordova-js-src/confighelper.js ---
@@ -22,16 +22,17 @@
 // config.xml wrapper (non-node ConfigParser analogue)
 var config;
 function Config(xhr) {
-function loadPreferences(xhr) {
+function load(xhr, tagName) {
 var parser = new DOMParser();
 var doc = parser.parseFromString(xhr.responseText, 
"application/xml");
 
-var preferences = doc.getElementsByTagName("preference");
+var preferences = doc.getElementsByTagName(tagName);
 return Array.prototype.slice.call(preferences);
 }
 
 this.xhr = xhr;
-this.preferences = loadPreferences(this.xhr);
+this.preferences = load(this.xhr, "preference");
+this.splashScreens = load(this.xhr, "splash");
--- End diff --

Calling `load` second time here will trigger re-parsing of `config.xml` 
which, I believe, is not very efficient. Might be better to rework `load` 
method to reuse already parsed document


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89729637
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
+ * @param targetName 'SplashScreen'|'SplashScreenPhone'
+ * @returns boolean */ 
+function splashScreenTargetIs(splash, targetName) {
+var matchesTarget, targetImg;
+
+if (splash.target) {
+// MRT syntax:
+matchesTarget = splash.target === targetName;
--- End diff --

Nit: `return splash.target === targetName` is the same


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89739598
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
+ * @param targetName 'SplashScreen'|'SplashScreenPhone'
+ * @returns boolean */ 
+function splashScreenTargetIs(splash, targetName) {
+var matchesTarget, targetImg;
+
+if (splash.target) {
+// MRT syntax:
+matchesTarget = splash.target === targetName;
+} else {
+// Fall back on find by size for old non-MRT syntax:
+targetImg = findPlatformImage(splash.width, splash.height);
+matchesTarget = 
splashScreenTargetProjectMatchesTargetName(targetImg.targetProject, targetName);
+}
+
+return matchesTarget;
+}
+
+// Updates manifests to match the app splash screen image types 
(PNG/JPG/JPEG)
+function updateSplashScreenImageExtensions(cordovaProject, locations) {
+
+// Saving all extensions used for targets to verify them later
+var extensionsUsed = {};
+
+function checkThatExtensionsAreNotMixed() {
+for (var target in extensionsUsed) {
+/*jshint loopfunc: true */
+if (extensionsUsed.hasOwnProperty(target)) {
+var extensionsUsedForTarget = extensionsUsed[target];
+
+// Check that extensions are not mixed:
+if (extensionsUsedForTarget.length > 1 && 
extensionsUsedForTarget.some(function(item) {
+return item !== extensionsUsedForTarget[0];
+})) {
+events.emit('warn', '"' + target + '" splash screens 
have mixed file extensions which is not supported. Some of the images will not 
be used.');
+}
+}
+}
+}
+
+function checkTargetMatchAndUpdateUsedExtensions(img, target) {
+var matchesTarget = splashScreenTargetIs(img, target);
+
+if (matchesTarget === true) {
+extensionsUsed[target] = extensionsUsed[target] || [];
+
extensionsUsed[target].push(path.extname(img.src.toLowerCase()));
+}
+
+return matchesTarget;
+}
+
+function updateSplashExtensionInManifest(manifestFileName, 
splashScreen) {
+var manifest = AppxManifest.get(path.join(locations.root, 
manifestFileName));
+var newExtension = path.extname(splashScreen.src);
+
+if (manifest.getVisualElements().getSplashScreenExtension() !== 
newExtension) {
+events.emit('verbose', 'Set ' + manifestFileName + ' 
SplashScreen image extension to "' + newExtension + '"');
+
manifest.getVisualElements().setSplashScreenExtension(newExtension);
+manifest.write();
+}
+}
+
+var splashScreens = 
cordovaProject.projectConfig.getSplashScreens('windows');
+
+var desktopSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_DESKTOP_TARGET_NAME);
+})[0];
+
+var phoneSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_PHONE_TARGET_NAME);
+})[0];
+
+checkThatExtensionsAreNotMixed();
--- End diff --

Could you please explain, what's the purpose of this check? Please give a 
couple of examples when it would yield a warning?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89727040
  
--- Diff: template/CordovaApp.Phone.jsproj ---
@@ -79,7 +79,9 @@
 
   Designer
 
-
+
--- End diff --

Could you please explain how does this work?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89739874
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
+ * @param targetName 'SplashScreen'|'SplashScreenPhone'
+ * @returns boolean */ 
+function splashScreenTargetIs(splash, targetName) {
+var matchesTarget, targetImg;
+
+if (splash.target) {
+// MRT syntax:
+matchesTarget = splash.target === targetName;
+} else {
+// Fall back on find by size for old non-MRT syntax:
+targetImg = findPlatformImage(splash.width, splash.height);
+matchesTarget = 
splashScreenTargetProjectMatchesTargetName(targetImg.targetProject, targetName);
+}
+
+return matchesTarget;
+}
+
+// Updates manifests to match the app splash screen image types 
(PNG/JPG/JPEG)
+function updateSplashScreenImageExtensions(cordovaProject, locations) {
+
+// Saving all extensions used for targets to verify them later
+var extensionsUsed = {};
+
+function checkThatExtensionsAreNotMixed() {
+for (var target in extensionsUsed) {
+/*jshint loopfunc: true */
+if (extensionsUsed.hasOwnProperty(target)) {
+var extensionsUsedForTarget = extensionsUsed[target];
+
+// Check that extensions are not mixed:
+if (extensionsUsedForTarget.length > 1 && 
extensionsUsedForTarget.some(function(item) {
+return item !== extensionsUsedForTarget[0];
+})) {
+events.emit('warn', '"' + target + '" splash screens 
have mixed file extensions which is not supported. Some of the images will not 
be used.');
+}
+}
+}
+}
+
+function checkTargetMatchAndUpdateUsedExtensions(img, target) {
+var matchesTarget = splashScreenTargetIs(img, target);
+
+if (matchesTarget === true) {
+extensionsUsed[target] = extensionsUsed[target] || [];
+
extensionsUsed[target].push(path.extname(img.src.toLowerCase()));
+}
+
+return matchesTarget;
+}
+
+function updateSplashExtensionInManifest(manifestFileName, 
splashScreen) {
+var manifest = AppxManifest.get(path.join(locations.root, 
manifestFileName));
+var newExtension = path.extname(splashScreen.src);
+
+if (manifest.getVisualElements().getSplashScreenExtension() !== 
newExtension) {
+events.emit('verbose', 'Set ' + manifestFileName + ' 
SplashScreen image extension to "' + newExtension + '"');
+
manifest.getVisualElements().setSplashScreenExtension(newExtension);
+manifest.write();
+}
+}
+
+var splashScreens = 
cordovaProject.projectConfig.getSplashScreens('windows');
+
+var desktopSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_DESKTOP_TARGET_NAME);
+})[0];
+
+var phoneSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_PHONE_TARGET_NAME);
+})[0];
+
+checkThatExtensionsAreNotMixed();
+
+var manifestSplashScreenMap = [{
--- End diff --

Nit: IMO dictionaly is a better candidate here than array of dictionaries 
w/ 2 keys each


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org