In order to correctly use CommIOSNotifications in AppDelegate we must call comsumeBackgroundQueue in ios.js. Therefore this differential exports minimal set of react-native methods that must be called from JavaScript.
Details
Details
build the app.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- marcin/eng-2334
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
All methods in this differential are copied from Ashoat's fork https://github.com/Ashoat/react-native-notifications/blob/master/RNNotifications/RNNotifications.m. Subtle visual differences are results of applying formatter.
Comment Actions
Copy-pasting code requires considering the license of what you're copying. Instead, can you look at the existing code as inspiration and implement something similar, but in your own style?
| native/ios/Comm/CommIOSNotifications/CommIOSNotifications.mm | ||
|---|---|---|
| 293–298 ↗ | (On Diff #20731) | I don't think this ternary follows our rules https://www.notion.so/commapp/Use-ternary-conditionals-sparingly-f4ba44a10259403592a1d15440a9847e |
| 308–309 ↗ | (On Diff #20731) | It feels like a statement jsReady = YES quite obviously means that we're marking the js thread as ready, so I don't think the comment is really useful. |
| 344–346 ↗ | (On Diff #20731) | This is formatted differently that other functions - we usually don't add whitespaces around colons |
| 348 ↗ | (On Diff #20731) | This syntax is inconsistent with the other place in this file |
Comment Actions
Please address concerns before landing
| native/ios/Comm/CommIOSNotifications/CommIOSNotifications.mm | ||
|---|---|---|
| 293–298 ↗ | (On Diff #20731) | Yeah let's avoid this |
| 308–309 ↗ | (On Diff #20731) | Agree |
| 344–346 ↗ | (On Diff #20731) | This makes me suspect that something is wrong with clang-format precommit hooks, which reinforces the concerns I laid out in D6063 |
| 348 ↗ | (On Diff #20731) | I personally prefer the dot syntax. @marcin, remember that it's your responsibility to rewrite this code in your own style – that style should be consistent throughout. Using inconsistent style makes me suspect that you're still copy-pasting |
Comment Actions
Nit
| native/ios/Comm/CommIOSNotifications/CommIOSNotifications.mm | ||
|---|---|---|
| 268–269 ↗ | (On Diff #21009) | It would probably be cleaner if you defined this callback before calling requestAuthorizationWithOptions:completionHandler:... that way it wouldn't be so indented, I think |