fix: changing playback time while video is playing #968
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughSign-in overlay classes were reordered and a typo in the sign-in text was fixed. Timeline playhead handling was made asynchronous: on drag while playing it stops playback, seeks to the computed frame, and restarts playback using Tauri commands (FPS/OUTPUT_SIZE); playbackTime is clamped and updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Timeline as Timeline Component
participant Tauri as Tauri Commands
User->>Timeline: Drag / mouse move on playhead
Timeline->>Timeline: compute left,bounds -> rawTime -> clamp newTime
alt editorState.playing == true
Timeline->>Tauri: stopPlayback()
Tauri-->>Timeline: ok / error
Timeline->>Tauri: seekTo(frame = round(newTime * FPS))
Tauri-->>Timeline: ok / error
Timeline->>Tauri: startPlayback(FPS, OUTPUT_SIZE)
Tauri-->>Timeline: ok / error
Note right of Timeline: If playing changed to false during awaits, bail and set playbackTime
else playing == false
Note right of Timeline: Only update playbackTime
end
Timeline->>Timeline: setPlaybackTime(newTime)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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 (3)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
373-373: Announce sign-in state to screen readersAdd a live region/role so assistive tech is notified when sign-in starts.
- <span>Signing In...</span> + <span role="status" aria-live="polite">Signing In...</span>apps/desktop/src/routes/editor/Timeline/index.tsx (2)
10-12: Verify Tauri bindings sourceConfirm that
~/utils/tauriexports the generated tauri_specta bindings (commands). If not, switch to the generated bindings module to stay aligned with our desktop guidelines and avoid stringly-typed invokes.
91-101: Guard against rapid re-entry to stop/seek/start loop when draggingIf
handleUpdatePlayheadcan fire repeatedly while playing, consider de-bouncing or serializing seeks (e.g., a simple in-flight flag or requestId token) to avoid thrashing the backend with overlapping stop/start cycles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx(1 hunks)apps/desktop/src/routes/editor/Timeline/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: In the desktop app, rely on unplugin-icons auto-imports; do not manually import icon modules
Use generated tauri_specta commands/events (commands, events) in the desktop frontend; listen to generated events directly
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/editor/Timeline/index.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsx
{apps/desktop,packages/ui-solid}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component naming (Solid): components in PascalCase; hooks/utilities in camelCase starting with 'use' where applicable
Files:
apps/desktop/src/routes/editor/Timeline/index.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
Files:
apps/desktop/src/routes/editor/Timeline/index.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/editor/Timeline/index.tsx (2)
apps/desktop/src/utils/tauri.ts (1)
commands(7-266)apps/desktop/src/routes/editor/context.ts (2)
FPS(46-46)OUTPUT_SIZE(48-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (1)
371-373: Typo fix and class reorder LGTM"Signing In..." correction and class reorder are fine and non-functional.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
Bug Fixes
Improvements