feat(setting): hide zoom and speed setting, only show when select track#143
Conversation
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Settings Panel Rendering Refactor src/components/video-editor/SettingsPanel.tsx |
Replaced zoomEnabled and trimEnabled boolean logic with ID-based conditional rendering. Zoom, trim, and speed UI sections now render only when their corresponding selected IDs exist. Trim deletion UI excluded when zoom is active to prevent overlap. Removed disabled button states and placeholder messages for inactive controls. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
🐰✨ A bunny's delight in logic refined!
Where booleans once scattered, IDs now align,
Zoom, trim, and speed dance in perfect view,
No overlap, no confusion—just controls anew!
Settings shine bright, the panel's renewed! 🎬
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately describes the main change: conditionally hiding zoom and speed settings, showing them only when tracks are selected. |
| Description check | ✅ Passed | The PR description follows the template with all key sections completed: Description, Motivation, Type of Change (with selections), Related Issue(s), Testing Guide, and Checklist. The content is clear and substantive. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/video-editor/SettingsPanel.tsx (1)
2047-2055:⚠️ Potential issue | 🟠 MajorClear
selectedSpeedIdwhen deleting from this footer.Line 2048 only calls
onSpeedDelete, butsrc/components/video-editor/timeline/TimelineEditor.tsx:787-792also callsonSelectSpeed(null)after deleting. With the footer now hidden/shown fromselectedSpeedId, this path can leave the speed dock visible against a stale selection.Suggested fix
- <Button - onClick={() => selectedSpeedId && onSpeedDelete?.(selectedSpeedId)} + <Button + onClick={() => { + if (!selectedSpeedId) return; + onSpeedDelete?.(selectedSpeedId); + onSelectSpeed?.(null); + }}// Outside this hunk, either clear selection inside the parent onSpeedDelete // implementation or thread the setter through SettingsPanelProps: onSelectSpeed?: (id: string | null) => void;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/SettingsPanel.tsx` around lines 2047 - 2055, The delete button only calls onSpeedDelete(selectedSpeedId) but does not clear the UI selection, leaving the speed dock visible for a stale selectedSpeedId; update the SettingsPanel delete flow to also clear the selection by invoking onSelectSpeed(null) after a successful delete (or ensure the parent implementation of onSpeedDelete clears selection), and if necessary add onSelectSpeed?: (id: string | null) => void to SettingsPanelProps so SettingsPanel can call it; adjust the handler connected to the Button (and any parent delete handlers used by TimelineEditor.tsx) to guarantee selectedSpeedId is reset to null after deletion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 2047-2055: The delete button only calls
onSpeedDelete(selectedSpeedId) but does not clear the UI selection, leaving the
speed dock visible for a stale selectedSpeedId; update the SettingsPanel delete
flow to also clear the selection by invoking onSelectSpeed(null) after a
successful delete (or ensure the parent implementation of onSpeedDelete clears
selection), and if necessary add onSelectSpeed?: (id: string | null) => void to
SettingsPanelProps so SettingsPanel can call it; adjust the handler connected to
the Button (and any parent delete handlers used by TimelineEditor.tsx) to
guarantee selectedSpeedId is reset to null after deletion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ec393280-58a1-493f-98b7-ff203c6c96d1
📒 Files selected for processing (1)
src/components/video-editor/SettingsPanel.tsx
2 participants
Description
This PR modifies the
SettingsPanelto conditionally show the Zoom Level and Playback Speed settings in the bottom dock. Previously, these panels were always visible but disabled when no region was selected; now, the entire dock is hidden and only appears when a corresponding zoom, trim, or speed track item is active on the timeline.Motivation
To reduce UI clutter and provide a more focused editing experience. Since these settings are context-dependent (requiring a specific timeline region to be selected), they are now treated as dynamic UI elements that only surface when relevant to the user's current selection.
Type of Change
Related Issue(s)
#119
Testing Guide
Checklist
Summary by CodeRabbit