Azure / communication-ui-library

UI Library for Azure Communication Services helps developers build communication applications with ease. From turn-key composites to UI components that can be composited together.
https://aka.ms/acsstorybook
MIT License
169 stars 71 forks source link

White background of display name in video gallery component disappears when video stream is not available #3302

Closed guy-weavix closed 1 year ago

guy-weavix commented 1 year ago

When the video stream is available, the display name has a white background color. When the video stream is unavailable, the display name loses the white background color.

To reproduce this issue: create a page with video gallery component:

import {
    VideoGallery
  } from '@azure/communication-react';
import { Stack } from '@fluentui/react';

  import React from 'react';

  export const VideoGalleryComponents = (): JSX.Element => {
    const MockLocalParticipant = {
        userId: 'user1',
        displayName: 'You',
        state: 'Connected',
        isMuted: true
      };

      const mockVideoElement = document.createElement('div');
      mockVideoElement.innerHTML = '<span />';
      mockVideoElement.style.width = '100%';
      mockVideoElement.style.height = '100%';
      mockVideoElement.style.background = 'url(https://media.giphy.com/media/SwImQhtiNA7io/giphy.gif)';
      mockVideoElement.style.backgroundPosition = 'center';
      mockVideoElement.style.backgroundRepeat = 'no-repeat';

      const MockRemoteParticipants = [
        {
          userId: 'user2',
          displayName: 'Peter Parker'
        },
        {
          userId: 'user5',
          displayName: 'Bruce Wayne',
          videoStream: {
            isAvailable: true,
            renderElement: mockVideoElement
          }
        }
      ];

      const containerStyle = { height: '100vh', width: '400vw' };
      return (
        <div style={containerStyle}>
          <VideoGallery
            layout='floatingLocalVideo'
            localParticipant={MockLocalParticipant}
            remoteParticipants={MockRemoteParticipants}
            onRenderAvatar={onRenderAvatar}
          />
        </div>
    );
  };

  export const onRenderAvatar = (): JSX.Element => {
    return (
        <Stack style={{ height: '100%', width: '100%', borderRadius: '0', backgroundColor: '#252525' }}>
            <div style={{
                height: '100%',
                width: '100%',
                display: 'flex',
                justifyContent: 'center',
                alignItems: 'center',
                position: 'relative'
            }}>
                <img src="https://media.giphy.com/media/SwImQhtiNA7io/giphy.gif" alt='Guy' style={{ height: '100%', width: '100%' }} />
            </div>
        </Stack>
    );
}

I would expect that the background on the display of all of the video tiles would be styled the same regardless of if the video stream is available or not.

image

My environment is using these packages:

I am running my example on Windows 11 on a PC using Chrome browser (114.0.5735.199)

dmceachernmsft commented 1 year ago

Hi @guy-weavix Thanks for filing the issue!

Looking at your code snippet something stands out to me in the CSS to your mock element. image This no-repeat value might be what's disabling the white background.

According to the MDN documentation on that CSS property it will disable any background repetition. So when the onRenderAvatar function you created is applied inside your mockVideoElement, this would remove the white background applied. This is because the property would be inherited from the parent on the displayName bar.

I would suggest not using that property and create unique mock elements if you want different appearances.

Hope this helps! 😊

guy-weavix commented 1 year ago

The suggestion to remove the background repeat value didn't resolve the issue with the display names on the other tiles that didn't use the mockVideoElement variable as its "video stream". My example could be made better to show a different GIF for the onRenderAvatar since it is using the same image as the "mockVideoElement" therefore may be giving the impression that the "mockVideoElement" is being used more than once. I needed a GIF with a darker "background" to highlight the issue.

dmceachernmsft commented 1 year ago

Oh I see what you mean. Sorry about that, I think I see what is happening. can you remove the width and height properties to the onRenderAvatar function?

I believe what is happening is those Gifs you are seeing without the white displayName background are avatars that are sized like streams. What would be happening here then is the underlying CSS for those tiles are in the state for when the video streams for those users are off. In those instances the displayName background is unset since our default background is grey.

guy-weavix commented 1 year ago

Having removed the height/width within the onRenderAvatar function, it now looks like this:

image

If the display name's background is unset when the video is off, is there a way to override that behavior on all video tiles? In the video gallery's styles option, I was able to override the local video tile's display name styling, but that level of custom styling doesn't seem available to the remote video tiles.

dmceachernmsft commented 1 year ago

So you should be able to do that if you use our VideoTile component, you should have access to those styles from that component if you use it in the render override. if you just override the avatar for the user our default VideoTile configuration will render under the hood.

Our VideoTile component has a prop called styles defined as followed: image here is the storybook link for that component for more information on usage as well.

Hope this helps!

dmceachernmsft commented 1 year ago

Seems issue has been resolved. please reopen issue if you have any further problems! 😊