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

Add Jitsi screensharing support in electron app #4967

Merged
merged 4 commits into from
Sep 25, 2017

Conversation

superdump
Copy link
Contributor

This is dependent on: matrix-org/matrix-react-sdk#1355

I don't like that I had to copy code for this. It is Apache v2.0 licensed code with no specific copyright header. However, the full https://github.com/jitsi/jitsi-meet-electron-utils is a native electron module that would significantly complicate its integration when we just want to use this small amount of JS code. Comments/help welcome on this.

@superdump superdump requested a review from dbkr September 4, 2017 07:38
@@ -21,7 +21,7 @@ import VectorBasePlatform, {updateCheckStatusEnum} from './VectorBasePlatform';
import dis from 'matrix-react-sdk/lib/dispatcher';
import { _t } from 'matrix-react-sdk/lib/languageHandler';
import Promise from 'bluebird';
import {remote, ipcRenderer} from 'electron';
import electron, {remote, ipcRenderer} from 'electron';
Copy link
Member

Choose a reason for hiding this comment

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

why not just import desktopCapturer into the destructured imports instead of importing the whole of electron
you're already "slighltly-modifying" the copied code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@dbkr dbkr merged commit 94855c0 into develop Sep 25, 2017
@t3chguy t3chguy deleted the rob/electron-screensharing branch May 12, 2022 09:00
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.

3 participants