-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Handle timeline selection state in editor UI actions #1030
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughImplements selection-aware gating across editor UI: Header actions, ShareButton reupload, and ConfigSidebar tabs now detect an active timeline selection, clear it on click, and either stop or proceed. UI adjustments include conditional tab styling, hidden tab indicator during selection, z-index tweaks, and repositioned selection overlay. Tauri delete command now returns null. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Header / ShareButton / ConfigSidebar
participant Ctx as EditorContext
participant TL as Timeline State
participant Act as Action Handler
User->>UI: Click control
UI->>Ctx: Read editorState.timeline.selection
alt Selection active
UI->>Ctx: setEditorState("timeline","selection", null)
UI-->>User: Close selection (stop original action)
else No selection
UI->>Act: Proceed with original action (e.g., Export / Upload / Switch tab)
Act-->>User: Action result
end
sequenceDiagram
actor User
participant Tabs as ConfigSidebar Tabs
participant Ctx as EditorContext
participant View as Panel Content
User->>Tabs: Click tab trigger
Tabs->>Ctx: setEditorState("timeline","selection", null)
Tabs->>Tabs: Update selected tab
Tabs->>View: Scroll to top and render panel
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/desktop/src/routes/editor/ShareButton.tsx (2)
186-199: Align tooltip with gating when a selection exists.The click first clears selection and returns, but the tooltip still says “Reupload video”. Match Header’s UX by showing “Close selection” when a selection is active.
Apply this diff:
- <Tooltip - content={ - upload.isPending ? "Reuploading video" : "Reupload video" - } - > + <Tooltip + content={ + editorState.timeline.selection + ? "Close selection" + : upload.isPending + ? "Reuploading video" + : "Reupload video" + } + >
193-199: Optional: factor a small guard to DRY the “clear selection or run action” pattern.Keeps intent explicit and reduces repetition across components.
Add inside ShareButton (or a shared util):
const clearSelectionOr = (fn: () => void) => { if (editorState.timeline.selection) { setEditorState("timeline", "selection", null); return; } fn(); };Then update the handler:
- onClick={() => { - if (editorState.timeline.selection) { - setEditorState("timeline", "selection", null); - return; - } - upload.mutate(); - }} + onClick={() => clearSelectionOr(() => upload.mutate())}apps/desktop/src/routes/editor/ConfigSidebar.tsx (1)
245-246: Avoid flipping KTabs between a controlled value andundefined.Solid/Kobalte Tabs behave best with a stable controlled
value. You’re already hiding the indicator and neutralizing selected styles during a selection, so you can keep the current tab value stable.Apply this diff:
- <KTabs - value={editorState.timeline.selection ? undefined : state.selectedTab} + <KTabs + value={state.selectedTab}apps/desktop/src/routes/editor/Header.tsx (1)
77-86: Selection-aware gating and tooltips: LGTM; consider a small helper to reduce repetition.Behavior matches PR goals (first click closes selection, second performs action), tooltips correctly switch to “Close selection”. You can reduce duplication by introducing a
clearSelectionOrhelper.Example helper (inside Header):
const clearSelectionOr = async (fn: () => void | Promise<void>) => { if (editorState.timeline.selection) { setEditorState("timeline", "selection", null); return; } await fn(); };Example usage:
- onClick={async () => { - if (editorState.timeline.selection) { - setEditorState("timeline", "selection", null); - return; - } - if (!(await ask("Are you sure you want to delete this recording?"))) return; - await commands.editorDeleteProject(); - }} + onClick={async () => + clearSelectionOr(async () => { + if (!(await ask("Are you sure you want to delete this recording?"))) return; + await commands.editorDeleteProject(); + }) + }Also applies to: 87-91, 96-104, 104-109, 118-129, 131-142, 160-167, 167-173, 176-183, 183-189, 199-208
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/routes/editor/ConfigSidebar.tsx(4 hunks)apps/desktop/src/routes/editor/Header.tsx(5 hunks)apps/desktop/src/routes/editor/ShareButton.tsx(2 hunks)apps/desktop/src/utils/tauri.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file: tauri.ts
Do not edit auto-generated file: tauri.ts
Files:
apps/desktop/src/utils/tauri.ts
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/utils/tauri.tsapps/desktop/src/routes/editor/ShareButton.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/Header.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/utils/tauri.tsapps/desktop/src/routes/editor/ShareButton.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/Header.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/desktop/src/utils/tauri.tsapps/desktop/src/routes/editor/ShareButton.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/Header.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/desktop/src/routes/editor/ShareButton.tsxapps/desktop/src/routes/editor/ConfigSidebar.tsxapps/desktop/src/routes/editor/Header.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-08T16:48:20.727Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: AGENTS.md:0-0
Timestamp: 2025-09-08T16:48:20.727Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated file: tauri.ts
Applied to files:
apps/desktop/src/utils/tauri.ts
🧬 Code graph analysis (1)
apps/desktop/src/routes/editor/Header.tsx (2)
apps/desktop/src/utils/tauri.ts (1)
commands(7-269)apps/desktop/src/routes/editor/ui.tsx (1)
EditorButton(353-418)
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (7)
apps/desktop/src/routes/editor/ShareButton.tsx (1)
23-24: Context update to include editorState/setEditorState: LGTM.apps/desktop/src/routes/editor/ConfigSidebar.tsx (4)
249-249: z-index bump for tab list: LGTM.
277-283: Conditional tab text styling based on selection: LGTM.
309-313: Hide indicator during selection: LGTM.
597-598: Selection panel repositioning under header (top-16) and layering: LGTM.apps/desktop/src/routes/editor/Header.tsx (1)
56-57: Context update to include editorState/setEditorState: LGTM.apps/desktop/src/utils/tauri.ts (1)
266-268: Confirm generator origin; call sites OK.
- Call sites: only apps/desktop/src/routes/editor/Header.tsx:85 calls
await commands.editorDeleteProject();— the result is awaited and not used, so callers don’t rely on a non‑null return.- apps/desktop/src/utils/tauri.ts (lines 266–268) contains the binding but does not include a
generated by [tauri-specta]banner; confirm this change was produced by tauri‑specta and not a manual edit.
UI improvements and interaction changes to the editor sidebar, header, and share button in the desktop app. The main focus is on handling timeline selections more intuitively, so the header is still in view.
Demo:
https://cap.link/vraxsh0ce9a4ye2
Summary by CodeRabbit