Grid mode #651
Grid mode #651
Conversation
There was a problem hiding this comment.
I think this looks really good! Excited to have this implemented.
I left a couple of questions about some comments/logs that might be able to be removed. And also a question about leaving a comment for participants' audio tracks. Just curious to hear your thoughts.
Also, are we going to be mimicking the dominant speaker behavior from the video composer at some point (green highlight around speaker, special ordering of participants)?
src/stories/mocks/twilio-video.js
Outdated
| import { action } from '@storybook/addon-actions'; | ||
| import EventEmitter from 'events'; | ||
|
|
||
| // navigator.permissions = false; |
There was a problem hiding this comment.
Can we remove this comment or is it needed?
| if (args[0] === '/token') { | ||
| return Promise.resolve({ | ||
| ok: true, | ||
| json: () => Promise.resolve({ token: 'yay' }), |
src/stories/App.stories.jsx
Outdated
| layout: 'fullscreen', | ||
| argTypes: { | ||
| participants: { | ||
| control: { type: 'range', min: 0, max: 100, step: 1 }, |
There was a problem hiding this comment.
Should max be 50? I noticed that when I run story book and have 50+ participants, I couldn't test out the dominant speaker feature for any participant > 51. Or is it because we are planning on supporting 100 participants in the future?
There was a problem hiding this comment.
Good catch! Looks like it has to do with this line here. The for loop was only processing the first 50 participants, even though the storybook slider went up to 100. I changed them both to 200.
| @@ -63,7 +63,7 @@ export default function useLocalTracks() { | |||
| if (isAcquiringLocalTracks || audioTrack || videoTrack) return Promise.resolve(); | |||
|
|
|||
| setIsAcquiringLocalTracks(true); | |||
|
|
|||
| console.log(3); | |||
| return ( | ||
| <div | ||
| className={clsx(classes.container, { | ||
| [classes.rightDrawerOpen]: isChatWindowOpen || isBackgroundSelectionOpen, | ||
| })} | ||
| > | ||
| <MainParticipant /> | ||
| <ParticipantList /> | ||
| <ParticipantAudioTracks /> |
There was a problem hiding this comment.
I'm wondering if we should have a comment explaining <ParticipantAudioTracks /> and why it is necessary for grid mode? I see you left one in Publication.tsx but I'm wondering if others will be confused as to why audio tracks aren't treated the same was as video tracks. Just a thought 😅
There was a problem hiding this comment.
Good idea! I added a comment.
Good question! Yes, but there's another ticket for that. For now, the Grid Mode is using the same dominant speaker behavior as the Collaboration Mode - which isn't ideal, but it's good enough for now. After this we'll make the Grid Mode dominant speaker work the same as the Video Componser. |
Contributing to Twilio
Pull Request Details
JIRA link(s):
Description
This PR add a basic Grid Mode to the app. In this mode, participants will be rendered in an adaptive grid format, with a maximum of 25 participants (for now).
To try the grid mode, you can just use the app with many participants, or you can run
npm run storybookto try out the app in a mocked video room.More changes:
Burndown
Before review
npm testBefore merge