-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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/lobby chat #10847
Feat/lobby chat #10847
Conversation
Hi, thanks for your contribution! |
@gpatel-fr we resolved most of the issues you raised in the previous pull request.
|
Hey @doganbros we deployed it here https://lobby-chat.jitsi.net/ so we can preview the feature ... but there is something wrong, when you click the chat button in the notification or participants pane it does not work ... can you check it out, please. |
@damencho probably the |
Good point I will check it tomorrow, I think I restarted prosody... But maybe I missed that, will see. |
Yep, you were right, now it works. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lib-jitsi-meet part is merged :) Congrats and thank you!
@@ -158,6 +158,20 @@ function filter_stanza(stanza) | |||
elseif stanza.name == 'iq' and stanza:get_child('query', DISCO_INFO_NS) then | |||
-- allow disco info from the lobby component | |||
return stanza; | |||
elseif stanza.name == 'message' then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be checking here whether the message is from moderator or to moderator ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @damencho we currently allow private messages and also messages only to a moderator from line 163 to 173. (Or should we implement it differently). I think we are left with checking messages from a moderator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me check again, when I was looking at it I had the impression that lobby users can send messages to lobby users privately or via group messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you allow all messages between lobby participants and group messages to moderators. So I was assuming group chats between participants are possible ... but there are not.
Can you easily check and do it: "messages to and from moderators are only allowed"... ?
It shouldn't be possible to find other participants ids, but just in case the check ^ seems better, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, we will check on that
react/features/lobby/actions.any.js
Outdated
@@ -241,3 +250,149 @@ export function hideLobbyScreen() { | |||
visible: false | |||
}; | |||
} | |||
|
|||
/** ...................................... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you drop the dots :)
{ this._renderStandardButtons() } | ||
</div> | ||
); | ||
} | ||
|
||
|
||
/** .................... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop and these, thank you.
@damencho I see... Will start to work on 1 and 3. Thank you too. |
Hello I have not experienced any crash or unconsistent behaviour on my test instance. A minor nit is that when there is one moderator and several candidates, the moderator workflow is not optimal. On the code I have noticed this small strangeness in chat/actions.any.js:
boolean for a participant ID ?? There is something not quite right here. -> should remove messages from chat (works only for accepted users)
if (knocking && lobbyMessageRecipient && lobbyMessageRecipient.id === participantId) this part shows clearly that this function can be called with a non moderator ID. (remark) On the prosody code, there is also a small nit: your code is duplicating a similar part in the local lobby_room = lobby_muc_service.get_room_from_jid(lobby_room_jid); if it makes sense to test the get_room_from_jid function in the presence handling, maybe it would make sense to test it also at this point. Last - I would like to apologize for my first review, on one minor vocabulary point I was overly harsh and my tone was not appropriate. I can only say I was tired at the time. Anyway, cheers for the very impressive work ! |
@gpatel-fr Thank you for your review and for looking deep into the PR. That is a great help! |
Thank you on be half of the team. Will check your comments... |
1ace000
to
d6227db
Compare
@damencho few quick notes about the updates:
|
@damencho any update regarding to new UI design? |
Hum, I was waiting for some replay ... Will check it tomorrow and will let you know. |
I think all I mentioned before is the latest state. So I have these designs: Yeah do not do anything outside of the feature you are doing to keep it simple. |
Sure. Will check the designs. |
I pass your question, to design ... that link I was given for mobile designs. |
Hi everyone, |
Of course don't implement anything that is not for this feature. I think I had summarized earlier what are the needed changes. About the chat area in lobby is that there is no point of the marking of the messages with different colors, both for mobile and web. In lobby the chat should look like a normal chat in the room. @mihaitiberiu can you also chime in. |
1b7a82b
to
5e6b7cf
Compare
@damencho Is it possible to give some priority to our PR? It will be perfect if we can ship the feature this week. |
Jenkins test this please |
lobby chat support knocking participants list updates knocking participants conditonal checks to show message button handle lobby chat message events lobby messages from or to moderators only Co-authored-by: Fecri Kaan Ulubey <f.kaan93@gmail.com>
Linting problems are fixed now |
Jenkins test this please |
I will try to look at it today and tomorrow. |
Jenkins test this please |
Congrats! Thank you for your contribution. |
Thank you too. |
* feat(lobby): lobby chat lobby chat support knocking participants list updates knocking participants conditonal checks to show message button handle lobby chat message events lobby messages from or to moderators only Co-authored-by: Fecri Kaan Ulubey <f.kaan93@gmail.com> * squash: Drop typos. Co-authored-by: Kusi Musah Hussein <kusimusah@gmail.com> Co-authored-by: Fecri Kaan Ulubey <f.kaan93@gmail.com> Co-authored-by: Дамян Минков <damencho@jitsi.org>
Better moderation capabilities in locked down rooms: challenge/response in lobby #9714
Lobby chat support
Knocking participants list updates
Knocking participants conditonal checks to show message button
Handle lobby chat events
Allow messages in mod_muc_lobby_rooms.lua