Adding Room type with Converse.js replacing jitsi #385

rene-grigory · 2021-03-23T04:37:45Z

Issue

Jitsi Rooms using the jitsi-meet for chat only consume a large quantity of processing resources making the rooms unusable at around 75 users.

Solution

Created a room type called CHATSTREAM. CHATSTREAM is very similar to an IFRAME room type except it does not make use of JitsiVideo. Instead, it uses a third party library called Converse.js.

The minified code for Converse.js was downloaded and installing in the app/client/js folder
The CSS is pulled in from a CDN (we could download this locally, but there was no noticeable blocking pulling from the CDN)
- Note: the original code uses SASS and Bootstrap for generating the CSS. Rather than pull all of the required Bootstrap and SASS folders from the source I decided it would be better to create an _app/client/styles/chatStreamRoom.scss to override the necessary styles.
- To set the color for the theme the SASS uses the $backgound-color variable

Additional Comments

I noticed that the app/client/node_modules was not in the _.gitignore file. I added it; we can undo if desired.
Converse.js injects the DOM Element at the bottom of the body so I made two changes to deal with this:
1. I moved the <script /> tags into the and applied the defer attribute. This causes the scripts to loaded while the DOM is being parsed and then executes the scripts after parsing, but prior to rendering in the order the scripts were called
2. I used absolute positioning to force the Converse DOM Element into a "reasonable" position on the page
  - I also kept a div with the jitsi-video class in the page to keep the left side opened for the Converse DOM Element to appear in
3. There is a log going on in the useEffect() method. We can Zoom or chat as needed.

hndrewaall · 2021-03-23T04:42:35Z

This looks really good! I'll check it out locally and play around with it. @morganecf could you take a look as well?

hndrewaall · 2021-03-23T04:51:54Z

For everyone's benefit, here's a screenshot from my local env:

hndrewaall · 2021-03-23T04:52:47Z

With this is it possible to control chat colors from the theme?

hndrewaall · 2021-03-23T04:55:50Z

I triggered this somehow

hndrewaall · 2021-03-23T04:56:44Z

Also it seems users aren't cleaned up. Is there a timeout we can control?

EDIT: They do indeed timeout!

hndrewaall · 2021-03-23T05:00:11Z

Also this is pretty minor, but the rooms don't seem to be listed in the chat server (as in, from the CLI client). I wonder if there's a setting that controls that in the client. There should be some way to make them public/listed/whatever

hndrewaall · 2021-03-23T05:02:28Z

Timeouts aside, I wonder if it's possible to hook room leave/window close/other things to explicitly have users leave rooms

hndrewaall · 2021-03-23T05:09:07Z

Also do you have any idea what the "hidden message" thing is supposed to do?

hndrewaall · 2021-03-23T05:21:50Z

BTW so this is what happens when you join with the same name:

I can see both pros and cons to this approach

rene-grigory · 2021-03-23T13:25:29Z

Some responses:

With this is it possible to control chat colors from the theme? I'm currently using the $background-color variable to set the borders/icons, but if we want to change other colors we'll need to identify which areas on the screen and which SASS variables to use.

I triggered this somehow No clue how you did that or what it even means.

Also it seems users aren't cleaned up. Is there a timeout we can control? It seems that Prosody requires that each room has an "owner". It assigns that role to the first user to join the room. If the "owner" leaves the room then Prosody generates a fake "owner": [email protected]. I have no idea how to deal with that.

Also this is pretty minor, but the rooms don't seem to be listed in the chat server (as in, from the CLI client). Sorry, no clue on my part.

Timeouts aside, I wonder if it's possible to hook room leave/window close/other things to explicitly have users leave rooms I should be able to do that by listening to the onBeforeUnload event (actually, looks like react-beforeunload is a dep so I can use that).

Also do you have any idea what the "hidden message" thing is supposed to do? I'm not sure what "hidden message" are you talking about.

BTW so this is what happens when you join with the same name: As far as the multiple nicks: since we don't force unique aliases I can see how having 10 Omens in a chat might get confusing. Allowing the users to alter their name for a given chat seems like a good idea to me. FYI: this is being enforced by Converse/Prosody; not me.

rene-grigory · 2021-03-23T13:41:04Z

I've added the beforeUnload listener to clean out the user.

hndrewaall · 2021-03-23T18:15:42Z

I see some settings here that may be relevant to things i mentioned: https://prosody.im/doc/modules/mod_muc

I will try playing around on the backend!

rene-grigory · 2021-03-23T18:49:50Z

So, I see that the "owner" is treated as if it is offline. In fact, when I have multiple users in the room and and one leaves (logs out), they still show as offline. I came across some initialization settings for the UI:

hide_offline_users: true,
muc_fetch_members: false,

These settings do prevent the standard offline members from showing in the list, however, the "owner" still shows. If I could hide that I would feel better. I'll look into it some more this evening.

hndrewaall · 2021-03-23T18:53:47Z

I think we might be able to disable the requirement for owners sticking around. I'm specifically looking at that "persist" option in the docs I linked

hndrewaall · 2021-03-23T19:12:44Z

BTW I am seeing a few console errors from the chat. Here's one I seem to see every time I load the chat:

hndrewaall · 2021-03-23T19:13:25Z

Another

hndrewaall · 2021-03-23T19:14:18Z

BTW this is the "Hidden Message" feature I mentioned. I don't know what it's supposed to do, but I don't think it's working

rene-grigory · 2021-03-23T23:16:49Z

It's a rather sill feature. See my screens shots below:

morganecf · 2021-03-23T23:17:46Z

Thanks for this @rene-grigory! I'll take a look tomorrow :)

hndrewaall · 2021-03-23T23:20:10Z

It's a rather sill feature. See my screens shots below:

okay this is basically what i expected. weird that it doesn't seem to do that in my env. i wonder what's broken

rene-grigory · 2021-03-23T23:21:27Z

I went to their site and opened a demo and I see the same error in the console that @hndrewaall reported:

morganecf

I played around with it for a bit and it looks awesome!

I was wondering whether we could control the user's converse chat profile picture, to stay consistent with the rest of the app. I was poking around the code and it seems like we can use the vcard API to update profile information: https://github.com/conversejs/converse.js/blob/master/src/headless/plugins/vcard.js#L291-L315

It expects a base64 encoded image, so we'd have to download and encode the jitsi user's avatar, which can be accessed here: https://github.com/morganecf/jitsi-party/blob/master/app/client/jsx/Room.jsx#L82

I ended up getting a proof of concept working with the following, where the image is just a square with a diffusion of red in it:

this._converse.api.listen.on("VCardsInitialized", () => {
            this._converse.api.vcard.set(this._converse.bare_jid, {
              image: 'iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg=='
            })
})

This doesn't have to be done in this PR, of course! I figured I'd share since I ended up following the question down a rabbit hole.

EDIT - I realized this may have been unclear so here's a screenshot of what I mean, where the user's default avatar is replaced with a custom one:

morganecf · 2021-03-25T21:23:03Z

app/client/jsx/ChatStreamRoom.jsx

+          plugins = this._converse.pluggable.plugins;
+
+          // add event listener to restucture the chatbox
+          this._converse.api.listen.on("chatBoxInsertedIntoDOM", () => {


Apparently the chatBoxInsertedIntoDOM event will be removed in the next major version: https://github.com/conversejs/converse.js/blob/master/CHANGES.md

We could replace it with one of these events: https://github.com/conversejs/converse.js/blob/master/src/plugins/minimize/index.js#L128-L133

morganecf · 2021-03-25T21:31:12Z

app/client/jsx/Room.jsx

+import { WebSocketApi } from "./WebAPI.jsx";
+import LocalStorage from "./LocalStorage.jsx";
+import Config from "./Config.jsx";
+import { ChatStreamRoom } from "./ChatStreamRoom.jsx";



The diff in this file is a little confusing because of what I presume is linting…is there any chance you could commit without the linting? That way our history will be cleaner and reviews will be easier!

morganecf · 2021-03-25T21:31:52Z

app/client/jsx/Room.jsx

+              "emphasis",
+              "strong",
+              "delete",
+              "link",


Nice! This is great. I test it out just to see and it looks good:

morganecf · 2021-03-25T21:34:31Z

app/config/schema/rooms.proto

+    // CHATSTREAM ROOMS ONLY
+    // Options related to the bosh service
+    // Required for chat streaming rooms.
+    string boshUrl = 17;


Should this be in Room? Since boshUrl is always the same, does it need to be part of the room config? Can we make it part of the general config?

Good question: I was just following the precedent, but it could certainly be generalized.

hndrewaall · 2021-03-31T18:23:48Z

@rene-grigory do you think you will be able to get this updated to address morg's feedback this weekend? I would really like to get this merged ASAP, as we will need to start working on the new theme(s) soon (and those will depend on any new theme knobs this generates)

…e events fire after the chatbox is inserted into the DOM

rene-grigory · 2021-04-01T21:14:30Z

I think I have addressed all of @morganecf's concerns.

morganecf · 2021-04-02T15:35:42Z

app/client/jsx/ChatStreamRoom.jsx

-          });
+          //   title.innerHTML = roomName;
+          //   dd.remove();
+          // });


Can we get rid of the comments?

morganecf · 2021-04-02T15:38:32Z

Thanks for getting to all the feedback! I think it looks good -- I would just remove that commented code and also make a ticket for the avatars linking to my initial comment above, so we can get to that later.

rene-grigory · 2021-04-02T23:09:19Z

Oops, good catch.

rene-grigory · 2021-04-08T03:29:11Z

@morganecf & @hndrewaall, I've added a number of styling variables for the Chat and emoji popup. I think everything is ready to be merged in.

hndrewaall · 2021-04-08T03:54:43Z

This looks really, really good. Thank you @rene-grigory ! I will leave to @morganecf for final approval

morganecf · 2021-04-08T04:14:37Z

app/client/styles/themes/_tst.scss

+$chat-participants: $fuckyou-turquoise; // the participants sidebar on the right of the chat
+$chat-participants-resize: $matte-blue; // color of the participants resize handle
+$participants-title: $matte-blue; // the title of the participants list
+$participants-name: $matte-blue; // the name of each participant in the list


Thanks for adding these comments, that's very helpful

morganecf · 2021-04-08T04:15:43Z

Looks good to me!

dbrudnicki and others added 17 commits March 14, 2021 13:25

initial converse inclusion with creation of streaming room

5adf316

Merge branch 'master' into chat

7c00673

cleaned up converse init

f865b07

pushing for andrew to look at

671a839

initial configuration for converse

02165d2

Merge branch 'master' into chat

dfde39e

merged from upstream

1d57567

merged from upstream

c10398c

removed node_modules from git

dfcdb08

created auto logout when leaving the room

09b1df7

got rid of needless code

1fe98b3

ready for styling and cleanup

b85be73

finished hijacking css

f155948

Merge branch 'master' into chat

7c00179

revered iframe link

21f35c3

code clean up

fd36859

Merge branch 'master' into chat

d7599ec

fixed comment in rooms.proto

80adc18

logout user in beforeUnload

9c99bb2

hndrewaall requested a review from morganecf March 23, 2021 19:15

morganecf reviewed Mar 25, 2021

View reviewed changes

rene-grigory added 2 commits March 25, 2021 18:30

hide offline users and brought in the latest convers.js lib

a093676

Merge branch 'master' into chat

85c5117

rene-grigory added 3 commits April 1, 2021 09:42

format cleanup

5165ca7

more cleanup and useing Config.baseUrl to point at the bash service

30346f8

replaced old listener with MutationObserver since none of the convers…

4eaa9ed

…e events fire after the chatbox is inserted into the DOM

morganecf reviewed Apr 2, 2021

View reviewed changes

removed comment and fixed console.error

83df75c

rene-grigory mentioned this pull request Apr 4, 2021

Create Avatars for Chat Room members #387

Open

added style variables for the chat box

f3b7421

morganecf reviewed Apr 8, 2021

View reviewed changes

hndrewaall merged commit 8949218 into morganecf:master Apr 8, 2021

Adding Room type with Converse.js replacing jitsi #385

Adding Room type with Converse.js replacing jitsi #385

Conversation

rene-grigory commented Mar 23, 2021 • edited Loading

Issue

Solution

Additional Comments

hndrewaall commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

hndrewaall commented Mar 23, 2021 • edited Loading

hndrewaall commented Mar 23, 2021 • edited Loading

hndrewaall commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

rene-grigory commented Mar 23, 2021 • edited Loading

rene-grigory commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

rene-grigory commented Mar 23, 2021 • edited Loading

hndrewaall commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

rene-grigory commented Mar 23, 2021

morganecf commented Mar 23, 2021

hndrewaall commented Mar 23, 2021

rene-grigory commented Mar 23, 2021

morganecf left a comment • edited Loading

Choose a reason for hiding this comment

morganecf Mar 25, 2021

Choose a reason for hiding this comment

morganecf Mar 25, 2021

Choose a reason for hiding this comment

morganecf Mar 25, 2021

Choose a reason for hiding this comment

morganecf Mar 25, 2021

Choose a reason for hiding this comment

rene-grigory Mar 25, 2021

Choose a reason for hiding this comment

hndrewaall commented Mar 31, 2021

rene-grigory commented Apr 1, 2021 • edited Loading

morganecf Apr 2, 2021

Choose a reason for hiding this comment

morganecf commented Apr 2, 2021

rene-grigory commented Apr 2, 2021

rene-grigory commented Apr 8, 2021

hndrewaall commented Apr 8, 2021

morganecf Apr 8, 2021

Choose a reason for hiding this comment

morganecf commented Apr 8, 2021

rene-grigory commented Mar 23, 2021 •

edited

Loading

hndrewaall commented Mar 23, 2021 •

edited

Loading

hndrewaall commented Mar 23, 2021 •

edited

Loading

rene-grigory commented Mar 23, 2021 •

edited

Loading

rene-grigory commented Mar 23, 2021 •

edited

Loading

morganecf left a comment •

edited

Loading

rene-grigory commented Apr 1, 2021 •

edited

Loading