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

RTC-14731 - update "showScreenSharingIndicator" method #2062

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

axeleriksson147
Copy link
Contributor

@axeleriksson147 axeleriksson147 commented Jan 5, 2024

showScreenSharingIndicator is a method used by Symphony Meetings. However, when we switched from Client 1.5 to 2.0, the window object for SymphonyElectron changed name and we haven't used this method in about 3 years. However, I am investigating https://perzoinc.atlassian.net/browse/RTC-14615 about screen sharing issues and one possible explanation is that our current way of sharing screen by posting messages via window.postMessage disappears or don't get picked up. Therefore we should look into using the exposed SymphonyElectron methods via the window object. It could be more reliable.

@@ -512,12 +512,9 @@ export class SSFApi {
}

/**
* Shows a banner that informs user that the screen is being shared.
* Shows a banner that informs the user that the screen is being shared.
Copy link
Contributor Author

@axeleriksson147 axeleriksson147 Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is now pretty much identical to openScreenSharingIndicator. The only difference is if requestId is tracked in SymphonyElectron or if it's passed. Should we remove openScreenSharingIndicator?

I see a handleMessage property in app-bridge.ts, and it says it's only for 1.5. I think that's what's being used right now by screen sharing, but is anyone else using this method? If not, then we should remove that entire piece of code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only RTC and client 1.5 was using the app-bridge.ts, now I think we will need remove the complete code

@axeleriksson147 axeleriksson147 changed the title RTC-14731 - Cleanup showScreenSharingIndicator method RTC-14731 - update "showScreenSharingIndicator" method Jan 5, 2024
Comment on lines -541 to -546
const destroy = () => {
throttledCloseScreenShareIndicator(stream.id);
stream.removeEventListener('inactive', destroy);
};

stream.addEventListener('inactive', destroy);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.electronjs.org/docs/latest/api/context-bridge#parameter--error--return-type-support - This doesn't work due to these reasons. SFE-RTC will have to be responsible for closing the screen sharing indicator (which is probably better, SymphonyElectron shouldn't do too much).

@KiranNiranjan KiranNiranjan merged commit dee1815 into finos:main Jan 5, 2024
6 checks passed
@axeleriksson147 axeleriksson147 deleted the axeleriksson/RTC-14731 branch January 5, 2024 11:53
axeleriksson147 added a commit to axeleriksson147/SymphonyElectron that referenced this pull request Jan 8, 2024
axeleriksson147 added a commit to axeleriksson147/SymphonyElectron that referenced this pull request Jan 8, 2024
sbenmoussati pushed a commit to sbenmoussati/SymphonyElectron that referenced this pull request Jan 15, 2024
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