-
Notifications
You must be signed in to change notification settings - Fork 4
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
Conversation
There was a problem hiding this 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 😂
…ge/vonage-video-react-app into cpettet/vidcs-3322-right-panel-work
@@ -24,6 +24,7 @@ const Emoji = ({ emojiWrapper }: EmojiProps): ReactElement => { | |||
animationTimingFunction: 'linear', | |||
animationIterationCount: 1, | |||
maxWidth: '35%', | |||
zIndex: 1, |
There was a problem hiding this comment.
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
There was a problem hiding this 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 🙏
frontend/src/components/MeetingRoom/ReportIssue/FeedbackForm/FilePicker/FilePicker.tsx
Show resolved
Hide resolved
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'}`; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀
There was a problem hiding this 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)]'; |
There was a problem hiding this comment.
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)' }}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
|
Tested LGTM!! 🚀 |
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)
How should this be manually tested?
Visual inspection on mobile
chat
,participant-list
,issues
)closed
to the panel namespamnotify our channel 🙈What are the relevant tickets?
A maintainer will add this ticket number.
Resolves VIDCS-3322
Checklist
[✅] Branch is based on
develop
(notmain
).[ ] 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?