fix(docker): improve logs and console UX when container is stopped #1868
Conversation
WalkthroughIntroduces an Changes
Sequence Diagram(s)sequenceDiagram
participant DCM as DockerContainerManagement
participant DCV as DockerConsoleViewer
participant SDLV as SingleDockerLogViewer
participant BLV as BaseLogViewer
Note over DCM: Container state updates / user opens container UI
DCM->>DCM: compute isContainerRunning & consoleBadge
DCM->>DCV: pass is-running
DCM->>SDLV: pass is-running
alt isRunning = true
DCV->>DCV: initialize terminal & connect session
DCV->>DCV: show interactive console
SDLV->>BLV: render logs (dimmed=false)
else isRunning = false
DCV->>DCV: skip init & reset session
DCV->>DCV: show "container not running" placeholder
SDLV->>BLV: render logs with dimmed=true and show historical message
BLV->>BLV: apply opacity-50 styling
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (5)web/**/*📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-07-24T18:48:44.035ZApplied to files:
📚 Learning: 2025-07-24T18:48:43.590ZApplied to files:
🔇 Additional comments (9)
Comment |
pujitm
force-pushed
the
fix/logs-console-indicators
branch
from
1c3b830 to
c2b3c61
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c3b8303f5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (running) { | ||
| initTerminal(); | ||
| } else { | ||
| hideSession(props.containerName); |
There was a problem hiding this comment.
When a container stops, this watcher only hides the existing session. If the user then starts the container again, initTerminal() will see the cached session and skip createSession, so the iframe keeps pointing at a terminated ttyd connection and the console won’t reconnect until the user manually refreshes. This shows up when a container is stopped and then started while its console tab is open. Consider destroying the session (or forcing a new one) when isRunning becomes false, or bypassing the existing-session fast path on restart.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1868 +/- ##
==========================================
- Coverage 46.40% 46.39% -0.02%
==========================================
Files 954 954
Lines 59791 59823 +32
Branches 5538 5542 +4
==========================================
+ Hits 27749 27753 +4
- Misses 31923 31951 +28
Partials 119 119 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/Docker/DockerConsoleViewer.vue (1)
160-173: Disable buttons when container is not running.The maximize and reconnect buttons remain interactive when
isRunningis false, which creates confusing UX—clicking them appears to do nothing or attempts operations that won't succeed on a stopped container.🔧 Suggested fix
<UButton size="xs" variant="ghost" icon="i-lucide-maximize-2" - :disabled="isConnecting || hasError || isPoppedOut" + :disabled="!isRunning || isConnecting || hasError || isPoppedOut" @click="openFullscreen" /> <UButton size="xs" variant="ghost" icon="i-lucide-refresh-cw" :loading="isConnecting" + :disabled="!isRunning" @click="reconnect" />
🧹 Nitpick comments (5)
web/src/components/Logs/BaseLogViewer.vue (1)
171-179: Avoid redundanttheme-*class binding unless it’s required for scoping.
theme-dark/theme-lightis already applied on the scroll container (Line 154-160), so duplicating it on the<pre>may be unnecessary. If not required, keep this binding focused onopacity-50to reduce churn and future confusion.Proposed diff
<pre class="hljs m-0 p-4 font-mono text-xs leading-6 whitespace-pre" - :class="{ - 'theme-dark': isDarkMode, - 'theme-light': !isDarkMode, - 'opacity-50': dimmed, - }" + :class="{ 'opacity-50': dimmed }" v-html="highlightedContent" />web/src/composables/useDockerTableColumns.ts (1)
105-125: HoistcolorMap/labelMapout of the cell render; confirm EXITED=errorsemantics.This currently allocates new objects for every cell render. Also, mapping “Stopped” to an
errorbadge may read as “failed” rather than “not running” (unless that’s the intent).Proposed diff
+const STATE_BADGE_COLOR_MAP: Partial<Record<ContainerState, 'success' | 'warning' | 'error'>> = { + [ContainerState.RUNNING]: 'success', + [ContainerState.PAUSED]: 'warning', + [ContainerState.EXITED]: 'error', +}; + +const STATE_BADGE_LABEL_MAP: Partial<Record<ContainerState, string>> = { + [ContainerState.RUNNING]: 'Running', + [ContainerState.PAUSED]: 'Paused', + [ContainerState.EXITED]: 'Stopped', +}; + export function useDockerTableColumns(options: DockerTableColumnsOptions) { const { compact, busyRowIds, updatingRowIds, containerStats, onUpdateContainer, getRowActionItems } = options; @@ cell: ({ row }) => { if (row.original.type === 'folder') return ''; - const state = row.original.state ?? ''; + const state = row.original.state ?? ''; const isBusy = busyRowIds.value.has(row.original.id); - const colorMap: Record<string, 'success' | 'warning' | 'error' | 'neutral'> = { - [ContainerState.RUNNING]: 'success', - [ContainerState.PAUSED]: 'warning', - [ContainerState.EXITED]: 'error', - }; - const labelMap: Record<string, string> = { - [ContainerState.RUNNING]: 'Running', - [ContainerState.PAUSED]: 'Paused', - [ContainerState.EXITED]: 'Stopped', - }; - const color = colorMap[state] || 'neutral'; - const label = labelMap[state] || state; + const color = STATE_BADGE_COLOR_MAP[state as ContainerState] || 'neutral'; + const label = STATE_BADGE_LABEL_MAP[state as ContainerState] || state; if (isBusy) { return h(USkeleton, { class: 'h-5 w-20' }); } return h(UBadge, { color }, () => label); }, },If you want, I can propose a cast-free variant once we confirm the generated type of
DockerContainer['state'](whether it’sContainerState | nullvsstring | null).web/src/components/Docker/ContainerOverviewCard.vue (1)
40-45: State label normalization (EXITED → “Stopped”) matches the intended UX.Optional: consider centralizing the state→label mapping so the table badge + overview badge can’t drift over time.
web/src/components/Docker/SingleDockerLogViewer.vue (1)
161-168: Consider pausing log polling when!isRunning(still show historical logs, just don’t refetch).Right now, a stopped container with
autoScroll=truewill keep refetching every 2s. If the intent is “historical logs”, pausing polling untilisRunningflips back to true is likely a better tradeoff.Also, since this touches GraphQL query usage under
web/**, ensurepnpm codegenhas been run (and generated outputs committed if applicable).Also applies to: 198-209
web/src/components/Docker/DockerConsoleViewer.vue (1)
107-116: Watcher logic correctly handles runtime state transitions.The watcher appropriately initializes the terminal when the container starts and hides the session when it stops.
♻️ Optional: Reset internal state when container stops for consistency
For cleanliness, you could reset internal state when
runningbecomesfalse:watch( () => props.isRunning, (running) => { if (running) { initTerminal(); } else { + isConnecting.value = false; + hasError.value = false; + isPoppedOut.value = false; hideSession(props.containerName); } } );Note: This is purely for consistency—the template logic already handles this correctly since
v-if="!isRunning"takes precedence.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/src/components/Docker/ContainerOverviewCard.vueweb/src/components/Docker/DockerConsoleViewer.vueweb/src/components/Docker/DockerContainerManagement.vueweb/src/components/Docker/SingleDockerLogViewer.vueweb/src/components/Logs/BaseLogViewer.vueweb/src/composables/useDockerTableColumns.ts
🧰 Additional context used
📓 Path-based instructions (5)
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/components/Docker/SingleDockerLogViewer.vueweb/src/components/Docker/DockerConsoleViewer.vueweb/src/components/Logs/BaseLogViewer.vueweb/src/composables/useDockerTableColumns.tsweb/src/components/Docker/DockerContainerManagement.vueweb/src/components/Docker/ContainerOverviewCard.vue
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/src/components/Docker/SingleDockerLogViewer.vueweb/src/components/Docker/DockerConsoleViewer.vueweb/src/components/Logs/BaseLogViewer.vueweb/src/composables/useDockerTableColumns.tsweb/src/components/Docker/DockerContainerManagement.vueweb/src/components/Docker/ContainerOverviewCard.vue
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use TypeScript imports with.jsextensions for ESM compatibility
Never add comments unless they are needed for clarity of function
Never add comments for obvious things, and avoid commenting when starting and ending code blocks
Files:
web/src/composables/useDockerTableColumns.ts
web/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Files:
web/src/composables/useDockerTableColumns.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use theanytype. Always prefer proper typing
Avoid using casting whenever possible, prefer proper typing from the start
Files:
web/src/composables/useDockerTableColumns.ts
🧠 Learnings (2)
📚 Learning: 2025-07-24T18:48:43.590Z
Learnt from: mdatelle
Repo: unraid/api PR: 1533
File: web/components/Docker/Console.vue:22-29
Timestamp: 2025-07-24T18:48:43.590Z
Learning: The Docker Console component in web/components/Docker/Console.vue is temporary/dummy code with placeholder terminal responses, used for building out the UI layout while developing the real components in PR #1533.
Applied to files:
web/src/components/Docker/SingleDockerLogViewer.vueweb/src/components/Docker/DockerConsoleViewer.vueweb/src/components/Docker/DockerContainerManagement.vue
📚 Learning: 2025-07-24T18:48:44.035Z
Learnt from: mdatelle
Repo: unraid/api PR: 1533
File: web/components/Docker/Edit.vue:16-32
Timestamp: 2025-07-24T18:48:44.035Z
Learning: In web/components/Docker/Edit.vue, the hardcoded configuration values in the config ref are intentional temporary/dummy data used during initial UI development phase while building out the real Docker components, as clarified by mdatelle in PR #1533.
Applied to files:
web/src/components/Docker/SingleDockerLogViewer.vueweb/src/components/Docker/DockerConsoleViewer.vueweb/src/components/Docker/DockerContainerManagement.vue
🔇 Additional comments (7)
web/src/components/Logs/BaseLogViewer.vue (1)
16-47: Public propdimmedlooks good; consider naming/usage consistency across viewers.web/src/components/Docker/SingleDockerLogViewer.vue (1)
14-25:isRunningdefaulting totrueis a nice backward-compatible API change.web/src/components/Docker/DockerContainerManagement.vue (1)
604-625:is-runningprop wiring into Logs/Console viewers matches the new stopped-state UX.Also applies to: 680-686
web/src/components/Docker/DockerConsoleViewer.vue (4)
9-9: LGTM!The
isRunningprop is properly typed and the default value oftruemaintains backward compatibility for existing usages.Also applies to: 14-14
31-33: LGTM!Correctly gates placeholder visibility behind
isRunningstate, preventing terminal UI from appearing for stopped containers.
43-48: LGTM!The early return guard properly prevents terminal initialization when the container is not running, and resets internal state for cleanliness.
177-184: LGTM!The conditional rendering logic is well-structured and the "Container is not running" message provides clear feedback. The UI styling is consistent with other state indicators.
| const isContainerRunning = computed(() => activeContainer.value?.state === 'RUNNING'); | ||
| const consoleBadge = computed(() => { | ||
| if (isContainerRunning.value && hasActiveConsoleSession.value) { | ||
| return { color: 'success' as const, variant: 'solid' as const, class: 'w-2 h-2 p-0 min-w-0' }; | ||
| } | ||
| return undefined; | ||
| }); | ||
| const legacyPaneTabs = computed(() => [ | ||
| { label: 'Overview', value: 'overview' as const }, | ||
| { label: 'Settings', value: 'settings' as const }, | ||
| { label: 'Logs', value: 'logs' as const }, | ||
| { | ||
| label: 'Console', | ||
| value: 'console' as const, | ||
| badge: hasActiveConsoleSession.value | ||
| ? { color: 'success' as const, variant: 'solid' as const, class: 'w-2 h-2 p-0 min-w-0' } | ||
| : undefined, | ||
| badge: consoleBadge.value, | ||
| }, | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the actual file to see the full context
head -n 420 web/src/components/Docker/DockerContainerManagement.vue | tail -n 50Repository: unraid/api
Length of output: 1523
🏁 Script executed:
# Search for UTabs usage with badge in the codebase
rg -n 'UTabs' web/src --type=vue -A 10 -B 2 | head -100Repository: unraid/api
Length of output: 81
🏁 Script executed:
# Search for badge property usage patterns
rg -n 'badge\s*:' web/src --type=vue -A 2 -B 1Repository: unraid/api
Length of output: 81
🏁 Script executed:
# Check if there's a package.json to see the @nuxt/ui version
cat web/package.json | grep -A 5 -B 5 '@nuxt/ui'Repository: unraid/api
Length of output: 451
🏁 Script executed:
# Search for UTabs in files without specifying vue type (use general file search)
rg -n 'UTabs' web/src -A 10 -B 2 | head -150Repository: unraid/api
Length of output: 2357
🏁 Script executed:
# Search for badge property patterns more broadly
rg -n 'badge' web/src --type=ts -A 2 -B 1 | head -100Repository: unraid/api
Length of output: 36
🏁 Script executed:
# Check the component template to see how UTabs is used with legacyPaneTabs
sed -n '415,500p' web/src/components/Docker/DockerContainerManagement.vueRepository: unraid/api
Length of output: 2945
🌐 Web query:
@nuxt/ui 4.0.0-alpha.0 UTabs badge API documentation
💡 Result:
UTabs (v4.0.0‑alpha.0) — badge API (summary)
- Each item in the UTabs items array may include a badge: badge?: string | number | BadgeProps. [1]. (ui4.nuxt.com)
- You can pass a primitive (string/number) or a full BadgeProps object (so you can control label, color, variant, size, etc.). Use UI item.ui to customize classes (e.g. trailingBadge, trailingBadgeSize). [1][2]. (ui4.nuxt.com)
Example:
- items: [{ label: 'Inbox', badge: 4 }, { label: 'Alerts', badge: { label: 'New', variant: 'subtle', size: 'sm' } }]. [1][2]. (ui4.nuxt.com)
Sources:
- Tabs component (items + badge API). [1]. (ui4.nuxt.com)
- Badge component (BadgeProps: label, color, variant, size, etc.). [2]. (ui4.nuxt.com)
Citations:
- 1: https://ui4.nuxt.com/docs/components/tabs?utm_source=openai
- 2: https://ui4.nuxt.com/docs/components/tabs?utm_source=openai
- 3: https://ui4.nuxt.com/docs/components/tabs?utm_source=openai
- 4: https://ui4.nuxt.com/docs/components/tabs?utm_source=openai
- 5: https://ui4.nuxt.com/docs/components/badge?utm_source=openai
Badge object violates @nuxt/ui BadgeProps API—green dot indicator won't render properly.
The consoleBadge returns { color, variant, class }, but UTabs expects badge?: string | number | BadgeProps where BadgeProps only supports: label, color, variant, size. The class property is not a valid BadgeProps field and will be ignored. Additionally, there's no label, which may cause the badge to not display at all.
Fix: Either provide a label (e.g. label: '●' or label: '') and remove class, or apply Tailwind classes via the tab item's ui prop instead:
{
label: 'Console',
value: 'console' as const,
badge: isContainerRunning.value && hasActiveConsoleSession.value
? { label: '●', color: 'success', size: 'xs' }
: undefined,
ui: { label: isContainerRunning.value && hasActiveConsoleSession.value ? 'text-success-500' : '' }
}
a87bb00 to
1ebba25
Compare
- Change "Exited" status label to "Stopped" for better clarity - Show "Container is not running" message in console tab when stopped - Dim historical logs when container is stopped with info message - Remove redundant status badge from container detail header - Console tab indicator only shows green when running with active session Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
pujitm
force-pushed
the
fix/logs-console-indicators
branch
from
1ebba25 to
a39d586
Compare
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.