Skip to content

refactor: edit view page#28

Merged
AbianS merged 15 commits into
mainfrom
refactor/edit-view-page
Nov 10, 2025
Merged

refactor: edit view page#28
AbianS merged 15 commits into
mainfrom
refactor/edit-view-page

Conversation

@AbianS

@AbianS AbianS commented Nov 7, 2025

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Tabbed Container Editor: Dashboard, Configuration, Logs, and Terminal with per-tab availability and tooltips.
    • Interactive Terminal: run commands with history, responsive sizing, and colored output.
    • Live Logs: polling, auto-scroll/follow toggle, clear, and loading/no-logs states.
  • Improvements

    • Container header actions: start, stop, delete with confirmations and feedback.
    • Unsaved changes warning, reset behavior, refined save/load/close flows.
  • UI/Style

    • Transparent title bar, polished badges/cards, and new tooltip UI.
  • Chores

    • Enabled terminal and tooltip runtime support.

- Add docker exec command support in Rust backend
- Create use-container-terminal hook with command history
- Implement interactive terminal UI with xterm.js
- Add FitAddon for responsive terminal sizing
- Support Enter, Backspace, Ctrl+C, Ctrl+L shortcuts
- Display stdout/stderr with color coding
- Match LogsTab theme with transparent background
- Handle empty command outputs gracefully

Fixes:
- Remove isExecuting check that blocked input
- Fix background color to match theme
- Handle commands with no output
Backend changes:
- Add TERM=xterm environment variable to docker exec
- Enables clear command and proper terminal features
- Fixes column formatting for ls and other commands

Frontend changes:
- Reduce font size from 13px to 12px (more compact)
- Increase columns from 80 to 100 (better space usage)
- Increase rows from 20 to 24 (more visible content)
- Add proper bottom padding to match horizontal margins
- Remove 'Use Ctrl+L' tip (clear command works now)
- Add scrollToBottom() after command execution

Fixes:
- clear command now works (no more TERM error)
- ls output shows in columns instead of one per line
- Terminal doesn't cut off at bottom
- Better visual consistency with LogsTab
@coderabbitai

coderabbitai Bot commented Nov 7, 2025

Copy link
Copy Markdown

Warning

Rate limit exceeded

@AbianS has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 56 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 51edd80 and 5d4044a.

📒 Files selected for processing (2)
  • src-tauri/src/commands/database.rs (2 hunks)
  • src/pages/edit-container/EditContainerPage.tsx (2 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds Tauri commands and DockerService methods for container logs and remote command execution; integrates log polling and an xterm-based terminal into a refactored tabbed Edit Container page; introduces hooks, UI components (including a Radix tooltip), CSS and dependency additions.

Changes

Cohort / File(s) Summary
Manifest & styling
package.json, src/App.css
Adds runtime deps @radix-ui/react-tooltip, @xterm/addon-fit, @xterm/xterm; sets .titlebar background to transparent.
Tooltip UI
src/shared/components/ui/tooltip.tsx
New Radix-based Tooltip primitives: Tooltip, TooltipTrigger, TooltipContent, TooltipProvider.
XTerm import
src/edit-container.tsx
Imports xterm stylesheet: @xterm/xterm/css/xterm.css.
Tauri commands & service
src-tauri/src/commands/docker.rs, src-tauri/src/lib.rs, src-tauri/src/services/docker.rs
Adds get_container_logs and execute_container_command Tauri commands and corresponding DockerService methods; run docker logs and docker exec (TTY, TERM, COLUMNS) and return string/JSON outputs.
Edit Container page (refactor)
src/pages/edit-container/EditContainerPage.tsx
Replaces single form with tabbed UI (Dashboard, Configuration, Logs, Terminal); adds lifecycle actions (start/stop/delete), loading/not-found states, unsaved-changes handling, and per-tab composition.
Form refactor
src/pages/edit-container/components/EditContainerForm.tsx
EditContainerForm converted to a pure form component accepting container, provider, and form props; reorganized into accordion sections; removed internal save/cancel flows.
Configuration UI & unsaved dialog
src/pages/edit-container/components/ConfigurationTab.tsx, src/pages/edit-container/components/ConfigurationView.tsx
Adds ConfigurationTab with save/reset and useUnsavedChangesWarning/UnsavedChangesDialog; adds ConfigurationView for read-only masked config display.
Dashboard & header
src/pages/edit-container/components/ContainerDashboard.tsx, src/pages/edit-container/components/ContainerHeader.tsx
Adds ContainerDashboard (quick info, connection, settings) and ContainerHeader (status badge and start/stop/delete action buttons with tooltips).
Logs tab & hook
src/pages/edit-container/components/LogsTab.tsx, src/pages/edit-container/hooks/use-container-logs.ts
Adds LogsTab UI and useContainerLogs hook: polling via get_container_logs, auto-scroll/follow behavior, clear logs, load/error states, line-capping and seen-line tracking.
Terminal tab & hook
src/pages/edit-container/components/TerminalTab.tsx, src/pages/edit-container/hooks/use-container-terminal.ts
Adds TerminalTab (xterm + FitAddon, input handling, prompt, resize, output rendering) and useContainerTerminal hook to invoke execute_container_command, expose history and execution state.
Wizard hook changes
src/pages/edit-container/hooks/use-database-edit-wizard.ts
useDatabaseEditWizard now exposes closeWindow and refetch, makes cancel synchronous, and resets form after save instead of closing the window.
Types & utilities
src/pages/edit-container/types/tabs.ts, src/pages/edit-container/utils/container-actions.ts
Adds ContainerTab enum and TAB_LABELS; adds ActionConfig/TabConfig and isActionAvailable/isTabAvailable helpers for availability logic.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant UI as LogsTab
  participant Hook as useContainerLogs
  participant Tauri as Tauri invoke
  participant Cmd as get_container_logs
  participant Svc as DockerService
  participant Shell as docker logs

  User->>UI: Open Logs tab
  UI->>Hook: startPolling(containerId, enabled)
  Hook->>Tauri: invoke("get_container_logs", {containerId, tailLines:500})
  Tauri->>Cmd: dispatch
  Cmd->>Svc: get_container_logs(app, id, tail)
  Svc->>Shell: docker logs --timestamps --tail N <id>
  Shell-->>Svc: log text
  Svc-->>Cmd: String
  Cmd-->>Tauri: String
  Tauri-->>Hook: String
  Hook-->>UI: setLogs(filtered, bounded)
  Note over Hook,UI: Repeat every 3000ms while enabled
Loading
sequenceDiagram
  autonumber
  actor User
  participant UI as TerminalTab (xterm)
  participant Hook as useContainerTerminal
  participant Tauri as Tauri invoke
  participant Cmd as execute_container_command
  participant Svc as DockerService
  participant Shell as docker exec

  User->>UI: Type command + Enter
  UI->>Hook: executeCommand(cmd, columns)
  Hook->>Tauri: invoke("execute_container_command", {containerId, command, columns})
  Tauri->>Cmd: dispatch
  Cmd->>Svc: execute_container_command(app, id, cmd, cols)
  Svc->>Shell: docker exec -it -e TERM=xterm-256color -e COLUMNS=<cols> <id> sh -lc "<cmd>"
  Shell-->>Svc: {stdout, stderr, exitCode}
  Svc-->>Cmd: JSON {stdout, stderr, exitCode}
  Cmd-->>Tauri: JSON
  Tauri-->>Hook: JSON
  Hook-->>UI: render outputs, update prompt
  UI-->>User: Display result with status
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

  • Pay special attention to shell invocation and escaping in src-tauri/src/services/docker.rs.
  • Inspect polling lifecycle, memory bounds, seen-line tracking, and clear-timestamp handling in src/pages/edit-container/hooks/use-container-logs.ts.
  • Review TerminalTab key handling, fit/resize logic, columns propagation, and xterm resource cleanup in src/pages/edit-container/components/TerminalTab.tsx.
  • Verify EditContainerPage tab transitions, UnsavedChangesDialog integration, and that EditContainerForm prop signature changes are reconciled across callers.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.23% 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 'refactor: edit view page' directly aligns with the primary changes, which involve a comprehensive rework of the EditContainerPage from a simple form into a tabbed, data-driven editor with new components and lifecycle controls.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (12)
package.json (1)

49-57: Pin new dependencies for reproducible builds.

Most deps are exact; these use caret ranges. Pin them to the resolved versions to avoid surprise upgrades during CI/release.

-    "@radix-ui/react-tooltip": "^1.2.8",
+    "@radix-ui/react-tooltip": "1.2.8",
-    "@xterm/addon-fit": "^0.10.0",
+    "@xterm/addon-fit": "0.10.0",
-    "@xterm/xterm": "^5.5.0",
+    "@xterm/xterm": "5.5.0",
src/pages/edit-container/types/tabs.ts (1)

8-13: Prepare for i18n/localization.

Hard‑coded English labels. If you plan translations, export label keys or use a t() call here instead of literals.

src/pages/edit-container/hooks/use-container-logs.ts (4)

32-33: Type the interval ref compatibly for browser and Node.

Use ReturnType to avoid DOM vs Node type mismatch.

-  const intervalRef = useRef<NodeJS.Timeout | null>(null);
+  const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null);

78-79: Use forEach instead of map for side effects.

map’s return value is unused; forEach clarifies intent.

-      logLines.map((line) => lastSeenLogsRef.current.add(line));
+      logLines.forEach((line) => lastSeenLogsRef.current.add(line));

142-153: Include startPolling/stopPolling in effect deps to satisfy hooks lint.

Functions are memoized; adding them avoids stale closures and silences lint.

-  }, [containerId, enabled]);
+  }, [containerId, enabled, startPolling, stopPolling]);

56-71: Alternative to client‑side dedup: fetch incrementally with --since.

If you add an optional since (RFC3339) to the Tauri command and service, you can avoid tracking large “seen” sets and reduce payload.

Happy to draft the cross‑file change (command arg, service: add "--since", hook: track last timestamp).

src/pages/edit-container/utils/container-actions.ts (2)

19-37: Clarify “stopped” vs “not running”.

requiresStopped currently returns true for 'error', 'creating', 'removing'. If you only want the 'stopped' state, check explicitly.

-  const isRunning = containerStatus === 'running';
+  const isRunning = containerStatus === 'running';
+  const isStopped = containerStatus === 'stopped';
@@
-  if (action.requiresStopped && isRunning) {
+  if (action.requiresStopped && !isStopped) {
     return false;
   }

Also consider guarding against both requiresRunning and requiresStopped being true (invalid config).


39-57: Apply the same “stopped” semantics to tabs.

Mirror the explicit 'stopped' check to avoid showing tabs during transitional/error states.

-  const isRunning = containerStatus === 'running';
+  const isRunning = containerStatus === 'running';
+  const isStopped = containerStatus === 'stopped';
@@
-  if (tab.requiresStopped && isRunning) {
+  if (tab.requiresStopped && !isStopped) {
     return false;
   }
src/App.css (1)

200-200: Transparent titlebar OK; verify drag region styling.

If this area is meant to be draggable in Tauri, ensure the element (or child) has -webkit-app-region: drag and interactive controls inside use no-drag.

src-tauri/src/commands/docker.rs (1)

42-66: Commands look consistent; consider incremental logs and basic input validation.

  • Optional: add a since (RFC3339 or seconds) arg to get_container_logs and plumb it to the service for lighter polling.
  • Sanity‑check container_id length/charset before invoking docker (defense‑in‑depth).

Would you like a small PR follow‑up to add the --since path end‑to‑end?

src/edit-container.tsx (1)

5-5: XTerm CSS import verified as single and correct.

No duplicate imports found. The import at src/edit-container.tsx:5 is the only occurrence. Consider moving the CSS import into the Terminal tab module if that component is route or tab-scoped, to avoid loading globally on non-terminal views.

src/shared/components/ui/tooltip.tsx (1)

23-25: Let Tooltip respect outer providers.

Wrapping every <TooltipPrimitive.Root> in a fresh provider forces delayDuration back to 0 and defeats any app-level configuration with TooltipProvider. Please drop the per-instance provider (or plumb provider props through) so global settings actually stick.

-function Tooltip({
-  ...props
-}: React.ComponentProps<typeof TooltipPrimitive.Root>) {
-  return (
-    <TooltipProvider>
-      <TooltipPrimitive.Root data-slot="tooltip" {...props} />
-    </TooltipProvider>
-  );
-}
+function Tooltip({
+  ...props
+}: React.ComponentProps<typeof TooltipPrimitive.Root>) {
+  return <TooltipPrimitive.Root data-slot="tooltip" {...props} />;
+}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cad7a75 and 3a4d818.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (20)
  • package.json (1 hunks)
  • src-tauri/src/commands/docker.rs (1 hunks)
  • src-tauri/src/lib.rs (1 hunks)
  • src-tauri/src/services/docker.rs (1 hunks)
  • src/App.css (1 hunks)
  • src/edit-container.tsx (1 hunks)
  • src/pages/edit-container/EditContainerPage.tsx (2 hunks)
  • src/pages/edit-container/components/ConfigurationTab.tsx (1 hunks)
  • src/pages/edit-container/components/ConfigurationView.tsx (1 hunks)
  • src/pages/edit-container/components/ContainerDashboard.tsx (1 hunks)
  • src/pages/edit-container/components/ContainerHeader.tsx (1 hunks)
  • src/pages/edit-container/components/EditContainerForm.tsx (2 hunks)
  • src/pages/edit-container/components/LogsTab.tsx (1 hunks)
  • src/pages/edit-container/components/TerminalTab.tsx (1 hunks)
  • src/pages/edit-container/hooks/use-container-logs.ts (1 hunks)
  • src/pages/edit-container/hooks/use-container-terminal.ts (1 hunks)
  • src/pages/edit-container/hooks/use-database-edit-wizard.ts (4 hunks)
  • src/pages/edit-container/types/tabs.ts (1 hunks)
  • src/pages/edit-container/utils/container-actions.ts (1 hunks)
  • src/shared/components/ui/tooltip.tsx (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (15)
src/pages/edit-container/components/TerminalTab.tsx (2)
src/shared/types/container.ts (1)
  • Container (14-29)
src/pages/edit-container/hooks/use-container-terminal.ts (1)
  • useContainerTerminal (24-90)
src/pages/edit-container/components/ContainerHeader.tsx (1)
src/shared/types/container.ts (1)
  • Container (14-29)
src-tauri/src/services/docker.rs (1)
src-tauri/src/commands/docker.rs (2)
  • get_container_logs (43-52)
  • execute_container_command (55-66)
src/shared/components/ui/tooltip.tsx (1)
src/shared/utils/cn.ts (1)
  • cn (4-6)
src/pages/edit-container/components/ConfigurationView.tsx (2)
src/shared/types/container.ts (1)
  • Container (14-29)
src/features/databases/registry/database-registry.ts (1)
  • databaseRegistry (71-71)
src/pages/edit-container/utils/container-actions.ts (1)
src/shared/types/container.ts (1)
  • ContainerStatus (1-6)
src-tauri/src/lib.rs (2)
src-tauri/src/services/docker.rs (2)
  • get_container_logs (537-566)
  • execute_container_command (568-616)
src-tauri/src/commands/docker.rs (2)
  • get_container_logs (43-52)
  • execute_container_command (55-66)
src/pages/edit-container/hooks/use-container-logs.ts (1)
src/core/tauri/invoke.ts (1)
  • invoke (7-18)
src/pages/edit-container/components/ConfigurationTab.tsx (3)
src/shared/types/container.ts (1)
  • Container (14-29)
src/features/databases/registry/database-registry.ts (1)
  • databaseRegistry (71-71)
src/pages/edit-container/components/EditContainerForm.tsx (1)
  • EditContainerForm (29-115)
src/pages/edit-container/hooks/use-container-terminal.ts (1)
src/core/tauri/invoke.ts (1)
  • invoke (7-18)
src-tauri/src/commands/docker.rs (1)
src-tauri/src/services/docker.rs (5)
  • get_container_logs (537-566)
  • new (13-15)
  • execute_container_command (568-616)
  • serde_json (123-123)
  • serde_json (136-136)
src/pages/edit-container/EditContainerPage.tsx (9)
src/pages/edit-container/hooks/use-database-edit-wizard.ts (1)
  • useDatabaseEditWizard (22-255)
src/pages/edit-container/components/ConfigurationTab.tsx (2)
  • useUnsavedChangesWarning (105-162)
  • ConfigurationTab (25-103)
src/core/tauri/invoke.ts (1)
  • invoke (7-18)
src/pages/edit-container/utils/container-actions.ts (2)
  • TabConfig (12-17)
  • isTabAvailable (39-57)
src/pages/edit-container/types/tabs.ts (1)
  • TAB_LABELS (8-13)
src/pages/edit-container/components/ContainerHeader.tsx (1)
  • ContainerHeader (18-119)
src/pages/edit-container/components/ContainerDashboard.tsx (1)
  • ContainerDashboard (15-142)
src/pages/edit-container/components/LogsTab.tsx (1)
  • LogsTab (11-148)
src/pages/edit-container/components/TerminalTab.tsx (1)
  • TerminalTab (11-213)
src/pages/edit-container/components/LogsTab.tsx (2)
src/shared/types/container.ts (1)
  • Container (14-29)
src/pages/edit-container/hooks/use-container-logs.ts (1)
  • useContainerLogs (24-165)
src/pages/edit-container/components/ContainerDashboard.tsx (2)
src/shared/types/container.ts (1)
  • Container (14-29)
src/features/databases/registry/database-registry.ts (1)
  • databaseRegistry (71-71)
src/pages/edit-container/components/EditContainerForm.tsx (6)
src/shared/types/container.ts (1)
  • Container (14-29)
src/features/databases/registry/database-provider.interface.ts (1)
  • DatabaseProvider (14-73)
src/shared/components/ui/form.tsx (4)
  • Form (161-161)
  • FormItem (162-162)
  • FormControl (164-164)
  • FormLabel (163-163)
src/shared/components/ui/accordion.tsx (4)
  • Accordion (63-63)
  • AccordionItem (63-63)
  • AccordionTrigger (63-63)
  • AccordionContent (63-63)
src/features/databases/components/dynamic-form-section.tsx (2)
  • DynamicFormSection (15-32)
  • DynamicFieldGroups (43-65)
src/shared/components/ui/checkbox.tsx (1)
  • Checkbox (29-29)
🪛 GitHub Check: Frontend (Lint + Tests + Build)
src/pages/edit-container/components/TerminalTab.tsx

[failure] 16-16:
'isExecuting' is declared but its value is never read.

⏰ 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). (1)
  • GitHub Check: Backend (Tests + Build)
🔇 Additional comments (6)
src/pages/edit-container/types/tabs.ts (1)

1-6: Enum definition is clear and type‑safe.

No issues.

src-tauri/src/lib.rs (1)

28-30: New commands correctly registered.

Shell plugin is already enabled; command wiring looks good.

Please confirm these are the only new commands expected for this release.

src/pages/edit-container/hooks/use-container-terminal.ts (1)

34-67: Command validation and history cap look great.

Throwing on empty commands keeps the backend happy, and the FIFO trim shields us from unbounded terminal history.

src/pages/edit-container/components/ContainerHeader.tsx (1)

24-98: Status-driven UI reads clearly.

The variant mapping and conditional start/stop buttons keep the header logic tidy and in sync with container state.

src/pages/edit-container/components/LogsTab.tsx (1)

69-144: Auto-scroll UX is well-considered.

I like how manual scrolling pauses follow mode and the explicit Follow button restores it—makes reading historical logs painless.

src/pages/edit-container/components/ContainerDashboard.tsx (1)

18-29: Clipboard flow handles failure gracefully.

Early exit when the provider is missing and the success/error toasts make this copy action robust.

Comment thread src/pages/edit-container/components/EditContainerForm.tsx Outdated
Comment thread src/pages/edit-container/components/TerminalTab.tsx Outdated
Comment thread src/pages/edit-container/EditContainerPage.tsx
Comment thread src/pages/edit-container/hooks/use-container-logs.ts
Comment thread src/pages/edit-container/hooks/use-container-logs.ts
Comment thread src/pages/edit-container/utils/container-actions.ts
@AbianS
AbianS requested a review from Copilot November 10, 2025 17:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/pages/edit-container/components/TerminalTab.tsx (1)

145-197: Consider moving helper functions inside the effect to avoid stale closures.

writePrompt and executeCommandInTerminal are defined in the component body but used within the effect's onData handler. This creates a potential stale closure: if the component re-renders but the effect doesn't re-run, the handler still references the old function closures. While this may work in practice (since the effect re-runs when containerId changes and executeCommand is memoized on that same dependency), it's fragile and ESLint exhaustive-deps may warn.

Move these functions inside the effect or use useCallback with proper dependencies for clearer data flow.

Option 1: Move functions inside the effect

   useEffect(() => {
     if (!terminalRef.current) return;

     // Initialize xterm.js terminal
     const terminal = new Terminal({
       // ... config
     });

     // ... setup code

+    /**
+     * Write command prompt
+     */
+    const writePrompt = (term: Terminal) => {
+      term.write(`\x1b[1;36m$\x1b[0m `);
+    };
+
+    /**
+     * Execute command and display output in terminal
+     */
+    const executeCommandInTerminal = async (
+      term: Terminal,
+      command: string,
+    ) => {
+      try {
+        const columns = term.cols;
+        const result = await executeCommand(command, columns);
+        // ... rest of implementation
+      } catch (error) {
+        // ... error handling
+      }
+    };
+
     // Welcome message
     terminal.writeln(/* ... */);
     writePrompt(terminal);

     // Handle user input
     terminal.onData((data) => {
       // ... existing handler
     });

     // ... rest of effect
-  }, [container.containerId, container.name]);
+  }, [container.containerId, container.name, executeCommand]);

-  const writePrompt = (terminal: Terminal) => {
-    terminal.write(`\x1b[1;36m$\x1b[0m `);
-  };
-
-  const executeCommandInTerminal = async (
-    terminal: Terminal,
-    command: string,
-  ) => {
-    // ... implementation
-  };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a4d818 and c80f2a9.

📒 Files selected for processing (1)
  • src/pages/edit-container/components/TerminalTab.tsx (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/edit-container/components/TerminalTab.tsx (2)
src/shared/types/container.ts (1)
  • Container (14-29)
src/pages/edit-container/hooks/use-container-terminal.ts (1)
  • useContainerTerminal (24-90)
⏰ 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: CodeQL analysis (javascript-typescript)
  • GitHub Check: Agent
  • GitHub Check: Backend (Tests + Build)
🔇 Additional comments (3)
src/pages/edit-container/components/TerminalTab.tsx (3)

1-9: LGTM!

Imports and interface declaration are clean and appropriate.


11-17: LGTM!

Component setup is correct. The previous lint issue with unused isExecuting has been properly resolved.


199-210: LGTM!

Render logic is clean with appropriate loading state handling.

Comment thread src/pages/edit-container/components/TerminalTab.tsx
Comment thread src/pages/edit-container/components/TerminalTab.tsx

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request transforms the edit container page from a simple form into a comprehensive multi-tab interface for managing Docker containers, adding interactive features like real-time logs viewing and terminal access.

Key Changes

  • Introduced a tabbed interface with Dashboard, Configuration, Logs, and Terminal tabs for better organization
  • Added real-time container log viewing with auto-scroll, polling, and clearing functionality
  • Implemented an interactive terminal using xterm.js for executing commands directly in containers
  • Refactored the configuration form into a dedicated tab with unsaved changes warnings

Reviewed Changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/shared/components/ui/tooltip.tsx New tooltip UI component built on Radix UI primitives
src/pages/edit-container/types/tabs.ts Enum and labels for container tab navigation
src/pages/edit-container/utils/container-actions.ts Utility functions to determine tab/action availability based on container status
src/pages/edit-container/hooks/use-database-edit-wizard.ts Updated to separate cancel/closeWindow actions and support form reset after saves
src/pages/edit-container/hooks/use-container-terminal.ts Hook for executing commands in containers with history tracking
src/pages/edit-container/hooks/use-container-logs.ts Hook for fetching and polling container logs with filtering
src/pages/edit-container/components/TerminalTab.tsx Terminal interface using xterm.js with full keyboard support
src/pages/edit-container/components/LogsTab.tsx Real-time log viewer with auto-scroll and clear functionality
src/pages/edit-container/components/ContainerHeader.tsx Header with container info and action buttons (start/stop/delete)
src/pages/edit-container/components/ContainerDashboard.tsx Dashboard tab showing container quick info and connection string
src/pages/edit-container/components/ConfigurationView.tsx Read-only configuration display (appears unused)
src/pages/edit-container/components/ConfigurationTab.tsx Editable configuration tab with save/reset and unsaved changes dialog
src/pages/edit-container/components/EditContainerForm.tsx Refactored into reusable form component without page-level concerns
src/pages/edit-container/EditContainerPage.tsx Complete redesign with tabs, header, and container lifecycle management
src/edit-container.tsx Added xterm CSS import
src/App.css Made titlebar background transparent
src-tauri/src/services/docker.rs Added get_container_logs and execute_container_command methods
src-tauri/src/lib.rs Registered new Tauri commands
src-tauri/src/commands/docker.rs Added command handlers for logs and terminal
package.json Added dependencies: @radix-ui/react-tooltip, @xterm/xterm, @xterm/addon-fit

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/pages/edit-container/EditContainerPage.tsx Outdated
Comment thread src/pages/edit-container/components/ConfigurationTab.tsx
Comment thread src-tauri/src/services/docker.rs
Comment thread src/pages/edit-container/hooks/use-container-logs.ts Outdated
Comment thread src/pages/edit-container/hooks/use-container-logs.ts
Comment thread src/pages/edit-container/hooks/use-container-logs.ts
Comment thread src/pages/edit-container/components/LogsTab.tsx
Comment thread src/pages/edit-container/components/TerminalTab.tsx

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
src/pages/edit-container/components/TerminalTab.tsx (1)

19-140: Add executeCommand to the dependency array.

The useEffect creates event handlers that reference executeCommand (via executeCommandInTerminal at line 87), but executeCommand is not included in the dependency array at line 140. Per React's exhaustive-deps rule, all captured values should be listed.

Apply this diff:

-  }, [container.containerId, container.name]);
+  }, [container.containerId, container.name, executeCommand]);

This prevents potential stale closure issues if executeCommand ever changes between renders.

src/pages/edit-container/hooks/use-container-logs.ts (1)

169-169: Add startPolling and stopPolling to useEffect dependencies.

React's rules of hooks require that all functions called within useEffect be included in the dependency array if their identity can change. While these are useCallback-memoized, their identities do change when their own dependencies (e.g., fetchLogs, isPolling) change. Omitting them risks stale closures or missed re-subscription when the hook's inputs evolve.

-  }, [containerId, enabled]);
+  }, [containerId, enabled, startPolling, stopPolling]);
🧹 Nitpick comments (2)
src/pages/edit-container/utils/container-actions.ts (1)

20-58: Consider extracting shared logic to reduce duplication.

Both isActionAvailable and isTabAvailable implement identical availability logic. Since ActionConfig and TabConfig share the same optional flags (requiresRunning, requiresStopped), you could extract a generic helper:

+function isAvailable(
+  config: { requiresRunning?: boolean; requiresStopped?: boolean },
+  containerStatus: ContainerStatus,
+): boolean {
+  const isRunning = containerStatus === 'running';
+  if (config.requiresRunning && !isRunning) return false;
+  if (config.requiresStopped && isRunning) return false;
+  return true;
+}
+
 export function isActionAvailable(
   action: ActionConfig,
   containerStatus: ContainerStatus,
 ): boolean {
-  const isRunning = containerStatus === 'running';
-
-  // If action requires running container but it's not running
-  if (action.requiresRunning && !isRunning) {
-    return false;
-  }
-
-  // If action requires stopped container but it's running
-  if (action.requiresStopped && isRunning) {
-    return false;
-  }
-
-  // Action is available
-  return true;
+  return isAvailable(action, containerStatus);
 }

 export function isTabAvailable(
   tab: TabConfig,
   containerStatus: ContainerStatus,
 ): boolean {
-  const isRunning = containerStatus === 'running';
-
-  // If tab requires running container but it's not running
-  if (tab.requiresRunning && !isRunning) {
-    return false;
-  }
-
-  // If tab requires stopped container but it's running
-  if (tab.requiresStopped && isRunning) {
-    return false;
-  }
-
-  // Tab is available
-  return true;
+  return isAvailable(tab, containerStatus);
 }
src/pages/edit-container/hooks/use-container-logs.ts (1)

131-131: Optional: Remove redundant setLoading(true).

fetchLogs already sets loading to true on line 52, so this call is redundant. Consider removing it for clarity.

     setIsPolling(true);
-    setLoading(true);
 
     // Fetch immediately
     fetchLogs();
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c80f2a9 and 51edd80.

📒 Files selected for processing (5)
  • src/pages/edit-container/EditContainerPage.tsx (2 hunks)
  • src/pages/edit-container/components/EditContainerForm.tsx (2 hunks)
  • src/pages/edit-container/components/TerminalTab.tsx (1 hunks)
  • src/pages/edit-container/hooks/use-container-logs.ts (1 hunks)
  • src/pages/edit-container/utils/container-actions.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/pages/edit-container/components/TerminalTab.tsx (2)
src/shared/types/container.ts (1)
  • Container (14-29)
src/pages/edit-container/hooks/use-container-terminal.ts (1)
  • useContainerTerminal (24-90)
src/pages/edit-container/EditContainerPage.tsx (6)
src/pages/edit-container/hooks/use-database-edit-wizard.ts (1)
  • useDatabaseEditWizard (22-255)
src/pages/edit-container/components/ConfigurationTab.tsx (2)
  • useUnsavedChangesWarning (105-162)
  • ConfigurationTab (25-103)
src/core/tauri/invoke.ts (1)
  • invoke (7-18)
src/pages/edit-container/utils/container-actions.ts (2)
  • TabConfig (13-18)
  • isTabAvailable (40-58)
src/pages/edit-container/components/ContainerHeader.tsx (1)
  • ContainerHeader (18-119)
src/shared/components/DeleteConfirmationDialog.tsx (1)
  • DeleteConfirmationDialog (20-70)
src/pages/edit-container/components/EditContainerForm.tsx (3)
src/shared/types/container.ts (1)
  • Container (14-29)
src/features/databases/registry/database-provider.interface.ts (1)
  • DatabaseProvider (14-73)
src/features/databases/components/dynamic-form-section.tsx (2)
  • DynamicFormSection (15-32)
  • DynamicFieldGroups (43-65)
src/pages/edit-container/hooks/use-container-logs.ts (1)
src/core/tauri/invoke.ts (1)
  • invoke (7-18)
src/pages/edit-container/utils/container-actions.ts (1)
src/shared/types/container.ts (1)
  • ContainerStatus (1-6)
🔇 Additional comments (6)
src/pages/edit-container/components/EditContainerForm.tsx (1)

34-38: Past review concern has been resolved.

The container context is now properly threaded through fieldOptions to all provider field builders (getBasicFields, getAuthenticationFields, getAdvancedFields), and advancedFields is computed once and reused. This eliminates the duplicate computation and ensures providers receive the correct edit-mode context.

src/pages/edit-container/utils/container-actions.ts (1)

1-1: Past review concern has been resolved.

The ComponentType type is now properly imported from 'react', fixing the previous TypeScript error in this .ts file.

src/pages/edit-container/EditContainerPage.tsx (1)

155-167: Past review concern has been resolved.

The unsaved changes flow now correctly calls cancel() (line 162) instead of form.reset(), which properly restores the form to the last saved container state rather than empty defaults.

src/pages/edit-container/components/TerminalTab.tsx (1)

16-16: Past review concerns have been resolved.

The unused isExecuting variable has been removed (line 16), and both welcome messages now properly handle undefined containerId with ?? 'N/A' fallbacks (lines 69, 112).

Also applies to: 67-71, 108-113

src/pages/edit-container/hooks/use-container-logs.ts (2)

30-44: Good fixes: loading state and LRU tracking address prior concerns.

The initialization of loading to false (line 32) resolves the stuck-spinner issue from previous reviews. The dual-ref LRU pattern (lastSeenLogsRef Set + seenQueueRef array) on lines 41-43 properly bounds memory growth up to MAX_SEEN_LOG_LINES.


88-97: LRU capping correctly implemented.

The forEach loop (addressing a prior review note) and FIFO queue pruning ensure lastSeenLogsRef never exceeds MAX_SEEN_LOG_LINES. This prevents unbounded memory growth.

Comment thread src/pages/edit-container/hooks/use-container-logs.ts
Comment thread src/pages/edit-container/hooks/use-container-logs.ts
@AbianS
AbianS merged commit ac33c01 into main Nov 10, 2025
3 checks passed
@AbianS
AbianS deleted the refactor/edit-view-page branch November 10, 2025 17:43
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