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: Select conversation issue on iOS #71

Merged
merged 1 commit into from
Jan 11, 2025

Conversation

avivdim
Copy link
Contributor

@avivdim avivdim commented Jan 7, 2025

On Android, the Zendesk messaging view worked as expected because it was displayed as part of the regular screen flow. On iOS, using rootController.show(viewController, sender: self) pushed the Zendesk view onto the navigation stack. However, the Zendesk view was just a single screen, and there was no deeper navigation (i.e., tapping on a conversation to open a detailed view), so the interaction issues didn't appear. After the update that introduced multi-conversation support, where tapping on a conversation opens a deeper screen, the previous approach caused issues because the Zendesk view was not properly integrated into the app's navigation stack. This prevented the deeper navigation from functioning correctly (e.g., tapping on a conversation didn't lead to a new screen).

Solution:

  1. We checked if a UINavigationController was available in the app.
  2. If a UINavigationController was present, we pushed the Zendesk view onto the navigation stack, allowing for proper
    navigation (e.g., tapping on a conversation now pushes a new screen).
  3. If no UINavigationController was found, we ensured the Zendesk view was presented fullscreen, but still part of the normal navigation context.

This fix resolves the interaction issue by ensuring the Zendesk view is correctly integrated into the app’s navigation stack, enabling the deeper screen navigation introduced by the multi-conversation update and providing a smooth user experience.

@leegeunhyeok leegeunhyeok self-requested a review January 7, 2025 18:51
@leegeunhyeok
Copy link
Owner

LGTM 👍

I'll test these changes on React Native 0.71 and 0.77 this week, and I'll approve them after testing.

refs #75

@leegeunhyeok
Copy link
Owner

Before After
Screen.Recording.2025-01-11.at.15.10.08.mov
Screen.Recording.2025-01-11.at.15.17.47.mov

@leegeunhyeok leegeunhyeok merged commit d468bed into leegeunhyeok:main Jan 11, 2025
1 check passed
@leegeunhyeok
Copy link
Owner

closes #59

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.

2 participants