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 Live Activity widget to sample app #345

Merged
merged 2 commits into from
Dec 8, 2022
Merged

Conversation

brismithers
Copy link
Contributor

@brismithers brismithers commented Dec 1, 2022

Description

One Line Summary

Add a Live Activity example to the sample app.

Details

Motivation

In order to showcase how a Xamarin app could produce a live activity, and have it be updated via the OneSignal platform. Xamarin does not have support WidgetKit/ActivityKit, so native bindings and native extensions were created. The general approach was adopted from the WidgetKit example provided by https://github.com/chamons/xamarin-ios-swift-extension.

Note that because this approach requires a native build of the widget in order for the Xamarin solution to build, the Live Activity functionality defaults to disabled. It can be enabled by specifying the LIVE_ACTIVITIES compiler constant. See Samples/LIVE_ACTIVITIES.md for more explicit instructions.

Scope

This change should only affect the sample application.

Testing

Manual testing

The sample app was tested on both iPhone and iPhoneSimulator, with live activity functionality enabled and disabled. Entering/exit live activity was driven in all variations to ensure correct behavior.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Reviewed 37 of 37 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @brismithers and @fhboswell)


.gitignore line 7 at r1 (raw file):

[Oo]bj/
[Bb]in/
DerivedData/

Normally you don't want to check this in, since it contains built binaries the build process will generate automatically.
I could see this being needed however based on the fact of how the project is being linked, however I see no new files in this PR added to to DerivedData/ so this isn't needed.

  • If you missed some files and this really is needed, this still is concerning, as I think DerivedData is Xcode version specific too, but could be wrong.

Copy link

@fhboswell fhboswell left a comment

Choose a reason for hiding this comment

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

A couple questions and comments.

Copy link

@fhboswell fhboswell left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @brismithers and @jkasten2)


.gitignore line 7 at r1 (raw file):

Previously, jkasten2 (Josh Kasten) wrote…

Normally you don't want to check this in, since it contains built binaries the build process will generate automatically.
I could see this being needed however based on the fact of how the project is being linked, however I see no new files in this PR added to to DerivedData/ so this isn't needed.

  • If you missed some files and this really is needed, this still is concerning, as I think DerivedData is Xcode version specific too, but could be wrong.

After seeing a demo it is my understanding that this directory is only used to hold the build output of the Xcode project which hosts the live activity, and therefore it makes sense to gitignore it.

Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @brismithers)


.gitignore line 7 at r1 (raw file):

Previously, fhboswell (Franklin Henry Boswell) wrote…

After seeing a demo it is my understanding that this directory is only used to hold the build output of the Xcode project which hosts the live activity, and therefore it makes sense to gitignore it.

I don't know why but I thought this was a delete rather than an add, disregard my first comment. No issue here.

@fhboswell fhboswell self-requested a review December 2, 2022 20:44
@brismithers brismithers merged commit 875290b into main Dec 8, 2022
@brismithers brismithers deleted the live-activities-sample-app branch December 8, 2022 14:13
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