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

[FIX] Reply and deep link from push notifications not working on iOS #4550

Merged
merged 5 commits into from
Sep 23, 2022

Conversation

diegolmello
Copy link
Member

@diegolmello diegolmello commented Sep 21, 2022

Proposed changes

Reply issue

We migrate AppDelegate to Objective-C++ on #4316, which is documented here https://reactnative.dev/docs/next/new-architecture-app-intro#ios---use-objective-c-mm-extension
However, we used import the swift bridging header on AppDelegate and we removed it by mistake.
Since there is no compatibility between Swift and Objective-C++, we had to patch react-native-notifications and do a workaround on Expo to stop importing using the Swift way (@import module) and make it import using obj-c way (<module/module.h>).
References:

Deep link issue

We worked on a fix on #4428, but that fixed only deep link with URL and not with a push notification tap.
In order to fix it, I added startMonitorNotifications from RNN again, which was also removed by mistake on the Obj-C++ migration.

Side note

We could also fix this by bringing back Obj-C to AppDelegate (.m rather than .mm), but that would be a shitty workaround, since we'd have to deal with this error in the future in order to enable the RN's New Architecture (Fabric).

Issue(s)

How to test or reproduce

from cold boot

  • Make sure app is closed
  • Receive a push notification
  • Tap on it
  • It should redirect to the room

resume

  • Make sure app is minimized
  • Receive a push notification
  • Tap on it
  • It should redirect to the room

reply

  • Receive a push notification
  • Reply
  • It should send the message

reply from cold boot

  • Make sure app is closed
  • Receive a push notification
  • Reply
  • It should send the message

Screenshots

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

@diegolmello diegolmello changed the title [WIP] iOS open from push [FIX] Reply and deep link from push notifications not working on iOS Sep 22, 2022
@diegolmello diegolmello marked this pull request as ready for review September 22, 2022 19:18
@diegolmello diegolmello merged commit e883795 into develop Sep 23, 2022
@diegolmello diegolmello deleted the fix.ios-push branch September 23, 2022 20:35
ivnxyz pushed a commit to NextiaDev/Rocket.Chat.ReactNative that referenced this pull request May 26, 2023
ivnxyz pushed a commit to NextiaDev/Rocket.Chat.ReactNative that referenced this pull request May 26, 2023
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.

None yet

2 participants