Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for custom AppX assets and asset resolutions. #1657

Closed

Conversation

surajreddy
Copy link
Contributor

This PR includes custom the ability to add a custom assets folder for AppX builds, along with support to specify the names for the AppX assets. This is based on #987 (comment)

It leaves the default behavior untouched.

Should I add option notes for the builds in this issue, or should I be updating the wiki directly?

Note: this doesn't add support for the BadgeLogo visual asset option.

Related: electron-userland/electron-builder-binaries#3

Some Reference Docs:
https://docs.microsoft.com/en-us/uwp/schemas/appxpackage/how-to-create-a-basic-package-manifest
https://docs.microsoft.com/en-us/uwp/schemas/appxpackage/uapmanifestschema/what-s-changed-in-windows-10

cc @feens

@develar
Copy link
Member

develar commented Jun 12, 2017

Should I add option notes for the builds in this issue, or should I be updating the wiki directly?

Just run yarn api.


const customAssetsFolder = this.options.assetsFolder
if (customAssetsFolder) {
const customAssetsPath = path.resolve(this.packager.projectDir, customAssetsFolder)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom dir must be relative to build resources directory by default. If doesn't exist, relative to project dir. Hint — use method statOrNull.

@develar
Copy link
Member

develar commented Jun 12, 2017

yarn lint ;)

const destination = path.join(this.outDir, packager.expandArtifactNamePattern(this.options, "appx", arch))
const args = ["pack", "/o", "/d", preAppx, "/p", destination]
use(this.options.makeappxArgs, (it: Array<string>) => args.push(...it))
this.writeManifest(templatePath, preAppx, safeName, arch, publisher, this.options.assetNames)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If custom dir specified, we should populate missed values in the asset names. Right? Otherwise strange — why we require explicit configuration of names.

/**
* Specify a custom assets folder to use for the assets.
*/
readonly assetsFolder?: string | null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dir should be equals by default to some convenient location, e.g. appxAssets. So, user can simply put files without explicit conf.

resources folder and supporting additional AppX assets.
@surajreddy surajreddy force-pushed the custom-appx-options branch from 381f5e5 to aec1fd1 Compare June 13, 2017 19:43
@surajreddy
Copy link
Contributor Author

surajreddy commented Jun 13, 2017

@develar I've updated the PR based on our conversations and linted the changes. Let me know if there's any other improvements you would like.

I have also included readme/documentation changes below on how this works - let me know if you have any doubts or questions.

README Changes

AppX Assets Support

AppX assets need to be placed in the appxAssets folder in the buildResources folder (by default this is build).

The assets should follow these naming conventions:

  • Logo: StoreLogo.png
  • Square150x150Logo: Square150x150Logo.png
  • Square44x44Logo: Square44x44Logo.png
  • [Optional] BadgeLogo: BadgeLogo.png
  • Wide310x150Logo: Wide310x150Logo.png
  • [Optional] Square310x310Logo: LargeTile.png
  • [Optional] Square71x71Logo: SmallTile.png
  • [Optional] SplashScreen: SplashScreen.png

All official AppX asset types are supported by the build process. These assets can include scaled assets by using target size and scale in the name. See https://docs.microsoft.com/en-us/windows/uwp/controls-and-patterns/tiles-and-notifications-app-assets for more information.

Default assets will be used for Logo, Square150x150Logo, Square44x44Logo and Wide310x150Logo if not provided. For assets marked [Optional], these assets will not be listed in the manifest file if not provided.

Note: Support for assets being provided in the default build directory with the names 44x44.png, 50x50.png, 150x150.png, 310x150.png is dropped. Please move your assets to the build/appxAssets folder and name them according to the convention above.

@black-snow
Copy link
Contributor

+500 interwebs for you, looking forward to having this in electron-builder

@develar
Copy link
Member

develar commented Jun 15, 2017

Merge in process. I will change "appx_assets" to "appx" since this folder in build resources and it is clear that it is appx assets.

is dropped

In general it violates our rules — should be at least one major release before final drop, but

  • it is non-default windows target.
  • your changes are awesome and wanted.
  • usages of Appx target is not very popular.

@develar
Copy link
Member

develar commented Oct 1, 2017

@surajreddy Hi, I need your help. To avoid intermediate file copying, now file mappings is used.

Problem is that not clear for me what should be used as /ProjectRoot arg to makepri new. And no test for it. For now I changed it to userAssetDir, but could you please check that my changes doesn't break this functionality? And add missed test? I pushed changes to master. f06324a

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

Successfully merging this pull request may close these issues.

3 participants