feat(zoom): hold-to-preview button for zoom focus editing (prototype for #612)#613
Conversation
When a zoom region is selected and paused, the editor shows the full un-zoomed frame for focus-point placement. This adds a press-and-hold "Preview" button so editors can momentarily see the zoomed result at the current focus + depth — like a before/after compare — without entering playback. - VideoPlayback: new transient isPreviewingZoom prop; shouldShowUnzoomedView now also requires !isPreviewingZoom, so the zoom transform is applied at the playhead while previewing - VideoEditor: isPreviewingZoom state wired to VideoPlayback and to onZoomPreviewStart/End handlers - SettingsPanel: hold button in the zoom controls (pointer down/up/leave/ cancel) - i18n: zoom.previewHold added across all 11 locales Prototype for siddharthvaddem#612 — placement (panel vs overlay) and hold-vs-toggle still open for maintainer direction.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds hold-to-preview gesture for zoom across three components: SettingsPanel renders a conditional button with pointer/keyboard/blur event handlers calling optional callbacks, VideoEditor tracks ChangesZoom Preview Hold-to-Preview Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes The changes are straightforward: prop threading through three components, a small ref sync pattern, and repetitive i18n key additions. The button event handling is moderately detailed but follows standard React patterns. No complex logic or edge cases to reason through—mostly wiring and UI. Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. Comment |
Make the label explicit about what holding previews (the zoom effect), across all 11 locales.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 947-958: The preview hold button should only render when both
lifecycle callbacks exist and must support keyboard users: change the render
condition from (onZoomPreviewStart || onZoomPreviewEnd) to (onZoomPreviewStart
&& onZoomPreviewEnd) and keep the pointer handlers
(onPointerDown/Up/Leave/Cancel) on the Button; additionally add keyboard
handlers onKeyDown (trigger onZoomPreviewStart for Space/Enter and guard against
repeat), onKeyUp and onBlur (trigger onZoomPreviewEnd) so the preview lifecycle
(start → end) is fully wired for keyboard and pointer input, referencing the
existing zoomEnabled, onZoomPreviewStart, onZoomPreviewEnd, and the Button
element that renders t("zoom.previewHold").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e4c6348-90b7-4c62-8039-470d6b3863c0
📒 Files selected for processing (14)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/i18n/locales/ar/settings.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/ja-JP/settings.jsonsrc/i18n/locales/ko-KR/settings.jsonsrc/i18n/locales/ru/settings.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/vi/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-TW/settings.json
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24ce67b5a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const hasSelectedZoom = selectedId !== null; | ||
| const shouldShowUnzoomedView = hasSelectedZoom && !isPlayingRef.current; | ||
| const shouldShowUnzoomedView = | ||
| hasSelectedZoom && !isPlayingRef.current && !isPreviewingZoomRef.current; |
There was a problem hiding this comment.
When the playhead is not inside the selected zoom (for example, clicking a zoom item only selects it; TimelineEditor does not seek), holding Preview disables the unzoomed edit guard but still relies on findDominantRegion(... currentTime ...). In that scenario the button previews no zoom or a different zoom at the playhead, while the settings panel is editing the selected zoom's focus/depth, so users cannot verify the selected region unless they manually seek into it first.
Useful? React with 👍 / 👎.
- VideoPlayback: while holding Preview, render the SELECTED zoom at full strength regardless of the playhead, instead of whatever findDominantRegion returns at currentTime (which is none/another zoom when the playhead is outside the selection). Uses getZoomScale/getRotation3D for the region's configured scale and 3D preset. - SettingsPanel: require both onZoomPreviewStart && onZoomPreviewEnd to render the button (full lifecycle), and add keyboard support — Space/Enter keydown (repeat-guarded) starts preview, keyup/blur ends it.
…t frame The a686fa0 override replaced findDominantRegion's resolved region with the raw stored region (forcing strength=1 / transition=null). findDominantRegion already resolves focus via getResolvedFocus — for focusMode:"auto" it interpolates the cursor-followed focus from telemetry and applies clamp/blend/ transition. The override bypassed all of that, so previewing an auto-focus zoom showed a stale static focus and an instant un-eased zoom that did not match real playback/export. Hold-to-preview now shows the natural zoom for the current playhead frame (true before/after compare). The isPreviewingZoom flag is kept — it only disables the un-zoomed editing guard so findDominantRegion's result is shown. Previewing while the playhead is outside any zoom shows no zoom by design.
|
Re: "Preview the selected zoom, not the playhead region" — reverted the selected-zoom override in The override (forcing Hold-to-preview is intentionally scoped to the current playhead frame — a true before/after compare of how the zoom actually renders at that moment. When the playhead sits outside any zoom there is nothing to preview, which is acceptable: you preview while parked on the region you're editing. The The keyboard a11y + |
|
screen recording would be appreciated 🙏 |
Got it! Just like this — and I can preview what the zoomed-in version of the selected region looks like 👀https://github.com/user-attachments/assets/99950b12-5ea6-4e52-a4bc-a7ac00a07809 |
# Conflicts: # src/components/video-editor/VideoPlayback.tsx
2 participants
Draft prototype for #612.
What
When a zoom region is selected and paused, the editor shows the full un-zoomed frame so you can place the focus point. This adds a press-and-hold "Preview" button in the zoom controls: while held, the preview renders the selected zoom at the current focus + depth (before/after compare); on release it returns to the un-zoomed editing view.
Interaction is hold-to-preview (not a toggle): the zoomed view is shown only while the button is held, so it doubles as a quick before/after compare.
How
VideoPlayback: new transientisPreviewingZoomprop;shouldShowUnzoomedViewnow also requires!isPreviewingZoom, so the zoom transform is applied at the playhead while previewing. No change to focus-point math or the default editing view.VideoEditor:isPreviewingZoomstate wired down toVideoPlaybackand toonZoomPreviewStart/End.SettingsPanel: hold button in the zoom controls (pointer down/up/leave/cancel).zoom.previewHoldadded across all 11 locales (i18n-check passes).Open for maintainer direction (kept minimal on purpose)
Test plan
npm testpassesSummary by CodeRabbit
Release Notes