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: sharing external video does not stop screenshare #12801

Merged

Conversation

ramonlsouza
Copy link
Member

What does this PR do?

  • Fixes an issue where sharing an external video in new layouts would not end screenshare
  • Moves screenshare notifications from media container to screenshare component

@sonarcloud
Copy link

sonarcloud bot commented Jul 21, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@antobinary
Copy link
Member

I am not sure if this is the best approach for solving the issue.
If we agree that we can only play one of [ external video, screenshare ] at a time, I would prefer if akka-apps was the one stopping screenshare, rather than bbb-html5 client deciding not to play it...
If we want to allow both to play simultaneously, we'd need to rethink layouts (and more).

Either way, unless I am missing some key point here, I am not sure about this client side decision

@prlanzarin
Copy link
Member

prlanzarin commented Jul 21, 2021

I am not sure if this is the best approach for solving the issue.
If we agree that we can only play one of [ external video, screenshare ] at a time, I would prefer if akka-apps was the one stopping screenshare, rather than bbb-html5 client deciding not to play it...

I agree with akka-apps being the one ejecting it.

That being said: this PR restores the functionality as it was [working,implemented] in 2.0/2.2/2.3 (essentially fixing a regression), so I'd treat re-doing the ejection control as a separate PR/issue.

@antobinary antobinary merged commit cd0232d into bigbluebutton:develop Jul 21, 2021
@antobinary
Copy link
Member

I'd treat re-doing the ejection control as a separate PR/issue.

Created #12803

@erpsolns
Copy link

Instead if the settings for the meeting having an option in apps menu to disable and enable stop screenshare when external video initiated - that would not be regression - that is moving forward

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

4 participants