fix: enable fullscreen for preview panel #399
Conversation
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
|
@karansingh-dev is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
""" WalkthroughA new ESLint configuration tailored for Next.js and TypeScript was added to the web app, along with updates to the project's dependencies to support ESLint. The preview panel component was enhanced to enable fullscreen functionality via the toolbar's expand button, including error handling for fullscreen requests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PreviewToolbar
participant PreviewPanel
participant Browser
User->>PreviewToolbar: Clicks fullscreen button
PreviewToolbar->>PreviewPanel: Calls onExpand()
PreviewPanel->>Browser: Requests fullscreen on preview container
alt Success
Browser-->>PreviewPanel: Fullscreen activated
else Failure
PreviewPanel->>User: Shows alert (fullscreen failed)
end
Estimated code review effort2 (~10–30 minutes) Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/components/editor/preview-panel.tsx (1)
46-56: Improve error handling and user experience.The fullscreen implementation is functional but has some areas for improvement:
- Using
alert()provides poor UX - consider using a toast notification instead- The error handling could be more specific about different failure scenarios
- Consider checking for fullscreen support more comprehensively
- //handle fullScreen - const handleFullScreen = () => { - // Only attempt fullscreen if ref is available - if (previewRef.current && previewRef.current.requestFullscreen) { - previewRef.current.requestFullscreen().catch(() => { - alert( - "Failed to enter fullscreen. Your browser may block this operation." - ); - }); - } - }; + // Handle fullscreen functionality + const handleFullScreen = () => { + if (!previewRef.current) return; + + // Check for fullscreen API support + if (!document.fullscreenEnabled) { + console.warn('Fullscreen is not supported or enabled in this browser'); + return; + } + + previewRef.current.requestFullscreen().catch((error) => { + console.error('Failed to enter fullscreen:', error); + // Consider using a toast notification here instead of alert + }); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/web/.eslintrc.json(1 hunks)apps/web/package.json(2 hunks)apps/web/src/components/editor/preview-panel.tsx(3 hunks)
🔇 Additional comments (5)
apps/web/package.json (1)
70-71: LGTM! ESLint configuration looks appropriate.The addition of ESLint and eslint-config-next to devDependencies is well-placed and aligns with the new ESLint configuration file.
apps/web/.eslintrc.json (1)
1-6: LGTM! Standard Next.js ESLint configuration.The configuration properly extends the recommended Next.js presets for core web vitals and TypeScript support. This is the standard setup for Next.js projects.
apps/web/src/components/editor/preview-panel.tsx (3)
395-398: LGTM! Clean prop passing implementation.The fullscreen handler is properly passed to the PreviewToolbar component with clear naming and structure.
404-412: LGTM! Component interface properly updated.The component signature correctly reflects the new onExpand prop with proper TypeScript typing.
514-514: LGTM! Event handler properly connected.The onClick handler is correctly wired to trigger the fullscreen functionality.
1 participant
Description
This pull request implements fullscreen functionality for the preview panel in the editor.
Clicking the maximize button now correctly triggers the browser's Fullscreen API and maximizes the preview panel as expected.
Fixes #385
Adds a robust handler for entering fullscreen via the maximize button.
Adds error handling if the browser blocks fullscreen access.
Does not change UI/UX layout—panel appearance and borders remain as before.
Type of change
How Has This Been Tested?
Test Configuration:
Screen Recording
fix_.full.screen.button.in.preview-panel.tsx.mp4
Checklist:
Additional context
This PR focuses specifically on enabling fullscreen for the preview panel, matching the current issue requirements. If further UX improvements are desired—such as keeping the toolbar visible in fullscreen mode—I would be happy to propose and work on those in a separate, follow-up PR.
Summary by CodeRabbit
New Features
Chores