fix(docker): improve start/stop UX with visual feedback #1865
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes add busy state tracking to the Docker container management UI by introducing an Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10–15 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 (5)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1865 +/- ##
==========================================
- Coverage 46.49% 46.47% -0.02%
==========================================
Files 954 954
Lines 59788 59813 +25
Branches 5552 5557 +5
==========================================
Hits 27799 27799
- Misses 31870 31895 +25
Partials 119 119 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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: 25d804fb67
ℹ️ 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 (toStart.length) { | ||
| setRowsStarting?.( | ||
| toStart.map((i) => i.id), | ||
| false | ||
| ); |
There was a problem hiding this comment.
The batch path marks rows as “starting” but only clears that state at the end of a successful run. If any stopMutation/startMutation rejects (e.g., a container fails to start), the function throws before hitting this reset, so startingRowIds stays set and the “Xs delay” badge can remain stuck until a full refresh. Wrapping the mutations in a try/finally (or clearing in the caller’s finally) would ensure the UI is restored after failures.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/composables/useContainerActions.ts (1)
81-126: Missing error handling will leave rows stuck in "starting" state.The
setRowsStartingcleanup (lines 120-125) executes only on success. If any mutation in the batch throws an error, the starting state persists indefinitely, leaving the UI in an inconsistent state.🐛 Wrap cleanup in try/finally
async function runStartStopBatch( toStart: { id: string; containerId: string; name: string }[], toStop: { id: string; containerId: string; name: string }[] ) { if (toStart.length) { onWillStartContainers?.(toStart); setRowsStarting?.( toStart.map((i) => i.id), true ); } - const totalOps = toStop.length + toStart.length; - let completed = 0; - for (const item of toStop) { - completed++; - const isLast = completed === totalOps; - await stopMutation( - { id: item.containerId }, - isLast - ? { - refetchQueries: [refetchQuery], - awaitRefetchQueries: true, - } - : { awaitRefetchQueries: false } - ); - } - for (const item of toStart) { - completed++; - const isLast = completed === totalOps; - await startMutation( - { id: item.containerId }, - isLast - ? { - refetchQueries: [refetchQuery], - awaitRefetchQueries: true, - } - : { awaitRefetchQueries: false } - ); - } - if (toStart.length) { - setRowsStarting?.( - toStart.map((i) => i.id), - false - ); + try { + const totalOps = toStop.length + toStart.length; + let completed = 0; + for (const item of toStop) { + completed++; + const isLast = completed === totalOps; + await stopMutation( + { id: item.containerId }, + isLast + ? { + refetchQueries: [refetchQuery], + awaitRefetchQueries: true, + } + : { awaitRefetchQueries: false } + ); + } + for (const item of toStart) { + completed++; + const isLast = completed === totalOps; + await startMutation( + { id: item.containerId }, + isLast + ? { + refetchQueries: [refetchQuery], + awaitRefetchQueries: true, + } + : { awaitRefetchQueries: false } + ); + } + } finally { + if (toStart.length) { + setRowsStarting?.( + toStart.map((i) => i.id), + false + ); + } } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/src/components/Docker/DockerContainersTable.vueweb/src/components/Docker/DockerNameCell.vueweb/src/components/Docker/docker-containers.query.tsweb/src/composables/useContainerActions.tsweb/src/composables/useDockerTableColumns.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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.tsweb/src/composables/useContainerActions.tsweb/src/components/Docker/docker-containers.query.ts
web/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Always run
pnpm codegenfor GraphQL code generation in the web directory
Files:
web/src/composables/useDockerTableColumns.tsweb/src/composables/useContainerActions.tsweb/src/components/Docker/DockerContainersTable.vueweb/src/components/Docker/DockerNameCell.vueweb/src/components/Docker/docker-containers.query.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.tsweb/src/composables/useContainerActions.tsweb/src/components/Docker/docker-containers.query.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.tsweb/src/composables/useContainerActions.tsweb/src/components/Docker/docker-containers.query.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/default.mdc)
Never add comments unless they are needed for clarity of function
Files:
web/src/composables/useDockerTableColumns.tsweb/src/composables/useContainerActions.tsweb/src/components/Docker/DockerContainersTable.vueweb/src/components/Docker/DockerNameCell.vueweb/src/components/Docker/docker-containers.query.ts
web/**/*.query.ts
📄 CodeRabbit inference engine (CLAUDE.md)
GraphQL queries must be placed in
.query.tsfiles
Files:
web/src/components/Docker/docker-containers.query.ts
web/**/*.{query.ts,mutation.ts}
📄 CodeRabbit inference engine (.cursor/rules/web-graphql.mdc)
All GraphQL under
web/must follow the.query.tsand.mutation.tsnaming convention
Files:
web/src/components/Docker/docker-containers.query.ts
🧠 Learnings (1)
📚 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/DockerContainersTable.vue
🔇 Additional comments (11)
web/src/components/Docker/docker-containers.query.ts (1)
91-91: LGTM!The addition of
autoStartWaitto the flatEntries meta section is consistent with the existing field in the containers query and properly supports the startup delay UI feature.web/src/composables/useContainerActions.ts (2)
16-16: LGTM!The optional
setRowsStartingcallback follows the same pattern assetRowsBusyand maintains backward compatibility.
162-194: LGTM!The
handleRowStartStoprefactoring properly manages starting state with correct error handling via try/finally, ensuring the starting state is always cleared.web/src/composables/useDockerTableColumns.ts (2)
29-29: LGTM!The addition of
startingRowIdsto the options interface follows the existing pattern forbusyRowIdsandupdatingRowIds.
90-108: LGTM!The per-row state computation and propagation to
DockerNameCellcorrectly threads throughisRowBusyandisRowStartingflags, enabling the UI to reflect starting states.web/src/components/Docker/DockerContainersTable.vue (3)
112-112: LGTM!The
startingRowIdsreactive set follows the established pattern for tracking row states.
179-186: LGTM!The
setRowsStartinghelper correctly manages the starting state set, mirroring the pattern used insetRowsBusy.
222-222: LGTM!The wiring of
setRowsStartingintouseContainerActionsandstartingRowIdsintouseDockerTableColumnscorrectly connects the starting state management across the component hierarchy.Also applies to: 391-391
web/src/components/Docker/DockerNameCell.vue (3)
13-14: LGTM!The new
isBusyandisStartingoptional props are properly typed and enable the component to render appropriate visual feedback.
34-41: LGTM!The
autoStartWaitandshowDelayedBadgecomputed properties correctly derive the startup delay and determine when to display the delay badge.
87-109: LGTM!The conditional rendering logic correctly prioritizes:
- Loading spinner during busy/updating states
- Startup delay badge during container start with configured delay
- Update available icon otherwise
This provides clear visual feedback for each operation state.
The autoStartWait delay only applies during boot/array start sequence, not when manually starting containers from the UI. Showing a delay badge during manual starts was misleading since no delay actually occurs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
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.