Details

Reviewers

tomek
atul
ashoat

Commits

rCOMM549d6d88c955: Export all react-native methods needed to replace rect-native-notifications

Summary

This differential implements all RCT_EXPORTED methods needed to replace react-native-notifications. Methods are implemented as in Ashoats fork.

Test Plan

This will be a matter of subsequent diffs, but for the sake of testing one can replace NotificationsIOS.<...> methods with CommIOSNotifications methods of the same name.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Dec 28 2022, 6:45 AM

Herald added a subscriber: ashoat. · View Herald TranscriptDec 28 2022, 6:45 AM

marcin added a parent revision: D6071: Use CommIOSNotifications in AppDelegate and in JavaScript to request permissions.Dec 28 2022, 6:46 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Dec 28 2022, 6:50 AM

Harbormaster failed remote builds in B14832: Diff 20223!

Add necessary formatting function

Harbormaster completed remote builds in B14834: Diff 20225.Dec 28 2022, 7:09 AM

marcin requested review of this revision.Dec 28 2022, 7:09 AM

marcin added a child revision: D6139: Export Background Fetch result constants from CommIOSNotifications to JavaScript.Jan 2 2023, 6:39 AM

All methods in this differential are copied form Ashoat's fork https://github.com/Ashoat/react-native-notifications/blob/master/RNNotifications/RNNotifications.m. Subtle visual differences are results of applying formatter.

Copy pasted code will be refactored

Update diff since CommIOSNotificationsBridgeQueue API changed

Harbormaster completed remote builds in B15190: Diff 20702.Jan 9 2023, 7:34 AM

marcin planned changes to this revision.Jan 9 2023, 8:04 AM

Remove conditionals throughout please, and please think more carefully about every line you write. You should know why you are writing if ([UNUserNotificationCenter class]) {, but in this case it seems like you did not know, and instead were simply copy-pasting

native/ios/Comm/CommIOSNotifications/CommIOSNotifications.mm
421 ↗	(On Diff #20702)	Remove this conditional please, it's irrelevant as we target a minimum of iOS 13 now and this class was introduced in iOS 10

(I didn't see the "planned changes", so perhaps you were still working on this)

In D6072#185996, @ashoat wrote:

(I didn't see the "planned changes", so perhaps you were still working on this)

Yes, you are right - I forgot to hit "Plan Changes" on this differential. I haven't refactored it yet to remove copy-pasted code.

You didn't forget, I just didn't see it!

Consequently use dot syntax.
Remove redundant check for [UNNotificationCenter class]
Change implementation of getAllDeliveredNotifications:()callback - we are using different formatting method, but it is still correct.

Harbormaster failed remote builds in B15411: Diff 21013!Jan 17 2023, 7:26 AM

Harbormaster completed remote builds in B15411: Diff 21013.Jan 17 2023, 7:42 AM

ashoat accepted this revision.Jan 17 2023, 8:24 AM

This revision is now accepted and ready to land.Jan 17 2023, 8:24 AM

Refactor

Harbormaster failed remote builds in B15434: Diff 21043!Jan 18 2023, 7:06 AM

Rebase before landing

Harbormaster completed remote builds in B15713: Diff 21422.Jan 27 2023, 1:21 AM

Closed by commit rCOMM549d6d88c955: Export all react-native methods needed to replace rect-native-notifications (authored by marcin). · Explain WhyJan 27 2023, 1:52 AM

This revision was automatically updated to reflect the committed changes.

marcin added a commit: rCOMM549d6d88c955: Export all react-native methods needed to replace rect-native-notifications.