Skip to content

Conversation

@richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Sep 18, 2025

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

  • New Features
    • Selection-aware editor UI: tabs and header actions now respond to active timeline selections, with contextual tooltips (e.g., “Close selection”).
    • Clicking tabs or key actions clears an active selection before proceeding.
  • Bug Fixes
    • Prevent unintended actions (Undo/Redo, Export, Share) when a selection is active by gating and clearing selection first.
    • Improved tab visuals: hide selection indicator during active selection and refine trigger styling.
    • Corrected overlay layering and layout under the editor header (z-index and spacing adjustments).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 18, 2025

Walkthrough

Implements 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

Cohort / File(s) Change summary
Selection-aware ConfigSidebar UI
apps/desktop/src/routes/editor/ConfigSidebar.tsx
Tabs reflect timeline.selection: no active tab highlight, hidden indicator, conditional trigger styles, higher z-index. Tab clicks clear selection before switching and scroll to top. Selection overlay/container positioning adjusted (top inset changes).
Header action gating via editorState
apps/desktop/src/routes/editor/Header.tsx
Consumes editorState/setEditorState from context. For Delete/Open/Captions/Performance/Undo/Redo/Export: if selection exists, clear it and return; tooltips show “Close selection”. Undo/Redo disabled logic updated. Context now provides editorState/setEditorState to consumers.
ShareButton upload gating
apps/desktop/src/routes/editor/ShareButton.tsx
Uses editorState/setEditorState. Reupload onClick clears selection and returns if active; otherwise proceeds with upload. Upload flow otherwise unchanged.
Tauri delete return type
apps/desktop/src/utils/tauri.ts
editorDeleteProject now returns Promise (returns TAURI_INVOKE result) instead of Promise.

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
Loading
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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • Brendonovich

Poem

A nibble of clicks, a hop through state,
I tidy selections before they propagate.
Tabs un-highlight, tooltips chime,
“Close that loop, then act in time.”
With nulls returned and flows anew—
Thump-thump! says Rabbit: shipped for you. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately and concisely summarizes the primary change: making editor UI actions handle timeline selection state; this directly matches the described edits to ConfigSidebar, Header, and ShareButton and focuses on behavior rather than implementation details. It is short, specific, and helpful for a teammate scanning PR history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch editor-navigation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 and undefined.

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 clearSelectionOr helper.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 428bb0e and ab4b404.

📒 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.ts
  • apps/desktop/src/routes/editor/ShareButton.tsx
  • apps/desktop/src/routes/editor/ConfigSidebar.tsx
  • apps/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.ts
  • apps/desktop/src/routes/editor/ShareButton.tsx
  • apps/desktop/src/routes/editor/ConfigSidebar.tsx
  • apps/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.ts
  • apps/desktop/src/routes/editor/ShareButton.tsx
  • apps/desktop/src/routes/editor/ConfigSidebar.tsx
  • apps/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.tsx
  • apps/desktop/src/routes/editor/ConfigSidebar.tsx
  • apps/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.

@richiemcilroy richiemcilroy merged commit 37e680d into main Sep 18, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants