mieszko4 / react-native-zoom-us

MIT License
122 stars 115 forks source link

Upgrade Android Zoom SDK to 5.16.2.16555 #314

Closed wilkinson4 closed 4 months ago

wilkinson4 commented 6 months ago

Looks great. I just noticed that there are some UI issues when trying to leave the meeting. I was following this test plan: https://github.com/mieszko4/react-native-zoom-us-test#smoke-test-procedure so I was trying to figure out why it is happening but I did not find too much time.

Do you also have such issues?

Hey @mieszko4. I am also experiencing that issue as well. It seems to happen if I join a meeting for a short period of time, and leave, but it doesn't happen when the host leaves. I am not getting an error in Logcat, but here is what it is logging when it happens: Screenshot 2023-11-20 at 1 51 52 PM I haven't figured out why it's trying to rejoin the meeting in this case instead of updating the UI by leaving the meeting.

mieszko4 commented 5 months ago

After investigating a bit, I've noticed that joining non-existing meeting also gets stuck when using noMeetingErrorMessage: true - so that could be related. We can remove this option until we figure out the issue.

The current blocker, however, is being stuck in this scenario:

  1. Join meeting
  2. Admit by host
  3. Leave the meeting (not by host)

This happens even with noMeetingErrorMessage: false. I will take a deeper look on why it happens.

wilkinson4 commented 5 months ago

After investigating a bit, I've noticed that joining non-existing meeting also gets stuck when using noMeetingErrorMessage: true - so that could be related. We can remove this option until we figure out the issue.

The current blocker, however, is being stuck in this scenario:

1. Join meeting

2. Admit by host

3. Leave the meeting (not by host)

This happens even with noMeetingErrorMessage: false. I will take a deeper look on why it happens.

@mieszko4 I have a PR to add the latest SDK version to the jitpack repo. I wanted to see if Zoom fixed that bug in a later version because it feels like a bug with their SDK because when I downgraded to 5.15, that leave meeting issue went away.

mieszko4 commented 5 months ago

Thnx @wilkinson4. We got hit by LFS Data limit in that repo so I recreated it but now your PR is gone (https://github.com/zoom-us-community/jitpack-zoom-us-archive/issues/2). I am a bit blocked now. Will think about a solution.

wilkinson4 commented 5 months ago

Thnx @wilkinson4. We got hit by LFS Data limit in that repo so I recreated it but now your PR is gone (zoom-us-community/jitpack-zoom-us-archive#2). I am a bit blocked now. Will think about a solution.

It sounds like when I try to push to a fork from the jitpack repo it is relying on your LFS limits, because I purchased a pack for myself just now and I'm still getting this error message even after re-forking.

Uploading LFS objects:   0% (0/14), 0 B | 0 B/s, done.
batch response: This repository is over its data quota. Account responsible for LFS bandwidth should purchase more data packs to restore access.
error: failed to push some refs to 'github.com:wilkinson4/jitpack-zoom-us-fork.git'
wilkinson4 commented 5 months ago

Thnx @wilkinson4. We got hit by LFS Data limit in that repo so I recreated it but now your PR is gone (zoom-us-community/jitpack-zoom-us-archive#2). I am a bit blocked now. Will think about a solution.

@mieszko4 did you try reaching out to Github support? Maybe they can do something so we can use those data packs you purchased? https://github.com/orgs/community/discussions/22906

mieszko4 commented 5 months ago

@wilkinson4 Resolved. You should be able to make a new PR in https://github.com/zoom-us-community/jitpack-zoom-us

wilkinson4 commented 5 months ago

@wilkinson4 Resolved. You should be able to make a new PR in https://github.com/zoom-us-community/jitpack-zoom-us

PR is up. Thank you! https://github.com/zoom-us-community/jitpack-zoom-us/pull/3

wilkinson4 commented 4 months ago

After investigating a bit, I've noticed that joining non-existing meeting also gets stuck when using noMeetingErrorMessage: true - so that could be related. We can remove this option until we figure out the issue.

The current blocker, however, is being stuck in this scenario:

1. Join meeting

2. Admit by host

3. Leave the meeting (not by host)

This happens even with noMeetingErrorMessage: false. I will take a deeper look on why it happens.

@mieszko4 I was able to resolve this with this commit 186f7c79bf4d21f6b6949c227344ac3b26569b69.

rposborne commented 4 months ago

@mieszko4 As we hit 1 month out from the next min SDK enforcement by zoom. What can we do to support getting this bump? Are there any further items that need to be worked?

https://developers.zoom.us/docs/meeting-sdk/minimum-version/

mieszko4 commented 4 months ago

@mieszko4 I was able to resolve this with this commit 186f7c7.

It did not work well for me each time.

wilkinson4 commented 4 months ago

@mieszko4 I just pushed new changes to fix the issue when a non-host leaves a meeting. Looks like in the onHostResume Android Lifecycle handler function we were getting a meeting status of Disconnecting which was causing it to try and re-join the meeting.

Also, I added back the call to updateVideoView that I had previously removed trying to fix this. Let me know if we are good to merge then. We are chasing a February minimum version that is coming up quick.

mieszko4 commented 4 months ago

Awesome! The fix looks promising. I will test it out tomorrow.

mieszko4 commented 4 months ago

Did the following Test Plan:

The last one gets stuck on Connecting screen sometimes. Only after closing the app it works correctly.

mieszko4 commented 4 months ago

I will merge it, we can figure out the last one in another PR.