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

VIDCS-3322: Implement full-screen right panel on mobile #97

Merged
merged 15 commits into from
Feb 27, 2025
Merged
2 changes: 1 addition & 1 deletion frontend/src/components/MeetingRoom/Chat/Chat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export type ChatProps = {
*/
const Chat = ({ handleClose, isOpen }: ChatProps): ReactElement | false => {
const { messages } = useSessionContext();
const heightClass = '@apply h-[calc(100vh_-_240px)]';
const heightClass = '@apply h-[calc(100dvh_-_240px)]';
Copy link
Collaborator

Choose a reason for hiding this comment

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

👏


return (
isOpen && (
Expand Down
1 change: 1 addition & 0 deletions frontend/src/components/MeetingRoom/Emoji/Emoji.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const Emoji = ({ emojiWrapper }: EmojiProps): ReactElement => {
animationTimingFunction: 'linear',
animationIterationCount: 1,
maxWidth: '35%',
zIndex: 1,
Copy link
Contributor Author

@cpettet cpettet Feb 26, 2025

Choose a reason for hiding this comment

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

Make emojis visible, even when someone has their panel/toolbar opened

};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ const ParticipantList = ({ handleClose, isOpen }: ParticipantListProps): ReactEl
</Tooltip>
</IconButton>
</div>
<List sx={{ overflowX: 'auto', height: 'calc(100vh - 240px)' }}>
<List sx={{ overflowX: 'auto', height: 'calc(100dvh - 240px)' }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

👏

<ParticipantListItem
key="you"
dataTestId="participant-list-item-you"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
REPORT_NAME_LIMIT,
} from '../../../../utils/constants';
import HelperText from './HelperText';
import useIsSmallViewport from '../../../../hooks/useIsSmallViewport';

export type FormType = {
title: string;
Expand Down Expand Up @@ -58,15 +59,17 @@ const FeedbackForm = ({
loading,
onFileSelect,
}: FeedbackFormType): ReactElement => {
const heightClass = '@apply h-[calc(100vh_-_240px)]';
const isSmallViewport = useIsSmallViewport();
const heightClass = '@apply h-[calc(100dvh_-_240px)]';
const widthClass = isSmallViewport ? '@apply w-[calc(100dvw_-_48px)]' : '';

const getColorStyle = (value: string, maxLength: number) => {
return value.length >= maxLength || value.length === 0 ? 'red' : 'inherit';
};

return loading ? (
<Box
sx={{ display: 'flex', justifyContent: 'center', alignItems: 'center', minHeight: '100vh' }}
sx={{ display: 'flex', justifyContent: 'center', alignItems: 'center', minHeight: '100dvh' }}
>
<CircularProgress />
</Box>
Expand Down Expand Up @@ -196,15 +199,14 @@ const FeedbackForm = ({
<FilePicker onFileSelect={onFileSelect} />
</Box>
</div>
<div className="absolute inset-x-12 bottom-6 flex">
<div className={`${widthClass} bottom-6 mx-[24px] flex`}>
<Button
type="submit"
variant="contained"
fullWidth
sx={{
textTransform: 'none',
fontSize: '1rem',
width: '18rem',
}}
>
Send
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,36 @@
import { render, screen, fireEvent } from '@testing-library/react';
import { describe, expect, it, vi, Mock } from 'vitest';
import { describe, expect, it, vi, Mock, beforeEach } from 'vitest';
import FilePicker from './FilePicker';
import * as util from '../../../../../utils/util';
import '@testing-library/jest-dom';

vi.mock('../../../../../utils/util', () => ({ isMobile: vi.fn() }));

describe('FilePicker component', () => {
const mockFileSelect = vi.fn();

beforeEach(() => {
(util.isMobile as Mock).mockImplementation(() => false);
});

it('renders the "Add screenshot" button initially', () => {
render(<FilePicker onFileSelect={mockFileSelect} />);
const addButton = screen.getByText(/add screenshot/i);
expect(addButton).toBeInTheDocument();
});

it('renders the "Capture screenshot" button initially', () => {
render(<FilePicker onFileSelect={mockFileSelect} />);
const addButton = screen.getByText(/capture screenshot/i);
expect(addButton).toBeInTheDocument();
describe('"Capture screenshot" button', () => {
it('is rendered on desktop devices', () => {
render(<FilePicker onFileSelect={mockFileSelect} />);
const addButton = screen.getByText(/capture screenshot/i);
expect(addButton).toBeInTheDocument();
});

it('is not rendered on mobile devices', () => {
(util.isMobile as Mock).mockImplementation(() => true);
const addButton = screen.queryByText(/capture screenshot/i);
expect(addButton).not.toBeInTheDocument();
});
});

it('uploads and previews a valid image file', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ChangeEvent, useRef, useState, ReactElement } from 'react';
import { Button, IconButton, Tooltip, Typography } from '@mui/material';
import { Delete } from '@mui/icons-material';
import captureScreenshot from '../../../../../utils/captureScreenshot';
import { isMobile } from '../../../../../utils/util';

// Setting the maximum file size to 20MB
const maxFileSize = 2e7;
Expand Down Expand Up @@ -92,14 +93,22 @@ const FilePicker = ({
)}
{!imageSrc ? (
<>
<Button
sx={{ width: '100%', textTransform: 'none', mb: 1 }}
variant="outlined"
component="label"
onClick={processScreenshot}
>
Capture screenshot
</Button>
{!isMobile() && (
// The screenshot capture relies on the getDisplayMedia browser API which is unsupported on mobile devices
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can use OT.checkScreenSharingCapability() instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's keep as-is since this utility is a little simpler 🙏

// See: https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getDisplayMedia#browser_compatibility
<Button
sx={{
width: '100%',
textTransform: 'none',
mb: 1,
}}
variant="outlined"
component="label"
onClick={processScreenshot}
>
Capture screenshot
</Button>
)}
<Button
sx={{ width: '100%', textTransform: 'none' }}
variant="outlined"
Expand Down
24 changes: 13 additions & 11 deletions frontend/src/components/MeetingRoom/RightPanel/RightPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import ParticipantList from '../ParticipantList/ParticipantList';
import Chat from '../Chat';
import ReportIssue from '../ReportIssue';
import type { RightPanelActiveTab } from '../../../hooks/useRightPanel';

const height = '@apply h-[calc(100vh_-_96px)]';
import useIsSmallViewport from '../../../hooks/useIsSmallViewport';

export type RightPanelProps = {
handleClose: () => void;
Expand All @@ -21,16 +20,19 @@ export type RightPanelProps = {
* @returns {ReactElement} RightPanel Component
*/
const RightPanel = ({ activeTab, handleClose }: RightPanelProps): ReactElement => {
const isSmallViewport = useIsSmallViewport();
const width = isSmallViewport ? 'w-dvw' : 'w-[360px]';
const margins = isSmallViewport ? 'm-0' : 'mr-4 mt-4';
const height = isSmallViewport
? '@apply h-[calc(100dvh_-_80px)]'
: '@apply h-[calc(100dvh_-_96px)]';
const className = `${height} absolute top-0 ${margins} ${width} overflow-hidden rounded bg-white transition-[right] ${activeTab === 'closed' ? 'right-[-380px] hidden' : 'right-0'}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the other reviewers, looks like mostly light refactoring.


return (
<div
data-testid="right-panel"
className={`${height} absolute top-0 mr-4 mt-4 w-[360px] overflow-hidden rounded bg-white transition-[right] ${activeTab === 'closed' ? 'right-[-380px] hidden' : 'right-0'}`}
>
<div>
<ParticipantList handleClose={handleClose} isOpen={activeTab === 'participant-list'} />
<Chat handleClose={handleClose} isOpen={activeTab === 'chat'} />
<ReportIssue handleClose={handleClose} isOpen={activeTab === 'issues'} />
</div>
<div data-testid="right-panel" className={className}>
<ParticipantList handleClose={handleClose} isOpen={activeTab === 'participant-list'} />
<Chat handleClose={handleClose} isOpen={activeTab === 'chat'} />
<ReportIssue handleClose={handleClose} isOpen={activeTab === 'issues'} />
</div>
);
};
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/pages/MeetingRoom/MeetingRoom.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import useRoomName from '../../hooks/useRoomName';
import isValidRoomName from '../../utils/isValidRoomName';
import useIsSmallViewport from '../../hooks/useIsSmallViewport';

const height = '@apply h-[calc(100dvh_-_80px)]';

/**
* MeetingRoom Component
*
Expand Down Expand Up @@ -82,7 +84,7 @@ const MeetingRoom = (): ReactElement => {
}, [publishingError, navigate, roomName]);

return (
<div data-testid="meetingRoom" className="w-screen bg-darkGray-100">
<div data-testid="meetingRoom" className={`${height} w-screen bg-darkGray-100`}>
{isSmallViewPort && <SmallViewportHeader />}
<VideoTileCanvas
isSharingScreen={isSharingScreen}
Expand Down
Loading