IDEMSInternational / open-app-builder

PLH App Frontend
GNU General Public License v3.0
5 stars 24 forks source link

Feat: templated iOS config #2256

Closed jfmcquade closed 3 months ago

jfmcquade commented 3 months ago

PR Checklist

Description

Builds on #2252, should be reviewed after that PR has been merged.

Adds a new workflow namespace, ios, for running workflows associated with iOS setup. Currently there is a single child action, configure, which populates the necessary configuration files for building to an iOS app, analogous to the android configure workflow introduced in #2252.

This workflow can be run via

yarn workflow ios configure

(or yarn workflow ios, which runs all (1) child actions).

In order to build an iOS app, the deployment config must contain a value for config.git.content_tag_latest (which is interpreted as the app version number, as of #2252), and additionally must contain values for two new properties, ios.app_id and ios.app_name, e.g.

config.ios.app_id = "international.idems.example_app"
config.ios.app_name = "Example App"

Note that unlike the Android app ID, the iOS app ID, as it appears in the app bundle, cannot contain underscore characters. This is not currently enforced in the config, instead underscores are converted to hyphens as part of the ios configure action. This allows for the app IDs as they appear in the deployment config to be identical and still use our current naming convention.

Follow ups

Testing

Provide values in the deployment config for ios.app_id and ios.app_name, e.g.

config.ios.app_id = "international.idems.example_app"
config.ios.app_name = "Example App"

Run yarn workflow ios to generate new populated configuration files. These should show up as git differences in two files: capacitor.config.ts and ios/App/App.xcodeproj/project.pbxproj. See screenshots

Screenshot 2024-03-25 at 15 01 18 Screenshot 2024-03-25 at 15 00 52

Without a Mac set up for iOS development, you won't be able to test that the generated files successfully configure an iOS to be built with the desired properties, but I can confirm that I can do so successfully, see screenshot below.

Dev notes

Only one iOS-specific file, ios/App/App.xcodeproj/project.pbxproj, contains the relevant values, so the implementation is more straightforward than for Android. This is partly because the iOS config files project.pbxproj and Info.plist support their own templating syntax, so I'd already removed any hardcoded strings from Info.plist as part of #2234: e.g. in Info.plist, CFBundleDisplayName is set to $(PRODUCT_NAME), where the value of PRODUCT_NAME is defined in project.pbxproj.

Possibly for discussion: I'm unsure of the best camelCase styling of "iOS". It seems like there's no clear consensus, so I think it's up to us to decide a convention for the codebase. Personally I think iOS is probably the most legible, where the "i" can be capitalised in accordance with camelCase/PascalCase conventions, e.g. iOSVariable, IOSClass, anIOSVariable. But I'm not strongly wedded to this: iosVariable, IosClass and anIosVariable could also work, but is a bit less legible to me.

Git Issues

Closes #2238 Closes #2239

Screenshots/Videos

The built app running in local emulation, displaying a custom app_name, "Debug App 1".

app built
jfmcquade commented 3 months ago

I take it the named variable replacement worked for the template.pbxproj file then? In hindsight I probably should have used that approach for Android too (a little more explicit), but I don't think it makes a huge difference either way.

Yes, it works. Yeh I can see how it might be good to be explicit, but agree it's not a big issue. For both of our current use cases, we're limiting the replaced variables to those provided as envAdditional, so there could also potentially be a boolean option on the replaceFiles() method to limit just to those variables explicitly provided to the function.

One minor issue I've just thought of is if using different app_id properties for ios and android how best to handle in the capacitor.config.ts file. Technically it shouldn't really make a difference as those values are only used when adding a platform for the first time (e.g. npx cap add ios), and makes no difference if id uses only alpha-numeric, but for now it seems the template is only updated as part of android workflow. Again, I don't think this is a major issue for now

Yeh I did think a bit about using different app_id properties for ios and android, which does seem desirable. The ios workflow action does also update the capacitor.config.ts file, so that file will contain values corresponding the platform that the dev/author most recently ran the configure action for. As you say, these values aren't used after the initial npx cap add command is run, so this shouldn't have any impact, but worth being aware of.

chrismclarke commented 3 months ago

2260 added to address item identified in follow-ups