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

Manage 'on stage' request directly in viewers list #1443

Merged

Conversation

roro-lv
Copy link
Contributor

@roro-lv roro-lv commented Mar 23, 2022

Purpose

For now, 'on stage' requests appear under the video on the instructor view. The goal of this PR is to keep this functionnal, but to put it directly in the viewers list instead. The desired result is shown below :

image

🚩 The 'invite' function is not implemented here, see #1438

@roro-lv roro-lv requested review from kernicPanel and lunika March 23, 2022 11:33
@roro-lv roro-lv self-assigned this Mar 23, 2022
@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch from 378d83a to 4fa9376 Compare March 23, 2022 13:33
@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch 6 times, most recently from 891d2b2 to 9bf3eba Compare March 24, 2022 13:36
@roro-lv roro-lv requested review from lunika and kernicPanel March 24, 2022 13:41
@roro-lv roro-lv removed the WIP label Mar 24, 2022
@roro-lv roro-lv marked this pull request as ready for review March 24, 2022 13:41
Copy link
Member

@kernicPanel kernicPanel left a comment

Choose a reason for hiding this comment

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

Nice work !
Just some minor suggestions.

Copy link
Member

@lunika lunika left a comment

Choose a reason for hiding this comment

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

Nice improvement. I have a question. As a student I was able to leave the discussion when I was on stage. I didn't find how to do it here. Maybe I missed something ?

@lunika
Copy link
Member

lunika commented Mar 29, 2022

As a student I was able to leave the discussion when I was on stage. I didn't find how to do it here. Maybe I missed something ?

The hand icon allowing to leave the discussion is still present, I didn't notice it 👍

@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch from 6516558 to 108ab95 Compare March 29, 2022 16:18
@roro-lv roro-lv requested a review from RVassili March 30, 2022 07:44
@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch from 4c02954 to 13147ed Compare March 30, 2022 10:41
@roro-lv roro-lv requested review from kernicPanel and lunika March 30, 2022 12:18
Copy link
Member

@lunika lunika left a comment

Choose a reason for hiding this comment

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

On firefox I have this rendering for the notification

Screenshot from 2022-03-30 16-24-49

@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch from e3935e7 to 0512ff4 Compare April 4, 2022 16:16
@RVassili RVassili force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch 2 times, most recently from 980d5ba to 96b7cd6 Compare April 5, 2022 08:25
@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch 3 times, most recently from f6ac6a9 to 5162c0b Compare April 5, 2022 09:31
Copy link
Member

@lunika lunika left a comment

Choose a reason for hiding this comment

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

const AvatarBox = styled(Box)`
// 2 last digits correspond to the alhpa of the hexcolor, in hexadecimal
background-color: ${normalizeColor('blue-active', theme)}19;
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you not using the Box's property 'background' ?

// minWidth is set otherwise avatar width is crushed by the margin of the component next to the avatar
min-width: 24px;
min-width: 26px;
Copy link
Contributor

Choose a reason for hiding this comment

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

you shouldn't need this

@@ -0,0 +1,179 @@
import React, { useMemo } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

to delete

@@ -0,0 +1,102 @@
import React, { useMemo } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

to delete

@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch from 2ca348a to 6baeb04 Compare April 5, 2022 15:50
@roro-lv roro-lv requested a review from RVassili April 5, 2022 15:53
@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch 4 times, most recently from 512f2fa to 4086278 Compare April 5, 2022 16:35
@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch 2 times, most recently from c94a16d to 51c1bb6 Compare April 6, 2022 08:18
roro-lv added 6 commits April 6, 2022 11:38
Add the jid in ParticipantType and update the participantTrackingPlugin in
order to record this information jointly to the name of the user.
Avatar was a little too small (a 24px square instead of  a 26px one) and color
was not exactly the same as on the mockup.
This badge indicates how many students are actually requesting to go on stage.
Add a new component ViewersList, which is in charge of displaying the viewers
list, and the on stage management buttons, depending on if the user is an
instructor or not. This commit also add all necessaries components needed for
ViewersList to work properly.
The StudentViewersList is not used anymore and the ViewersList replaces it. The
Viewers component makes some little changes to the UI of the viewers list and
add 'on-stage request' management features directly into the list.
The 'on-stage request' mangement system has been moved into the viewers list,
so the old components displayed under the video are not needed anymore.
@roro-lv roro-lv force-pushed the feature/roro-lv/manage-on-stage-request-in-viewers-list branch from 51c1bb6 to 1617e34 Compare April 6, 2022 10:03
@roro-lv roro-lv merged commit 96850f5 into master Apr 6, 2022
@roro-lv roro-lv deleted the feature/roro-lv/manage-on-stage-request-in-viewers-list branch April 6, 2022 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants