feat(deployments): Add bind-mount creation from the file browser#62
Conversation
Code Review SummaryThis PR successfully implements a 'one-click' bind-mount feature from the file browser. It includes a custom regex-based parser for Docker Compose files to identify existing mounts and available services, providing a seamless UX for wiring host files into containers. 🚀 Key Improvements
💡 Minor Suggestions
|
| </div> | ||
| <div class="modal-footer"> | ||
| <button class="btn btn-secondary" @click="showMountModal = false">Cancel</button> | ||
| <button class="btn btn-primary" :disabled="!mountTargetPath.trim()" @click="confirmMount"> |
There was a problem hiding this comment.
The 'Add Mount' button lacks a visual loading state. If the API call takes time, the user might click multiple times. While the modal closes immediately in confirmMount, it is better practice to disable the button or show a spinner if the operation is asynchronous.
| <button class="btn btn-primary" :disabled="!mountTargetPath.trim()" @click="confirmMount"> | |
| <button class="btn btn-primary" :disabled="!mountTargetPath.trim() || isSubmitting" @click="confirmMount"> | |
| <i class="pi" :class="isSubmitting ? 'pi-spin pi-spinner' : 'pi-check'" /> | |
| Add Mount | |
| </button> |
|
|
||
| export function toComposeRelativePath(path: string): string { | ||
| if (!path || path === "/") return "."; | ||
| return path.startsWith("/") ? `.${path}` : path; |
There was a problem hiding this comment.
This logic might result in '..//path' if the input is '/path'. While many systems normalize this, it is safer to handle the slash explicitly to ensure a clean relative path like './path'.
| return path.startsWith("/") ? `.${path}` : path; | |
| return path.startsWith("/") ? `.${path}` : `./${path}`; |
Deploying flatrun-ui with
|
| Latest commit: |
97908e2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://21e270a7.flatrun-ui.pages.dev |
| Branch Preview URL: | https://feat-compose-mount-from-file.flatrun-ui.pages.dev |
| return value; | ||
| } | ||
|
|
||
| function parseShortMount(value: string): Omit<ComposeMount, "service"> | null { |
There was a problem hiding this comment.
The parser assumes a simple colon-delimited string. This will fail on Windows-style host paths (e.g., C:\path:target) or if quotes are used around individual parts. While sufficient for Linux-based containers, adding a check for empty parts improves robustness.
| function parseShortMount(value: string): Omit<ComposeMount, "service"> | null { | |
| function parseShortMount(value: string): Omit<ComposeMount, "service"> | null { | |
| const parts = value.split(":"); | |
| // Basic validation to ensure we don't process malformed strings | |
| if (parts.length < 2 || parts.length > 3) return null; | |
| const source = parts[0].trim(); | |
| const target = parts[1].trim(); | |
| if (!source || !target) return null; |
Browsing a deployment's files now offers a one-click way to mount any file or directory into a compose service. The dialog reuses the deployment's service list, defaults the container path to the host path, and surfaces the updated compose tab after the change so the result is visible in context. A file or directory already wired into a compose service shows a distinct indicator on its mount button, with a tooltip naming the services that already consume it. The mount targets are detected by parsing the deployment's compose for short-syntax bind entries. The file browser action row was getting visually heavy as features landed; row buttons are slightly tighter and the actions column reserves less horizontal space so the surrounding layout breathes.
9a99a4c to
97908e2
Compare
| } | ||
|
|
||
| function parseShortMount(value: string): Omit<ComposeMount, "service"> | null { | ||
| const parts = value.split(":"); |
There was a problem hiding this comment.
The current split(':') assumes the path itself doesn't contain a colon. While uncommon on Linux, it is a valid character in some scenarios. However, more importantly, it misses trimming whitespace from the individual parts, which can lead to incorrect path values if the YAML has spaces around the colon (e.g., - ./src : /app).
| const parts = value.split(":"); | |
| const parts = value.split(':').map(p => p.trim()); |
Browsing a deployment's files now offers a one-click way to mount any file or directory into a compose service. The dialog reuses the deployment's service list, defaults the container path to the host path, and surfaces the updated compose tab after the change so the result is visible in context.
Files already mounted in any service show a distinct indicator on the mount button with a tooltip naming the consuming services, so users can see what is already wired at a glance.
Closes #61