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

Video 8473 mobile grid mode #681

Merged
merged 39 commits into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
e30bb3f
use collab participants
olipyskoty May 3, 2022
4b12bcc
add useGridParticipants to app
olipyskoty May 6, 2022
d7fa279
update variable names
olipyskoty May 6, 2022
b255bf1
add binary search algo
olipyskoty May 10, 2022
d63937b
update failing tests for mocks and imports
olipyskoty May 10, 2022
5860f60
remove room prop from useGridParticipants
olipyskoty May 10, 2022
48290f5
change helper function back to arrow function
olipyskoty May 10, 2022
1918f2b
change border size
olipyskoty May 10, 2022
b67069a
storybook: randomize participant backgrounds
olipyskoty May 11, 2022
9cc5bdf
account for null values in useDominantSpeaker
olipyskoty May 11, 2022
399942c
update splice to account for local participant
olipyskoty May 11, 2022
b4557c8
update failing tests
olipyskoty May 11, 2022
5aeb1ed
change variable name from excludeNull to includeNull
olipyskoty May 11, 2022
24547e5
fix failing test
olipyskoty May 11, 2022
d74ff84
fix typo in test description
olipyskoty May 11, 2022
073712d
fix comment to accurately describe code
olipyskoty May 11, 2022
1c09e4c
fix import name
olipyskoty May 18, 2022
49beacf
fix typo in test description
olipyskoty May 18, 2022
a2bf2e5
update variable names for clarity
olipyskoty May 18, 2022
277b4aa
update if statement logic
olipyskoty May 18, 2022
84373a8
add comment to useParticipants
olipyskoty May 18, 2022
49f8e12
add grid mode to mobile menu
olipyskoty May 20, 2022
2a7e387
add mobile grid mode feature
olipyskoty May 20, 2022
b6118c6
get mobile grid mode working
olipyskoty May 20, 2022
b033574
add swiper mock
olipyskoty May 24, 2022
c6bcc0a
finalize mobile grid view styling
olipyskoty May 24, 2022
6635b6c
make network quality smaller for mobile grid view
olipyskoty May 24, 2022
fa1f5f0
Merge branch 'feature/grid-mode' into VIDEO-8473-mobile-grid-mode
olipyskoty May 24, 2022
0970696
package lock
olipyskoty May 24, 2022
96cc3f9
remove unnecessary styling from networkqualitylevel
olipyskoty May 24, 2022
1bc7269
switch statement refactor
olipyskoty May 24, 2022
b2864a7
add dominant speaker logic for mobile grid view
olipyskoty May 24, 2022
18778b4
add test case for isMobileGridActive prop
olipyskoty May 24, 2022
83d897d
update mock for GridView tests
olipyskoty May 24, 2022
0b710ea
revert networkqualitylevel changes
olipyskoty May 24, 2022
ce1e6bd
fix TS warning
olipyskoty May 24, 2022
fdb76b8
correct MobileGridView snapshot tests
olipyskoty May 24, 2022
1647b5c
remove mocked participants from room object and account for participa…
olipyskoty May 24, 2022
aebc1cc
fix typo in usegridparticipants
olipyskoty May 24, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"react-scripts": "4.0.3",
"rimraf": "3.0.2",
"strip-color": "^0.1.0",
"swiper": "^8.1.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there package-lock changes to commit?

"ts-node": "^9.1.1",
"twilio": "^3.63.1",
"twilio-video": "^2.20.1",
Expand Down
9 changes: 9 additions & 0 deletions src/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ import { shallow } from 'enzyme';
import useHeight from './hooks/useHeight/useHeight';
import useRoomState from './hooks/useRoomState/useRoomState';

jest.mock('swiper/react/swiper-react.js', () => ({
Swiper: jest.fn(),
SwiperSlide: jest.fn(),
}));

jest.mock('swiper', () => ({
Pagination: jest.fn(),
}));

Comment on lines +10 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

This is clearly needed to make the tests pass, but I can't understand why 😭 . Everything is shallow rendered. Do you have any ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jest is failing due to a syntax error: SyntaxError: Cannot use import statement outside a module. Sent you the screenshot in slack.. not entirely sure why mocking the file makes it pass though 🤔

jest.mock('./hooks/useRoomState/useRoomState');
jest.mock('./hooks/useHeight/useHeight');

Expand Down
32 changes: 15 additions & 17 deletions src/components/MenuBar/Menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,23 +135,21 @@ export default function Menu(props: { buttonClassName?: string }) {
<Typography variant="body1">Room Monitor</Typography>
</MenuItem>

{!isMobile && (
<MenuItem
onClick={() => {
setIsGridModeActive(isGrid => !isGrid);
setMenuOpen(false);
}}
>
<IconContainer>
{isGridModeActive ? (
<CollaborationViewIcon style={{ fill: '#707578', width: '0.9em' }} />
) : (
<GridViewIcon style={{ fill: '#707578', width: '0.9em' }} />
)}
</IconContainer>
<Typography variant="body1">{isGridModeActive ? 'Collaboration Mode' : 'Grid Mode'}</Typography>
</MenuItem>
)}
<MenuItem
onClick={() => {
setIsGridModeActive(isGrid => !isGrid);
setMenuOpen(false);
}}
>
<IconContainer>
{isGridModeActive ? (
<CollaborationViewIcon style={{ fill: '#707578', width: '0.9em' }} />
) : (
<GridViewIcon style={{ fill: '#707578', width: '0.9em' }} />
)}
</IconContainer>
<Typography variant="body1">{isGridModeActive ? 'Collaboration Mode' : 'Grid Mode'}</Typography>
</MenuItem>

<MenuItem onClick={() => setAboutOpen(true)}>
<IconContainer>
Expand Down
77 changes: 77 additions & 0 deletions src/components/MobileGridView/MobileGridView.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { MobileGridView } from './MobileGridView';
import { shallow } from 'enzyme';
import { useAppState } from '../../state';
import useVideoContext from '../../hooks/useVideoContext/useVideoContext';

const mockLocalParticipant = { identity: 'test-local-participant', sid: 0 };

jest.mock('swiper/react/swiper-react.js', () => ({
Swiper: jest.fn(),
SwiperSlide: jest.fn(),
}));

jest.mock('swiper', () => ({
Pagination: jest.fn(),
}));

jest.mock('../../hooks/useVideoContext/useVideoContext');
jest.mock('../../state');

const mockUseVideoContext = useVideoContext as jest.Mock<any>;
const mockUseAppState = useAppState as jest.Mock<any>;

mockUseAppState.mockImplementation(() => ({ maxGridParticipants: 9 }));
mockUseVideoContext.mockImplementation(() => ({
room: {
localParticipant: mockLocalParticipant,
participants: [],
},
}));

describe('the MobileGridView component', () => {
it('should render correctly when there is only the localParticipant', () => {
const wrapper = shallow(<MobileGridView />);
expect(wrapper).toMatchSnapshot();
});

it('should render correctly when there is one participant plus the localParticipant', () => {
mockUseVideoContext.mockImplementation(() => ({
room: {
localParticipant: mockLocalParticipant,
participants: [{ identity: 'test-participant-1', sid: 1 }],
},
}));
const wrapper = shallow(<MobileGridView />);
expect(wrapper).toMatchSnapshot();
});

it('should render correctly when there are two participants plus the localParticipant', () => {
mockUseVideoContext.mockImplementation(() => ({
room: {
localParticipant: mockLocalParticipant,
participants: [
{ identity: 'test-participant-1', sid: 1 },
{ identity: 'test-participant-2', sid: 2 },
],
},
}));
const wrapper = shallow(<MobileGridView />);
expect(wrapper).toMatchSnapshot();
});

it('should render correctly when there are 3 or more participants plus the localParticipant', () => {
mockUseVideoContext.mockImplementation(() => ({
room: {
localParticipant: mockLocalParticipant,
participants: [
{ identity: 'test-participant-1', sid: 1 },
{ identity: 'test-participant-2', sid: 2 },
{ identity: 'test-participant-3', sid: 3 },
{ identity: 'test-participant-4', sid: 4 },
],
},
}));
const wrapper = shallow(<MobileGridView />);
expect(wrapper).toMatchSnapshot();
});
});
95 changes: 95 additions & 0 deletions src/components/MobileGridView/MobileGridView.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import { CSSProperties } from 'react';
import { makeStyles } from '@material-ui/core';
import Participant from '../Participant/Participant';
import useDominantSpeaker from '../../hooks/useDominantSpeaker/useDominantSpeaker';
import useGridParticipants from '../../hooks/useGridParticipants/useGridParticipants';
import useVideoContext from '../../hooks/useVideoContext/useVideoContext';

import { Swiper, SwiperSlide } from 'swiper/react/swiper-react.js';
import 'swiper/swiper.min.css';
import 'swiper/modules/pagination/pagination.min.css';
import { Pagination } from 'swiper';
import { Participant as IParticipant } from 'twilio-video';

const useStyles = makeStyles({
participantContainer: {
position: 'absolute',
top: 0,
right: 0,
bottom: 0,
left: 0,
height: '100%',
'& .swiper': {
height: '100%',
Copy link
Contributor

Choose a reason for hiding this comment

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

Here I think we need to specify something so that we can see the inactive pagination dots. Digging through devtools, it looks like we can update the --swiper-pagination-bullet-inactive-color CSS variable to be white instead of black. I think adding this here seems to do the trick:

'--swiper-pagination-bullet-inactive-color': 'white',

Then we can see the total number of pages:
image

'--swiper-pagination-bullet-inactive-color': 'white',
},
'& .swiper-wrapper': {
height: '100%',
},
'& .swiper-slide': {
height: '90%', // To leave room for the pagination indicators
paddingBottom: '1em',
},
},
swiperSlide: {
display: 'flex',
flexWrap: 'wrap',
alignSelf: 'center',
alignContent: 'flex-start',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if alignContent is needed. Is it doing anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't look like it. It was at one point when I was messing around with the CSS, but looks like it is useless now 🙂

},
});

export function MobileGridView() {
const classes = useStyles();
const { room } = useVideoContext();
const participants = useGridParticipants(true);
const dominantSpeaker = useDominantSpeaker(true);
const remoteParticipantCount = participants.length;

const pages: IParticipant[][] = [[]];
// Add the localParticipant to the front of the array to ensure they are always the first participant:
pages[0].push(room!.localParticipant);

for (let i = 0; i < remoteParticipantCount; i++) {
const pageNumber = Math.floor(i / 6);
if (!pages[pageNumber]) {
pages[pageNumber] = [];
}
// Each page should have a max of 6 participants:
if (pages[pageNumber].length < 6) {
pages[pageNumber].push(participants[i]);
} else {
pages[pageNumber + 1] = [participants[i]];
}
}

const participantVideoStyles: CSSProperties = {
width: remoteParticipantCount < 3 ? '100%' : '50%',
// The height of each participant's video is determined by the number of participants on the grid
// page. Here the array indices represent a remoteParticipantCount. If the count is 4 or greater,
// the height will be 33.33%
height: ['100%', '50%', '33.33%', '50%', '33.33%'][Math.min(remoteParticipantCount, 4)],
padding: '0.2em',
boxSizing: 'border-box',
};

return (
<div className={classes.participantContainer}>
<Swiper pagination={true} modules={[Pagination]} className="mySwiper">
{pages.map((page, i) => (
<SwiperSlide key={i} className={classes.swiperSlide}>
{page.map(participant => (
<div style={participantVideoStyles} key={participant.sid}>
<Participant
participant={participant}
isLocalParticipant={room!.localParticipant === participant}
isDominantSpeaker={dominantSpeaker === participant}
/>
</div>
))}
</SwiperSlide>
))}
</Swiper>
</div>
);
}
Loading