-
Notifications
You must be signed in to change notification settings - Fork 26
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
Feat: iOS build #2234
Feat: iOS build #2234
Conversation
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) |
…le with valid values
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 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. |
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 |
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:
Testing
An initial iOS build is available in appetize here.
Dev notes
Steps followed:
sudo xcode-select -s /Applications/Xcode.app/Contents/Developer
brew install cocoapods
. Installing by other methods caused errorsnpx cap add ios
pod 'FirebaseCore', '~> 10'
toios/App/Podfile
(see Firebase docs):yarn build
npx cap sync
npx cap open ios
to open the project with the xcode application.GoogleService-Info.plist
is requiredios/App/App.xcodeproj/project.pbxproj
, which is committedWith the initial setup done, the workflow for running the app in emulation would be:
yarn build
npx cap sync ios
(or if only web code changed,npx cap copy ios
)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.
Initial.emulation.mov