IDEMSInternational / open-app-builder

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

Feat: iOS build #2234

Closed jfmcquade closed 3 months ago

jfmcquade commented 4 months ago

PR Checklist

Description

Adds the capacitor-managed iOS build of the app. The iOS build has not been extensively tested, so there are bugs and features that should be fixed as follow-ups.

In addition to the files generated by running npx cap add ios, the following changes were required:

An initial iOS build is available in appetize here.

Dev notes

Steps followed:

  1. install xcode
  2. install xcode cli tools
    • Or otherwise configure xcode CLI tools, this worked for me: sudo xcode-select -s /Applications/Xcode.app/Contents/Developer
  3. install cocoapods
    • For me, I needed to install using homebrew, i.e. brew install cocoapods. Installing by other methods caused errors
  4. Run npx cap add ios
  5. Run yarn build
  6. Run npx cap sync
  7. Run npx cap open ios to open the project with the xcode application.
  8. Start the project in emulation via the xcode UI.
    • This would initially not compile as the Firebase secrets file GoogleService-Info.plist is required
      • This file can't be added from the node project directly and must be added through xcode's UI, see this thread. Although that thread does suggest some workarounds, including using the Trapeze project API and using the xcode npm package (not actively maintained).
      • This file does not get committed to the repo, but some references to the file are added to ios/App/App.xcodeproj/project.pbxproj, which is committed
    • As we are at our limit of Firebase apps for a single project (see this mattermost thread), rather than registering a new app in Firebase I added a dummy version of the file from here, which did enable the app to compile in xcode.

With the initial setup done, the workflow for running the app in emulation would be:

  1. yarn build
  2. npx cap sync ios (or if only web code changed, npx cap copy ios)
  3. npx cap open ios

TODO / Follow-Ups

Testing and bug fixes

Now that the app can be ran in emulation, it would be sensible to perform some general functional testing and create issues for any bugs that are discovered. From some cursory testing, I noticed bugs with the following components and functionality, each of which should be investigated further:

Git Issues

Closes #

Screenshots/Videos

Initial run in emulation. Shows the app installing and opening correctly. Issues are immediately obvious with the header, footer and toggle bar.

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/510f3b5d-f4f5-4775-9fb8-2bd5d0300081

chrismclarke commented 4 months ago

Did you manage to test crashlytics update on ios/android as part of the pr or is that still required?

Otherwise not much I can do to test from this side as I don't have a mac to run the code on. Would you be able to share a couple of screenshots or videos to get a feel for how things are looking?

From code side I'm guessing it's mostly just generated from capacitor scripts and firebase install instructions, but is there anything specifically you want me to look at?

Otherwise, I assume this PR should be fine just to get this merged in as a core addition and then iterate (once confirmed crashlyitics migration doesn't break things).

I would probably recommend an early priority should be to add an appetize ios github action to make it easier for non-mac owners to do functional review/testing - which might take a little extra work as I think the old repo-based action has been refactored to reusable so will possibly need to adapt so that we can still test from main repo (likely checking out the debug repo content)

jfmcquade commented 3 months ago

Thanks @chrismclarke. Appreciate there's not much you can do to test without being able to run the code.

I've attached a video to the PR description showing an initial run of the app in the iOS emulator. I've also added a link to a build manually uploaded to appetize (available here).

I've also broken out the various follow-ups into issues. They are grouped under the iOS support milestone.

I have now tested crashlytics using the new package on this feature branch for an Android build. Following your suggestion in this PR comment, I hardcoded some method calls to the crashlytics service on startup. I can confirm that the recordException and crash methods trigger events that can be seen in the crashlytics console: here and here. However I can't find the effects of log method in the crashlytics console – is it supposed to show as a non-fatal issue do you know? The wording in the capacitor plugin docs is ambiguous to me about what the expected outcome is.

The method calls I added were as follows:

this.log({message: "Test PR 2234 - log"})
this.recordException({message: "Test PR 2234 - exception"})
this.crash({message: "Test PR 2234 - crash"})

Pending that crashlytics question, I would say I'd be happy to merge this PR as-is if you are, then we can work on the related issues as follow-ups.

chrismclarke commented 3 months ago

Thanks @jfmcquade , all sounds good to me. Thanks for adding the follow-up issues, I've added a few comments but happy to discuss more as needed and take on whatever you need me to (can decide on next call).

I don't think we ever call the crashlytics log event (we have glitchtip for logging) so I wouldn't be too worries. See a bunch of test issues in non-fatal so assume that's all fine (annoyingly the links you post never quite work as firebase personalise the /u property depending on how many logged in profiles/accounts you have and so always a bit of a crapshoot to figure out

image