refactor: Resizable Video Editor Layout, Migrated inline styles to TailwindCSS#228
Merged
siddharthvaddem merged 1 commit intosiddharthvaddem:mainfrom Mar 17, 2026
Conversation
Contributor
📝 WalkthroughWalkthroughThe VideoEditor layout is restructured from a vertical stack to a horizontal three-panel PanelGroup: a left video panel (with playback and controls), a central resizable Timeline panel, and a right Settings panel. PanelResizeHandle components enable adjustable widths; existing callbacks and data bindings are preserved. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
prayaslashkari
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/video-editor/VideoEditor.tsx (1)
1235-1244: Deduplicate video dimension/aspect-ratio derivation.The same
videoPlaybackRef.current?.videofallback logic appears in multiple places. Extracting shared derived values once per render reduces drift risk between preview and settings calculations.Also applies to: 1384-1389
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 1235 - 1244, Extract the repeated fallback logic around videoPlaybackRef.current?.video into a single derived-constants block at the top of the render (e.g., const currentVideo = videoPlaybackRef.current?.video, const currentVideoWidth = currentVideo?.videoWidth || 1920, const currentVideoHeight = currentVideo?.videoHeight || 1080), then use those constants wherever the code calls getNativeAspectRatioValue(...) or otherwise reads video dimensions (including the calls inside the VideoEditor component that reference cropRegion and getAspectRatioValue). Replace every inline occurrence of videoPlaybackRef.current?.video?.videoWidth/videoHeight with currentVideoWidth/currentVideoHeight and pass those into getNativeAspectRatioValue and related calculations to ensure a single source of truth per render and eliminate duplicated fallback logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1295-1297: Add explicit resize cursor and keyboard focus-visible
styles to the PanelResizeHandle elements to improve affordance and
accessibility: update the PanelResizeHandle (the instances rendering the div
with className "w-8 h-1 bg-white/20 rounded-full") to include cursor-col-resize
(or a responsive cursor for horizontal/vertical as appropriate) and
focus-visible utility classes (e.g., outline-none focus-visible:ring-2
focus-visible:ring-offset-1 focus-visible:ring-indigo-500 or similar) so
keyboard users see a clear focus ring and pointer users see a resize cursor;
apply the same change to both PanelResizeHandle occurrences (the handle around
the small bar near the first instance and the repeating instance at the second
location).
---
Nitpick comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1235-1244: Extract the repeated fallback logic around
videoPlaybackRef.current?.video into a single derived-constants block at the top
of the render (e.g., const currentVideo = videoPlaybackRef.current?.video, const
currentVideoWidth = currentVideo?.videoWidth || 1920, const currentVideoHeight =
currentVideo?.videoHeight || 1080), then use those constants wherever the code
calls getNativeAspectRatioValue(...) or otherwise reads video dimensions
(including the calls inside the VideoEditor component that reference cropRegion
and getAspectRatioValue). Replace every inline occurrence of
videoPlaybackRef.current?.video?.videoWidth/videoHeight with
currentVideoWidth/currentVideoHeight and pass those into
getNativeAspectRatioValue and related calculations to ensure a single source of
truth per render and eliminate duplicated fallback logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8075ce45-3568-4a51-8e79-4cfa9dbbe204
📒 Files selected for processing (1)
src/components/video-editor/VideoEditor.tsx
prayaslashkari
force-pushed
the
feature/resizeable-video-editor
branch
from
987a7c6 to
9a5d94a
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/video-editor/VideoEditor.tsx (1)
1295-1297:⚠️ Potential issue | 🟡 MinorAdd resize cursor + focus-visible styles on handles.
Line 1295 and Line 1340 still miss explicit resize cursor and keyboard focus-ring styles, which weakens discoverability/accessibility.
Suggested patch
-<PanelResizeHandle className="bg-[`#09090b`]/80 hover:bg-[`#09090b`] transition-colors rounded-full flex items-center justify-center"> +<PanelResizeHandle className="bg-[`#09090b`]/80 hover:bg-[`#09090b`] transition-colors rounded-full flex items-center justify-center cursor-row-resize focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#34B27B`]/70"> <div className="w-8 h-1 bg-white/20 rounded-full"></div> </PanelResizeHandle> -<PanelResizeHandle className="h-full bg-[`#09090b`]/80 hover:bg-[`#09090b`] transition-colors rounded-full flex items-center justify-center"> +<PanelResizeHandle className="h-full bg-[`#09090b`]/80 hover:bg-[`#09090b`] transition-colors rounded-full flex items-center justify-center cursor-col-resize focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#34B27B`]/70"> <div className="w-1 h-8 bg-white/20 rounded-full"></div> </PanelResizeHandle>Also applies to: 1340-1342
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 1295 - 1297, The PanelResizeHandle elements (component name: PanelResizeHandle) are missing explicit resize cursors and keyboard focus-visible ring styles; update the className for each PanelResizeHandle instance (the one around the small div at the split panes) to include a proper resize cursor (e.g., cursor-col-resize or cursor-row-resize as appropriate) and focus-visible utilities (e.g., focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-1 focus-visible:ring-color) so keyboard users get a visible focus ring and pointer users see the resize cursor; ensure both occurrences (the handle near the top and the one near line ~1340) are updated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1295-1297: The PanelResizeHandle elements (component name:
PanelResizeHandle) are missing explicit resize cursors and keyboard
focus-visible ring styles; update the className for each PanelResizeHandle
instance (the one around the small div at the split panes) to include a proper
resize cursor (e.g., cursor-col-resize or cursor-row-resize as appropriate) and
focus-visible utilities (e.g., focus:outline-none focus-visible:ring-2
focus-visible:ring-offset-1 focus-visible:ring-color) so keyboard users get a
visible focus ring and pointer users see the resize cursor; ensure both
occurrences (the handle near the top and the one near line ~1340) are updated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53c09b6e-3e42-4948-aa1a-3d686686b331
📒 Files selected for processing (1)
src/components/video-editor/VideoEditor.tsx
prayaslashkari
changed the title
prayaslashkari
changed the title
8 tasks
Closed
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Template
Description
Motivation
Made the panels resizable both horizontally and vertically with added constraints.
Type of Change
Related Issue(s)
Screenshots / Video
n/a
Video (if applicable):
resizeable-windows.mov
Testing
Checklist
Thank you for contributing!
Summary by CodeRabbit