Fire RN events on meeting- and auth events #61
Conversation
| case MeetingEndReason.END_BY_SELF: return "endedBySelf"; | ||
| case MeetingEndReason.END_FOR_FREEMEET_TIMEOUT: return "endedFreeMeetingTimeout"; | ||
| case MeetingEndReason.END_FOR_JBHTIMEOUT: return "endedJBHTimeout"; | ||
| case MeetingEndReason.END_FOR_NOATEENDEE: return "endedNoAttendee"; |
There was a problem hiding this comment.
is this android only? it is missing in ios
There was a problem hiding this comment.
Oops, looks like I do have it for iOS: https://github.com/mieszko4/react-native-zoom-us/pull/61/files#diff-bf95962c6ae718fab4cbb875c876bb68ec9b13b786619231627748158785ac7aR251
There was a problem hiding this comment.
🤔 I do not understand your reply. My point is that endedNoAttendee has no corresponding reason for ios (see https://marketplacefront.zoom.us/sdk/meeting/ios/_mobile_r_t_c_constants_8h_source.html line 487)
so it should be put as the last case and marked as such:
| case MeetingEndReason.END_FOR_NOATEENDEE: return "endedNoAttendee"; | |
| case MeetingEndReason.END_FOR_NOATEENDEE: return "endedNoAttendee"; // Android only |
| inMeetingService.removeListener(this); | ||
| InMeetingShareController inMeetingShareController = inMeetingService.getInMeetingShareController(); | ||
| inMeetingShareController.removeListener(this); |
There was a problem hiding this comment.
I think we should check if inMeetingService/inMeetingShareController is null before calling removeListener
There was a problem hiding this comment.
You are absolutely right. I missed doing this in a few other places, too. I'll fix that.
ios/RNZoomUs.m
Outdated
| case MobileRTCAuthError_None: return @"none"; // iOS only | ||
| case MobileRTCAuthError_OverTime: return @"overTime"; // iOS only | ||
| case MobileRTCAuthError_NetworkIssue: return @"networkIssue"; // iOS only | ||
| case MobileRTCAuthError_ClientIncompatible: return @"clientIncompatible"; |
There was a problem hiding this comment.
in order to make the mappings to be more readable. put common ios/android cases on top and sort all of the return alphabetically.
In this way it will be easier to compare what is in java and what is in objective-c
| case MeetingEndReason.END_FOR_JBHTIMEOUT: return "endedJBHTimeout"; | ||
| case MeetingEndReason.END_FOR_NOATEENDEE: return "endedNoAttendee"; | ||
| case MeetingEndReason.KICK_BY_HOST: return "endedRemovedByHost"; | ||
| default: return "endedUnknownReason"; |
There was a problem hiding this comment.
I believe that should be "unknown" for consistency with other events
There was a problem hiding this comment.
This is still a MeetingEvent, which is why I made it "endedUnknownReason" instead of "unknown". I could make it a separate event, but for our use case, it's not actually that useful to have the meeting end events separate from other meeting events (error, connect).
| case MeetingEndReason.END_BY_SELF: return "endedBySelf"; | ||
| case MeetingEndReason.END_FOR_FREEMEET_TIMEOUT: return "endedFreeMeetingTimeout"; | ||
| case MeetingEndReason.END_FOR_JBHTIMEOUT: return "endedJBHTimeout"; | ||
| case MeetingEndReason.END_FOR_NOATEENDEE: return "endedNoAttendee"; |
There was a problem hiding this comment.
🤔 I do not understand your reply. My point is that endedNoAttendee has no corresponding reason for ios (see https://marketplacefront.zoom.us/sdk/meeting/ios/_mobile_r_t_c_constants_8h_source.html line 487)
so it should be put as the last case and marked as such:
| case MeetingEndReason.END_FOR_NOATEENDEE: return "endedNoAttendee"; | |
| case MeetingEndReason.END_FOR_NOATEENDEE: return "endedNoAttendee"; // Android only |
|
@jcamins What would be really useful is having an example in |
|
@mieszko4 I have not forgotten your request. I created a ticket for myself to make sure that I do it. |
This adds RN events on:
We could fire events on other things as well, but these are the ones that I have needed to handle screen sharing in our React Native app smoothly. I'm not certain this is the right approach, since—particularly on Android with InMeetingServiceListener methods—it results in a rather large amount of additional code, but it works well for us.