Chore: Migrating from using the cordova-res package to using the capacitor-assets package for asset generation #2364

kwAsant · 2024-08-13T15:37:05Z

PR Checklist

PR title descriptive (can be used in release notes)

Description

The capacitor-assets library was added to package.json using yarn.
Added commands for generating splash images and icons within android.ts and android.workflow.ts for Android.
Added commands for generating splash images and icons within ios.ts and ios.workflow.ts for iOS
Due to the nature of the capacitor-assets package, splash image and icon generation can co-occur using new commands.
Updated the config.ts and deployment.model.ts files to incorporate an asset path for both android and ios.
Updated ios.ts and android.ts has been enabled to support legacy.
It is important to note that the command, yarn workplace android configure must be used before running the new commands.

Git Issues

Closes #2072

Screenshots/Videos

It is recommended that you watch these videos in full-screen

Android:

android.yarn.commands.mov

iOS:

ios.yarn.commands.mov

…ing the ➤ YN0000: ┌ Resolution step ➤ YN0000: └ Completed in 0s 259ms ➤ YN0000: ┌ Fetch step ➤ YN0000: └ Completed in 0s 607ms ➤ YN0000: ┌ Link step ➤ YN0000: └ Completed in 0s 769ms ➤ YN0000: Done in 1s 717ms command. Solved by chnaging the to that of the default that the terminal runs from (which is the directory).

Updated to allow for new to be created. Changed commands in to allow for iOS app launcher icons and splash screens to be generated. Changed commands in to allow for Android app launcher icons and splash screens to be generated with updated and legacy methods.

…the and .

jfmcquade

Good job figuring this out. I've added quite a few comments (mostly minor) inline.

Also, you should remove the now unused cordova-res package via yarn remove, and delete any imports that may still exist.

packages/data-models/deployment.model.ts

packages/scripts/src/tasks/providers/android.ts

jfmcquade · 2024-08-16T15:51:08Z

packages/scripts/src/tasks/providers/ios.ts

+  const relativeAssetPath = path.relative(process.cwd(), assetPath).replace(/^(\.\.\/|\.\/)+/, "");
+
+  const cmd = `npx @capacitor/assets generate --assetPath ${relativeAssetPath} --ios`;
+  const cwd = process.cwd().replace("/packages/scripts", ""); // output will be: "/../../idems/open-app-builder/packages/scripts"


nit: Rather than doing a string replace, it would probably be better to use the path npm library to get the correct path – path.relative(from, to) might be what you need

I have found out that only relative paths work for the npx command, so I am attempting to find methods of converting the path to a relative one (without the /../..)

With research ( and help with ChatGPT ), this may be the best way to get rid of leading dots in a directory.

OK. Line 68 is effectively just getting the relative path to the root directory, right? I think you can delete this line and instead import ROOT_DIR from packages/scripts/src/paths.ts (look at the file changes for this PR, you'll see that packages/scripts/src/tasks/providers/android.ts used to import ROOT_DIR). Then you should be able to just pass this value to the execSync() function below, like:

execSync(cmd, { stdio: "inherit", cwd: ROOT_DIR });

See also comment on line 65

I have made these changes, and they function the same.

packages/scripts/src/tasks/providers/android.ts

jfmcquade · 2024-08-16T15:58:10Z

packages/scripts/src/tasks/providers/ios.ts

+    });
+  }
+
+  const relativeAssetPath = path.relative(process.cwd(), assetPath).replace(/^(\.\.\/|\.\/)+/, "");


is there a nicer way to do this, without the .replace() and regex? Potentially an existing method of path? It could be worth thinking if this whole logic of manipulating the path and then then executing the command relative to the cwd could be simplified (I'm not saying that it definitely can, but it's quite hard to follow currently)

See comment on line 68 below. I think this line could also use the ROOT_DIR value instead of process.cwd(), and avoid doing the regex replacement. Try logging out ROOT_DIR and assetPath at this point, and see if there's a method(s) of path that could be used to combine them and give the output you want

Made the change here too.

packages/workflows/src/android.workflows.ts

jfmcquade · 2024-08-16T16:01:14Z

packages/workflows/src/ios.workflows.ts

+      },
+    ],
+  },
+  // Generate iOS assets from source images (splash.png, icon.png and, optionally, icon-foreground.png and icon-background.png)


I think icon.png should be icon-only.png instead

I am not too sure what this comment is specifically referencing. When I checked my assets, the image was called icon.png. However, I think any name changes can be solved by accessing the Google Drive.

I was going off the docs, which suggest that the source asset should be called "icon-only.png". So I think that's what this comment should specify as the name for the source file too.

Okay will do

jfmcquade · 2024-08-21T10:37:28Z

packages/scripts/src/tasks/providers/android.ts

-    },
-  };
+  // Generate android assets
+  const assetDirPath = assetPath.includes(".png") ? path.dirname(assetPath) : assetPath;


Do we know whether the file has to be a .png? Either way, I think it would be better to check whether the path is to a file in a more general way. Have a look at these docs (we're already importing fs so can use its methods)

jfmcquade · 2024-08-21T10:40:39Z

packages/workflows/src/ios.workflows.ts

@@ -19,6 +19,20 @@ const childWorkflows: IDeploymentWorkflows = {
      },
    ],
  },
+  set_icons_and_splash_images: {
+    label:
+      "Generate app launcher icon and splash screen image from icon.png and splash.png asset respectively. Then if provided, it will generate adaptive icon from icon-foreground.png and icon-background.png. Copy generated files to relevant folders.",


I think again that this should be "icon-only.png" instead of "icon.png". And no harm in linking to the docs again... Also copy over this comment to corresponding Android equivalent

I think this one may have to be changed within the gdrive. The comment I have changed but the actual name itself might have to be changed.

chore: migrating files from the old branch to the new branch

0542ffa

kwAsant requested a review from jfmcquade August 13, 2024 15:37

kwAsant temporarily deployed to debug August 13, 2024 15:37 — with GitHub Actions Inactive

jfmcquade mentioned this pull request Aug 14, 2024

Chore: Migrating from using the cordova-res package to using the capacitor-assets package for asset generation #2362

Closed

1 task

kwAsant added 2 commits August 14, 2024 16:39

chore: Ammended the file to incorporate legacy considerations within …

3e441a7

…the and .

kwAsant temporarily deployed to debug August 14, 2024 16:37 — with GitHub Actions Inactive

jfmcquade requested changes Aug 16, 2024

View reviewed changes

chore: altered code in order to align with Johnny's feedback

a423a93

kwAsant temporarily deployed to debug August 20, 2024 11:16 — with GitHub Actions Inactive

kwAsant requested a review from jfmcquade August 20, 2024 15:32

chore: Altering code to align with Johnny's feedback

12f4ca1

kwAsant temporarily deployed to debug August 21, 2024 10:23 — with GitHub Actions Inactive

jfmcquade reviewed Aug 21, 2024

View reviewed changes

kwAsant added 2 commits August 21, 2024 16:24

chore: Applying advised changes

cf99cf6

chore: Changed comment in

c4f3033

kwAsant temporarily deployed to debug August 21, 2024 16:17 — with GitHub Actions Inactive

kwAsant requested a review from jfmcquade August 22, 2024 09:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore: Migrating from using the cordova-res package to using the capacitor-assets package for asset generation #2364

Chore: Migrating from using the cordova-res package to using the capacitor-assets package for asset generation #2364

kwAsant commented Aug 13, 2024 •

edited

Loading

jfmcquade left a comment

jfmcquade Aug 16, 2024

kwAsant Aug 19, 2024

kwAsant Aug 19, 2024

jfmcquade Aug 21, 2024

kwAsant Aug 21, 2024

jfmcquade Aug 16, 2024

jfmcquade Aug 21, 2024

kwAsant Aug 21, 2024

jfmcquade Aug 16, 2024

kwAsant Aug 19, 2024

jfmcquade Aug 21, 2024

kwAsant Aug 22, 2024

jfmcquade Aug 21, 2024 •

edited

Loading

jfmcquade Aug 21, 2024

kwAsant Aug 21, 2024

Chore: Migrating from using the cordova-res package to using the capacitor-assets package for asset generation #2364

Are you sure you want to change the base?

Chore: Migrating from using the cordova-res package to using the capacitor-assets package for asset generation #2364

Conversation

kwAsant commented Aug 13, 2024 • edited Loading

Description

Git Issues

Screenshots/Videos

jfmcquade left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfmcquade Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwAsant commented Aug 13, 2024 •

edited

Loading

jfmcquade Aug 21, 2024 •

edited

Loading