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.

Sign up for GitHub

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

Jump to bottom

Push Notifications Feature #823

Closed
wants to merge 55 commits into from
Closed

Push Notifications Feature #823

wants to merge 55 commits into from
+3,386 −79

Conversation

pmpurifoy
Copy link
Contributor

This change adds the Push Notifications feature for packaged applications with demo app and unit tests.

  • Will add build files for NuGet script.

@ghost ghost added the needs-triage label May 14, 2021
@pmpurifoy pmpurifoy requested a review from aeloros May 14, 2021 22:49
@pmpurifoy pmpurifoy added the area-Notifications Toast notification, badges, Live Tiles, push notifications label May 14, 2021
jonwis
jonwis requested changes May 15, 2021
View reviewed changes
Copy link
Member

@jonwis jonwis left a comment

Choose a reason for hiding this comment

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

Initial round of comments; the implementation needs a thorough review of threading (such as the static locking thing in create channel) and a scrubbing of "accidental reference capture" instances.

dev/AppLifecycle/AppInstance.cpp Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationBackgroundTask.h Show resolved Hide resolved
dev/PushNotifications/PushNotificationChannel.cpp Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationChannel.cpp Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationChannel.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsDemoApp/main.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsDemoPackage/Package.appxmanifest Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsTestApp/main.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsTestApp/main.cpp Outdated Show resolved Hide resolved
oldnewthing
oldnewthing reviewed May 15, 2021
View reviewed changes
dev/AppLifecycle/AppInstance.cpp Outdated Show resolved Hide resolved
dev/PushNotifications/GetRawNotificationEventArgs.h Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationActivationInfo.cpp Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationActivationInfo.cpp Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationActivationInfo.h Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotifications.idl Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotifications.idl Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotifications.idl Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotifications.idl Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotifications.idl Outdated Show resolved Hide resolved
oldnewthing
oldnewthing reviewed May 17, 2021
View reviewed changes
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsDemoApp/main.cpp Show resolved Hide resolved
test/TestApps/PushNotificationsDemoApp/main.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsTestApp/main.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsTestApp/main.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsTestApp/main.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsTestApp/main.cpp Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationManager.cpp Outdated Show resolved Hide resolved
@jonwis
Copy link
Member

jonwis commented May 24, 2021 •
edited
Loading 

static const struct ExtensionMap

Future: make this contextexpr

In reply to: 847315891

In reply to: 847315891

Refers to: dev/AppLifecycle/ExtensionContract.h:15 in bd4bc2b. [](commit_id = bd4bc2b, deletion_comment = False)

oldnewthing
oldnewthing reviewed May 25, 2021
View reviewed changes
dev/PushNotifications/PushNotifications.idl Outdated Show resolved Hide resolved
dev/AppLifecycle/AppInstance.cpp Show resolved Hide resolved
dev/AppLifecycle/AppInstance.cpp Outdated Show resolved Hide resolved
dev/AppLifecycle/AppInstance.cpp Outdated Show resolved Hide resolved
dev/AppLifecycle/AppInstance.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsDemoApp/main.cpp Show resolved Hide resolved
test/TestApps/PushNotificationsDemoApp/main.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsTestApp/main.cpp Outdated Show resolved Hide resolved
DrusTheAxe
DrusTheAxe requested changes May 25, 2021
View reviewed changes
dev/PushNotifications/PushNotificationManager.cpp Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationReceivedEventArgs.h Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationRegistrationToken.cpp Outdated Show resolved Hide resolved
dev/PushNotifications/PushNotificationRegistrationToken.h Outdated Show resolved Hide resolved
dev/PushNotifications/externs.h Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/Shared.cpp Outdated Show resolved Hide resolved
oldnewthing
oldnewthing reviewed May 25, 2021
View reviewed changes
test/inc/TestDef.h Outdated Show resolved Hide resolved
dev/AppLifecycle/ExtensionContract.h Show resolved Hide resolved
danielayala94
danielayala94 requested changes Jun 1, 2021
View reviewed changes
test/PushNotificationTests/APITests.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/APITests.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/APITests.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/APITests.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/APITests.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/APITests.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/APITests.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/APITests.cpp Outdated Show resolved Hide resolved
test/PushNotificationTests/APITests.cpp Outdated Show resolved Hide resolved
test/TestApps/PushNotificationsTestApp/main.cpp Outdated Show resolved Hide resolved
@aeloros aeloros mentioned this pull request Jun 2, 2021
Closed
@aeloros aeloros mentioned this pull request Jun 3, 2021
Closed
@pmpurifoy pmpurifoy mentioned this pull request Jun 21, 2021
Closed
@pmpurifoy pmpurifoy removed the request for review from aeloros June 21, 2021 18:38
@pmpurifoy
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pmpurifoy
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@danielayala94
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@danielayala94
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@danielayala94
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@pmpurifoy
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@pmpurifoy
Copy link
Contributor Author

Closing this PR and opening PR on WindowsAppSDK repo because the pipeline changed the repo URI.

@pmpurifoy pmpurifoy closed this Jun 24, 2021
@pmpurifoy pmpurifoy mentioned this pull request Jun 24, 2021
Merged
@pmpurifoy pmpurifoy deleted the WNP_Reunion_PR branch June 24, 2021 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EHO-makai EHO-makai EHO-makai left review comments

@danielayala94 danielayala94 danielayala94 approved these changes

@oldnewthing oldnewthing oldnewthing left review comments

@sharath2727 sharath2727 sharath2727 left review comments

@riverar riverar riverar left review comments

@hulumane hulumane hulumane approved these changes

@jonwis jonwis jonwis approved these changes

@DrusTheAxe DrusTheAxe DrusTheAxe approved these changes

Assignees
No one assigned
Labels
area-Notifications Toast notification, badges, Live Tiles, push notifications needs-triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@pmpurifoy @jonwis @oldnewthing @DrusTheAxe @danielayala94 @riverar @sharath2727 @hulumane @EHO-makai @aeloros