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

feat: add generic iframe #9286

Closed
wants to merge 6 commits into from

Conversation

DanielHabenicht
Copy link

This will add a generic IFrame quite similar to the etherpad IFrame, but with the following in mind:

  • you can use pre defined template strings for roomname and language (which can be extended in the future)
  • makes another application available in jitsi (because etherpad and the whiteboard are quite common requirements)

This will fix #5295. Note that only one shared app can be open at a time, the other will be close automatically.

The preconfigured text is quite general:

image

But I would suspect people will overwrite it in their deployment to fit their application name (e.g. Open shared whiteboard)

@jitsi-jenkins
Copy link

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

@DanielHabenicht
Copy link
Author

DanielHabenicht commented Jun 9, 2021

Any Updates? I signed the CLA today :)

@damencho
Copy link
Member

damencho commented Jun 9, 2021

I will talk offline with rest of the team about it.
There is one thing that worries me and I think it need to be changed.
You had implemented it like etherpad, but that has many problems. People were complaining that when speaker changes or someone joins ... The document is being closed.
In my opinion both need to be implemented as fake participants ... like the YouTube video sharing feature.

@damencho
Copy link
Member

damencho commented Jun 9, 2021

But I don't want to make you do changes before talking to design and product people.

@DanielHabenicht
Copy link
Author

DanielHabenicht commented Jun 10, 2021

Found a Bug in the mean time:

image

Does someone happen to know why the button gets stripped from the moderator view?
(only for the first load of the conference)

@damencho
Copy link
Member

So only moderator opens new document ... Isn't it, and after that participants will see it ...

@DanielHabenicht
Copy link
Author

Partly:

  1. Navigate to jitsi (where you can only start a meeting as a moderator)
  2. Login as Moderator
  3. --> newly added Button is not displayed
  4. Simple reload the site (without needing to login again) --> newly added button is displayed

or as simple user:
(someone else starts a meeting, and user can join anonymously)

  1. Load jitsi meeting
  2. --> Button is displayed

@DanielHabenicht
Copy link
Author

@damencho Any updates on the internal discussion?

@DanielHabenicht
Copy link
Author

DanielHabenicht commented Jul 1, 2021

So only moderator opens new document ... Isn't it, and after that participants will see it ...

I think I found the issue for the moderator bug, but I don't know where to fix it. Etherpad has a listener which gets called here:

jitsi-meet/conference.js

Lines 2212 to 2216 in 2174368

room.addCommandListener(this.commands.defaults.ETHERPAD,
({ value }) => {
APP.UI.initEtherpad(value);
}
);

and here:
conference.addCommandListener(ETHERPAD_COMMAND,
({ value }) => {
let url;

But I can't find where to add the COMMAND that they are subscribing to. (searched whole codebase but did not find it)
Any tip on where to send the Command for genericiframe?

@damencho
Copy link
Member

damencho commented Jul 1, 2021

@damencho Any updates on the internal discussion?

They said ok. The concerns were that there should be a way to control stuff, so no random URLs can be used, but I saw the template in the config which exactly does that (haven't looked at the code).

Are you opening the document as etherpad or as youtbue video share?

I'm just out of resources to look at the PR, sorry hope to be less busy in a month or so ...

@DanielHabenicht
Copy link
Author

DanielHabenicht commented Jul 1, 2021

@damencho Any updates on the internal discussion?

They said ok. The concerns were that there should be a way to control stuff, so no random URLs can be used, but I saw the template in the config which exactly does that (haven't looked at the code).

Are you opening the document as etherpad or as youtbue video share?

I'm just out of resources to look at the PR, sorry hope to be less busy in a month or so ...

I've basically copied the code of the etherpad and implemented it in a more general way to allow templating in the URL. That's why I am asking what I might have missed. (Because etherpad gets the commands send to the listener but I am not receiving them for my implementation)

@damencho
Copy link
Member

damencho commented Jul 1, 2021

I was looking at some point at the etherpad code and was thinking we should remove that, and implement it the way the shared video is done, with a fake participant and a thumbnail.
For example, there are multiple reports that etherpad automatically closes when speaker changes ... and in all UI logic we are taking care of the way shared video is implemented and the etherpad way is neglected as we do not use it ...

@DanielHabenicht
Copy link
Author

So how are we going to proceed? Implement it and then make a refactoring?

One other question: Did you receive the agreement from my company, so that I can now use my Company Email? (daniel.habenicht@t-systems.com)
I am going to update the PR soon.

@saghul
Copy link
Member

saghul commented Jul 28, 2021

@DanielHabenicht Damencho is out for a few days, he'll get back to you.

@damencho
Copy link
Member

damencho commented Aug 4, 2021

One other question: Did you receive the agreement from my company, so that I can now use my Company Email? (daniel.habenicht@t-systems.com)
I am going to update the PR soon.

Yes, it is received.

@damencho
Copy link
Member

damencho commented Aug 4, 2021

Implement it and then make a refactoring?

Can you change it, the way shared video is implemented?

@stale
Copy link

stale bot commented Jan 8, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix Issue won't be fixed label Jan 8, 2022
@gigabyteservice
Copy link

When whiteboard is added inside the webview it’s really important that teacher, presenter or moderator video thumbnail will visible over the whiteboard through etherpad webview in react native application. If someone think about that then it’s really helpful

@stale stale bot removed the wontfix Issue won't be fixed label Feb 11, 2022
@DanielHabenicht
Copy link
Author

DanielHabenicht commented Feb 18, 2022

Implement it and then make a refactoring?

Can you change it, the way shared video is implemented?

@damencho does this still hold true?
I've got assigned to update our branch with the latest jitsi version, but much has changed, so i am looking at implementing it the right way.

@damencho
Copy link
Member

@DanielHabenicht yes.

@DanielHabenicht
Copy link
Author

@DanielHabenicht yes.

@damencho I added a different PR with the described feature. Please have a look.

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.

Shared drawing canvas / whiteboard
5 participants