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(webcam): avatar loading #14938

Merged

Conversation

frankemax
Copy link
Collaborator

@frankemax frankemax commented May 3, 2022

What does this PR do?

Add a new avatar component to video list item
Change the design of the components, following the new video list idea suggested here #13844 (comment)
Add icons related to the state of the user: presenter / dialIn

Closes Issue(s)

Closes #12481

Points to review

  • I'm not really used to using styled components. If there is a better way to define the styles, feel free to suggest changes.
  • Several places on the site use avatar component. As his styles have been changed, it would be good to take a look to see if didn't break anything.
  • It will be nice to test out different image sizes and formats (GIFs) in the user avatar.

Screenshots

Presenter / dial In / moderator / client

When a user joins in a meeting, he will see this during of other users webcam loading:

image

loadingInAction2.mp4

Extended loading

loading.mp4

Talking indicator

TalkingIndicator.mp4

Squeezed

image

Reconnecting animation

Peek.2022-05-05.15-21.mp4

Avatar emoji status

Peek.2022-05-05.16-50.mp4

@prlanzarin prlanzarin self-requested a review May 3, 2022 17:19
@frankemax frankemax changed the title feat(video-list-item): avatar loading feat(webcam): avatar loading May 3, 2022
@prlanzarin prlanzarin added this to the Release 2.6 milestone May 3, 2022
@prlanzarin
Copy link
Member

cc @ramonlsouza feel free to assign yourself as a reviewer if you deem appropriate

@prlanzarin
Copy link
Member

Several places on the site use avatar component. As his styles have been changed, it would be good to take a look to see if didn't break anything.

@frankemax maybe you should extend the user-list avatar styles in a decoupled style sheet in video-list-etc? I think that may be feasible (and recommendable) from a very brief vertical glance since most of the changes done in the user-list styles are conditional to a isVideoContainer boolean.

@ramonlsouza ramonlsouza self-requested a review May 4, 2022 12:17
@frankemax
Copy link
Collaborator Author

@prlanzarin now it's ready for re-review

Add a new avatar component to video list item
Change the design of the components, following the new video list idea
Add icons related to the state of the user
Add correct prop types to user-status
Fix spinner
Add unhealthy stream filter to the avatar
Fix mirror own webcam not working
Probably broke after e45deb5
Fixes an issue that when the user joins with a custom avatar, the talking
indicator div ends up covering the avatar image.
@frankemax frankemax force-pushed the add-video-avatar-loading-2.6 branch from 61fa545 to 5152ce4 Compare May 9, 2022 19:44
@sonarcloud
Copy link

sonarcloud bot commented May 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 11 Code Smells

No Coverage information No Coverage information
2.5% 2.5% Duplication

Copy link
Member

@prlanzarin prlanzarin left a comment

Choose a reason for hiding this comment

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

This seems good as a first version.

A few pending things (which I discussed with @frankemax) that, IMO, should be addressed in subsequent PRs/patches:

  • There can be a tiny gap between the talking indicator frame and the video container corner due to the new border radius
  • The user name position was moved to the lower left side as a suggestion of mine to 1) make things easier 2) make the connecting vs connected views have consistent UI element positions. This probably warrants an (N)ACK from @tylercopeland once it lands on some testing server and he gets to use it.
  • Avatar user icons (presenter/dial in) do not disappear when the container elements are hidden due to constrained width. They should.
  • Review the camera container background color. I found the new one a bit confusing since it's very similar to the client's main background color. But that's my view, so cc @tylercopeland

@prlanzarin prlanzarin merged commit c6dedfe into bigbluebutton:develop May 10, 2022
@tylercopeland
Copy link
Collaborator

@prlanzarin

  • Updating the location of the name is fine by me. I like how it's visually the same as the video experience. So creating consistency between those two experience is a great idea.
  • In terms of a background colour, we can explore something darker. I can provide a colour code for this items.

@prlanzarin
Copy link
Member

@tylercopeland

Thanks.

In terms of a background colour, we can explore something darker. I can provide a colour code for this items.

Cool. As a suggestion: one of our designers suggested #3B4A5C a while ago when we were exploring a better background color for webcam letter-boxing. Maybe it fits this scenario as well?

@NarsimaChilkuri
Copy link

@frankemax Hi this implementation is working fine, but how to modify this in a way that this avatar placeholder will be previewed everytime user disables his/her webcam

@frankemax
Copy link
Collaborator Author

frankemax commented Jan 3, 2023

@NarsimaChilkuri
I'm not sure which is the best way. I know there is an issue for this: #13844

And designs here: #13844 (comment)

I believe that just changing the HTML5 you can reach a satisfactory result, but you would have to rethink the layout context logic and cameras grid in a way that resembles the proposed designs.

Starting with these points #13844 (comment) would be a good start...

@narsimachilkuri45
Copy link

@frankemax can you give me an overview what changes are required to establish the satisfactory result which i need

@narsimachilkuri45
Copy link

@frankemax i need to show placeholder of webcam connecting only if you stop video webcam

@frankemax
Copy link
Collaborator Author

frankemax commented May 1, 2023

@narsimachilkuri45 #13844 (comment)
Planned 2.7

BUT, if you want to try on your own, I'm doing this for the mobile-sdk right now, maybe this PR will inspire you:
mconf/bbb-mobile-sdk#167

@narsimachilkuri45
Copy link

@frankemax it is for mobile sdk right, if possible can you guide me for the changes in bigbluebutton-html5 for web version, so i can implement it over there

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.

Webcams: allow using avatars as webcam container placeholders
6 participants