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

Conversation

cpettet
Copy link
Contributor

@cpettet cpettet commented Feb 25, 2025

What is this PR doing?

Description

For small viewport devices, renders the right panel as full-screen.

Screens and GIFs (updated screenshots 2025.02.26)

gif-smaller
screenshot_2025-02-26_at_2 16 30___pm_480
Screenshot 2025-02-26 at 2 18 18 PM

How should this be manually tested?

Visual inspection on mobile
  • join a meeting room with ~8 people
  • for each panel (chat, participant-list, issues)
  • change the following line from closed to the panel name
# frontend/src/hooks/useRightPanel.tsx, line#25
activeTab: 'closed',
  • when testing chat, send maybe 10 messages and ensure that nothing is broken visually
  • when testing participant-list, ensure there's some vertical scrolling on mobile. ~8 participants should be enough, but add as many as necessary.
  • when testing issues, add to the fields, attach a screenshot, and spamnotify our channel 🙈
  • open a meeting room page on mobile and on desktop
  • ensure there's no strangeness visually

What are the relevant tickets?

A maintainer will add this ticket number.

Resolves VIDCS-3322

Checklist

[✅] Branch is based on develop (not main).
[ ] Resolves a Known Issue.
[ ] If yes, did you remove the item from the docs/KNOWN_ISSUES.md?
[ ] Resolves an item reported in Issues.
If yes, which issue? Issue Number?

behei-vonage
behei-vonage previously approved these changes Feb 25, 2025
Copy link
Contributor

@behei-vonage behei-vonage left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀
it's that 100dvh everywhere 😂

@@ -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

Copy link
Collaborator

@v-kpheng v-kpheng left a comment

Choose a reason for hiding this comment

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

LGTM! 💪 🚀

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 🙏

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.

Copy link
Contributor

@behei-vonage behei-vonage left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Copy link
Collaborator

@maikthomas maikthomas left a comment

Choose a reason for hiding this comment

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

Nice one!

@@ -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.

👏

@@ -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.

👏

@dwivedisachin
Copy link
Collaborator

Tested LGTM!! 🚀

@cpettet cpettet merged commit 2cc5721 into develop Feb 27, 2025
7 checks passed
@cpettet cpettet deleted the cpettet/vidcs-3322-right-panel-work branch February 27, 2025 19:30
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.

5 participants