feat: file manager selection now allows pop-out to browser #2502
Conversation
WalkthroughAdds a Browse popout UI for Path inputs: client JS creates/attaches a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Input as Path Input
participant ClientJS as Client JS
participant Anchor as dfm-popout Anchor
participant Browse as /Main/Browse (Window)
Note over ClientJS,Input: Initialization
ClientJS->>Input: ensureFileManagerPopout(input)
ClientJS-->>Anchor: create & attach anchor (returns handle)
ClientJS->>Input: bind input events -> updatePopoutState
Note over Anchor,Browse: User flow
Anchor->>ClientJS: click
ClientJS->>Browse: open "/Main/Browse?dir=<input.value>" (noopener)
Browse-->>ClientJS: selected path (postMessage / callback)
ClientJS->>Input: set chosen path value
ClientJS->>Input: updatePopoutState
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
elibosley
changed the title
![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
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/javascript/jquery.filetree.js (2)
2-2: Consider addingnoreferrerto window features.The
window.opencall uses"noopener"but is missing"noreferrer". While the security risk is minimal for internal navigation, including both follows best practices for preventing information leakage.🔎 Suggested enhancement
In the minified code, locate:
-window.open("/Main/Browse?dir="+encodeURIComponent(n),"_blank","noopener") +window.open("/Main/Browse?dir="+encodeURIComponent(n),"_blank","noopener,noreferrer")
2-2: Consider maintaining unminified source for better maintainability.The entire file is minified, which makes it difficult to review, debug, and maintain. Consider keeping the source in an unminified format and using a build process to generate the minified version for production.
This would improve:
- Code review accuracy
- Debugging experience during development
- Future maintenance and modifications
- Version control diffs
Note: If minifying jQuery plugins directly in source control is an established convention in this project, feel free to disregard this suggestion.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix/javascript/jquery.filetree.js
⏰ 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: Analyze (javascript-typescript)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Check warning
Code scanning / CodeQL
Unsafe HTML constructed from library input Medium
Copilot Autofix
AI 4 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
Check warning
Code scanning / CodeQL
Unsafe HTML constructed from library input Medium
Copilot Autofix
AI 4 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php (1)
875-878: Consider extracting popout HTML to reduce duplication.The popout anchor HTML is duplicated between line 520 (in
makeConfig) and line 877 (inensureFileManagerPopout). If the popout structure changes, both locations must be updated.🔎 Proposed refactor to centralize popout HTML
+// Create popout HTML +function createFileManagerPopout() { + return '<a class="view dfm-popout hand" onclick="openFileManagerFromInput(this)" title="_(Browse)_"><i class="icon-u-tab"></i></a>'; +} + function makeConfig(opts) { confNum += 1; var icons = {'Path':'folder-o', 'Port':'minus-square-o', 'Variable':'file-text-o', 'Label':'tags', 'Device':'play-circle-o'}; var newConfig = $("#templateDisplayConfig").html(); var popout = ''; if (opts.Type == "Path") { - popout = "<a class='view dfm-popout hand' onclick='openFileManagerFromInput(this)' title=\"_(Browse)_\"><i class=\"icon-u-tab\"></i></a>"; + popout = createFileManagerPopout(); }Then update
ensureFileManagerPopout:function ensureFileManagerPopout(input) { if ($(input).siblings('.dfm-popout').length) return; - $(input).after('<a class="view dfm-popout hand" onclick="openFileManagerFromInput(this)" title="_(Browse)_"><i class="icon-u-tab"></i></a>'); + $(input).after(createFileManagerPopout()); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.phpemhttp/plugins/dynamix/javascript/jquery.filetree.js
🚧 Files skipped from review as they are similar to previous changes (1)
- emhttp/plugins/dynamix/javascript/jquery.filetree.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-28T15:54:58.673Z
Learnt from: mgutt
Repo: unraid/webgui PR: 2496
File: emhttp/plugins/dynamix/Browse.page:901-906
Timestamp: 2025-12-28T15:54:58.673Z
Learning: In the unraid/webgui codebase, CSRF validation is centralized in the global auto_prepend_file (local_prepend.php) which runs before every PHP request. Do not add per-script CSRF checks in individual files like Browse.page or Control.php. If a script relies on global validation, ensure it does not duplicate CSRF logic; otherwise extend the central preface to cover the needed checks.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
⏰ 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: Analyze (javascript-typescript)
🔇 Additional comments (3)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php (3)
867-873: LGTM!The two-tier search strategy (siblings first, then closest span) handles different DOM structures gracefully. The input value is properly sanitized by
openFileManagerPath, so there are no security concerns.
518-521: The icon class is correct and the pattern is consistent with codebase conventions.The
icon-u-tabicon class is properly defined inemhttp/plugins/dynamix/styles/default-fonts.cssand renders correctly. The popout anchor structure with inlineonclickhandler matches the established pattern throughout the codebase (e.g., similar elements in DiskList.php, ShareList.php, and Browse.php). Thehandclass provides appropriate visual feedback for keyboard users, and addinghrefattributes is not required to match the codebase's existing accessibility approach in this context.
861-865: LGTM. The function correctly handles security withencodeURIComponentandnoopener. The/Main/Browseendpoint accepts thedirparameter and validates it server-side usingvaliddir(), which restricts access to/mntand/bootdirectories and prevents directory traversal viarealpath(). The early return for empty paths is good defensive programming.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
emhttp/plugins/dynamix/javascript/jquery.filetree.js (1)
2-2: Minified code limits review effectiveness.The entire file is minified, making it extremely difficult to perform a thorough code review. While minification is appropriate for production, having the source file in the repository (with a build step for minification) would improve maintainability and security review capabilities.
Consider maintaining the unminified source in the repository and adding a build step for minification, or at least ensure a source map is available for debugging.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix/javascript/jquery.filetree.jsemhttp/plugins/dynamix/styles/default-dynamix.css
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-05-08T18:20:16.531Z
Learnt from: zackspear
Repo: unraid/webgui PR: 2185
File: emhttp/plugins/dynamix/Shares.page:5-15
Timestamp: 2025-05-08T18:20:16.531Z
Learning: In the Unraid WebGUI codebase, translation strings can use Markdown syntax (like `**text**` for bold) which will be properly parsed and rendered in the UI, rather than requiring HTML tags.
Applied to files:
emhttp/plugins/dynamix/javascript/jquery.filetree.js
📚 Learning: 2025-07-11T17:24:33.977Z
Learnt from: Squidly271
Repo: unraid/webgui PR: 2293
File: emhttp/plugins/dynamix.docker.manager/include/Helpers.php:302-304
Timestamp: 2025-07-11T17:24:33.977Z
Learning: In the Unraid WebGUI Docker manager (emhttp/plugins/dynamix.docker.manager/include/Helpers.php), the xmlSecurity() function call is intentionally commented out to allow users to include < and > characters in their Docker template variables on their own servers. Default templates still explicitly disallow tags, but users can customize templates as needed in their self-hosted environment.
Applied to files:
emhttp/plugins/dynamix/javascript/jquery.filetree.js
📚 Learning: 2025-06-21T00:10:40.789Z
Learnt from: zackspear
Repo: unraid/webgui PR: 2258
File: emhttp/plugins/dynamix/DashStats.page:0-0
Timestamp: 2025-06-21T00:10:40.789Z
Learning: In the Unraid webgui codebase (emhttp/plugins/dynamix), replacing `<i>` elements with `<button>` elements for accessibility would require extensive CSS refactoring due to legacy CSS having direct button styles that would conflict with icon-based toggles.
Applied to files:
emhttp/plugins/dynamix/javascript/jquery.filetree.js
📚 Learning: 2025-06-03T21:27:15.912Z
Learnt from: zackspear
Repo: unraid/webgui PR: 2230
File: emhttp/plugins/dynamix/include/Templates.php:63-74
Timestamp: 2025-06-03T21:27:15.912Z
Learning: In the Unraid WebGUI codebase (emhttp/plugins/dynamix/include/Templates.php), there are known duplicate ID issues in checkbox templates across multiple template instances that the maintainers are aware of but have chosen not to address due to the effort required for legacy code improvements.
Applied to files:
emhttp/plugins/dynamix/javascript/jquery.filetree.js
📚 Learning: 2025-05-08T18:20:08.009Z
Learnt from: zackspear
Repo: unraid/webgui PR: 2185
File: emhttp/plugins/dynamix.vm.manager/VMs.page:23-23
Timestamp: 2025-05-08T18:20:08.009Z
Learning: In the Unraid webGUI, text within translation strings (_() function) is parsed as Markdown, so Markdown syntax like `**bold**` is appropriate and will be properly rendered.
Applied to files:
emhttp/plugins/dynamix/javascript/jquery.filetree.js
📚 Learning: 2025-05-08T18:20:24.450Z
Learnt from: zackspear
Repo: unraid/webgui PR: 2185
File: emhttp/plugins/dynamix.docker.manager/Docker.page:23-23
Timestamp: 2025-05-08T18:20:24.450Z
Learning: In the Unraid WebGUI codebase, messages within the `_()` translation function support markdown syntax (e.g., `**text**` for bold) which will be properly parsed and rendered when displayed.
Applied to files:
emhttp/plugins/dynamix/javascript/jquery.filetree.js
📚 Learning: 2025-05-30T00:50:13.286Z
Learnt from: zackspear
Repo: unraid/webgui PR: 2212
File: emhttp/plugins/dynamix/include/DefaultPageLayout/MainContentTabbed.php:37-47
Timestamp: 2025-05-30T00:50:13.286Z
Learning: The .page template system in unraid/webgui is legacy implementation. There is no current appetite to change the eval() usage in .page template files like MainContentTabbed.php, even when security concerns are raised.
Applied to files:
emhttp/plugins/dynamix/javascript/jquery.filetree.js
🪛 GitHub Check: CodeQL
emhttp/plugins/dynamix/javascript/jquery.filetree.js
[warning] 2-2: Unsafe HTML constructed from library input
This HTML construction which depends on library input might later allow cross-site scripting.
[warning] 2-2: Unsafe HTML constructed from library input
This HTML construction which depends on library input might later allow cross-site scripting.
🔇 Additional comments (3)
emhttp/plugins/dynamix/styles/default-dynamix.css (1)
1751-1757: LGTM! Clean CSS implementation for the popout button.The spacing adjustments properly position the popout button by adding padding to the input and using negative margin to overlay the button. The values are well-coordinated to create a seamless inline button appearance.
emhttp/plugins/dynamix/javascript/jquery.filetree.js (2)
2-2: CodeQL XSS warnings are false positives; server-side validation is robust.The
/Main/Browseendpoint implements strong path traversal protection viarealpath()combined with an allowlist check restricting access to only/mntand/bootdirectories. Thedirparameter is properly escaped withhtmlspecialchars()before output.The client-side code is also secure:
- jQuery element creation (safe)
encodeURIComponent()for URL encodingwindow.open()with"noopener"The multi-layer defense (realpath + allowlist validation + HTML escaping) effectively prevents path traversal and XSS attacks.
2-2: Replace_(Browse)_with proper client-side translation syntax.The
title="_(Browse)_"attribute uses server-side translation placeholder syntax (_(text)_) that's only processed by PHP'sparse_text()function. The JavaScript_()function is a stub that returns input unchanged, so this placeholder will render as literal text_(Browse)_to users instead of being translated. Either use_("Browse")if translation is handled elsewhere, or provide a static string like"Browse"if translation is not needed.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-05-31T00:04:34.204Z
Learnt from: zackspear
Repo: unraid/webgui PR: 0
File: :0-0
Timestamp: 2025-05-31T00:04:34.204Z
Learning: For the unraid/webgui legacy codebase, ARIA labels and accessibility concerns should not be addressed in individual PRs. These will be handled via a different solution in the future.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
📚 Learning: 2025-10-03T02:57:29.994Z
Learnt from: ljm42
Repo: unraid/webgui PR: 2414
File: etc/rc.d/rc.nginx:374-376
Timestamp: 2025-10-03T02:57:29.994Z
Learning: Repo unraid/webgui: In etc/rc.d/rc.nginx, maintainers prefer not to add explicit mv-failure checks or EXIT trap clearing around atomic writes in build_servers(), build_locations(), and build_ini(); treat mv failures (e.g., disk full/permissions) as non-recoverable and keep the implementation simple.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
📚 Learning: 2025-08-13T03:13:53.283Z
Learnt from: Squidly271
Repo: unraid/webgui PR: 2333
File: emhttp/plugins/dynamix/include/publish.php:28-36
Timestamp: 2025-08-13T03:13:53.283Z
Learning: In the unraid/webgui codebase, PR #2333 by Squidly271 supersedes earlier work that already addressed all legacy publish() calls using the old overloaded syntax. There are no remaining legacy calls that need backward compatibility handling in the publish() function.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
📚 Learning: 2025-02-27T21:52:42.541Z
Learnt from: dlandon
Repo: unraid/webgui PR: 2035
File: emhttp/plugins/dynamix/SyslogFilter.page:19-26
Timestamp: 2025-02-27T21:52:42.541Z
Learning: In the unraid/webgui repository, basic error handling is sufficient for file operations like checking file existence and using the @ operator to suppress errors, without additional error messages or notifications.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
📚 Learning: 2025-12-28T15:54:58.673Z
Learnt from: mgutt
Repo: unraid/webgui PR: 2496
File: emhttp/plugins/dynamix/Browse.page:901-906
Timestamp: 2025-12-28T15:54:58.673Z
Learning: In the unraid/webgui codebase, CSRF validation is centralized in the global auto_prepend_file (local_prepend.php) which runs before every PHP request. Do not add per-script CSRF checks in individual files like Browse.page or Control.php. If a script relies on global validation, ensure it does not duplicate CSRF logic; otherwise extend the central preface to cover the needed checks.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
⏰ 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: Analyze (javascript-typescript)
🔇 Additional comments (3)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php (3)
518-521: LGTM: Popout correctly restricted to Path type in template.The conditional logic correctly ensures only Path-type configurations receive the popout button in the rendered template, which aligns with the intended functionality.
839-842: Improved fileTree detection and insertion logic.The change from
next()tosiblings()at line 842 is more robust, as it correctly detects an open fileTree regardless of whether the popout element is between the input and the tree. The dynamicinsertAfterlogic at lines 839-840 ensures the fileTree is positioned correctly after the popout when present.
862-887: Well-structured popout helper functions.The four new functions are cleanly implemented:
openFileManagerPathproperly encodes the path and usesnoopenerfor securityopenFileManagerFromInputincludes fallback logic to locate the input elementensureFileManagerPopoutguards against duplicate popoutsupdatePopoutStateprovides dynamic show/hide behavior
| $('input[name="confValue[]"]').each(function() { | ||
| if ($(this).siblings('.dfm-popout').length) { | ||
| $(this).addClass('dfm-popout-target').on('input change', function() { | ||
| updatePopoutState(this); | ||
| }); | ||
| updatePopoutState(this); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Event listeners not attached on initial page load.
This document-ready handler at lines 903-910 attempts to attach popout event listeners and initialize popout state for existing inputs. However, it runs before the handler at lines 1903-1973 that loads Settings and creates the config inputs via makeConfig.
As a result, $('input[name="confValue[]"]') returns an empty collection, so no event listeners are attached and updatePopoutState is never called for configs loaded from an existing template.
Impact: Popouts won't reflect the correct initial state (visible/hidden) until the user interacts with the input, and dynamic updates on value changes won't work.
Proposed fix
Move this logic to run after configs are created, either by:
Option 1: Move it into the Settings-loading handler after configs are appended:
} else {
$("#configLocation").append(newConf);
}
}
+ // Initialize popout state for all Path configs
+ $('input[name="confValue[]"]').each(function() {
+ if ($(this).siblings('.dfm-popout').length) {
+ $(this).addClass('dfm-popout-target').on('input change', function() {
+ updatePopoutState(this);
+ });
+ updatePopoutState(this);
+ }
+ });
} else {
$('#canvas').find('#Overview:first').hide();
}Option 2: Extract to a named function and call it after loading Settings:
+function initializePopoutListeners() {
+ $('input[name="confValue[]"]').each(function() {
+ if ($(this).siblings('.dfm-popout').length) {
+ $(this).addClass('dfm-popout-target').on('input change', function() {
+ updatePopoutState(this);
+ });
+ updatePopoutState(this);
+ }
+ });
+}
+
$(function() {
- $('input[name="confValue[]"]').each(function() {
- if ($(this).siblings('.dfm-popout').length) {
- $(this).addClass('dfm-popout-target').on('input change', function() {
- updatePopoutState(this);
- });
- updatePopoutState(this);
- }
- });
var ctrl = "<span class='status'><input type='checkbox' class='advancedview'></span>";Then call initializePopoutListeners(); after line 1957.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php around lines
903-910 the popout listener/initialization runs too early (before Settings and
config inputs are created at ~lines 1903-1973), so
$('input[name="confValue[]"]') is empty; move the logic to run after configs are
created by either moving the block into the Settings-loading handler or
extracting it as a named function (e.g., initializePopoutListeners) and calling
that function immediately after makeConfig/after line 1957 so existing inputs
get their dfm-popout class, event listeners, and initial updatePopoutState
invocation.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php (2)
837-840: Restrict popout to supported paths (mnt/boot only).The
ensureFileManagerPopout(p)call executes for all inputs, including Device-type inputs that browse/dev(lines 545, 812). The Browse.page endpoint'svaliddir()function only accepts paths under/mntor/boot, so Device paths will show a non-functional popout button.Add a root-path check before calling
ensureFileManagerPopout:Proposed fix
function openFileBrowser(el, top, root, filter, on_folders, on_files, close_on_select) { if (on_folders === undefined) on_folders = true; if (on_files === undefined) on_files = true; if (!filter && !on_files) filter = 'HIDE_FILES_FILTER'; if (!root.trim()) {root = "/mnt/user/"; top = "/mnt/";} p = $(el); - ensureFileManagerPopout(p); - updatePopoutState(p); + // Only add popout for paths supported by /Main/Browse (mnt or boot) + var rootPath = root.split('/')[1] || ''; + if (['mnt', 'boot'].includes(rootPath)) { + ensureFileManagerPopout(p); + updatePopoutState(p); + } var popout = p.siblings('.dfm-popout');
912-919: Event listeners not attached—runs before configs are created.This document-ready handler executes before the Settings-loading handler at lines 1912-1982, which creates the config inputs via
makeConfig(line 1960). As a result,$('input[name="confValue[]"]')returns an empty collection, so no event listeners are attached andupdatePopoutStatenever runs for existing template configs.Move this initialization to run after configs are created:
Proposed fix
Option 1: Move into the Settings-loading handler after configs are appended:
} else { $("#configLocation").append(newConf); } } + // Initialize popout state for all Path configs + $('input[name="confValue[]"]').each(function() { + if ($(this).siblings('.dfm-popout').length) { + $(this).addClass('dfm-popout-target').on('input change', function() { + updatePopoutState(this); + }); + updatePopoutState(this); + } + }); } else { $('#canvas').find('#Overview:first').hide(); }Option 2: Extract to a named function and call after loading Settings:
+function initializePopoutListeners() { + $('input[name="confValue[]"]').each(function() { + if ($(this).siblings('.dfm-popout').length) { + $(this).addClass('dfm-popout-target').on('input change', function() { + updatePopoutState(this); + }); + updatePopoutState(this); + } + }); +} + $(function() { - $('input[name="confValue[]"]').each(function() { - if ($(this).siblings('.dfm-popout').length) { - $(this).addClass('dfm-popout-target').on('input change', function() { - updatePopoutState(this); - }); - updatePopoutState(this); - } - }); var ctrl = "<span class='status'><input type='checkbox' class='advancedview'></span>";Then call
initializePopoutListeners();after line 1966.
🧹 Nitpick comments (1)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php (1)
891-896: LGTM: State management keeps UI clean.Hiding the popout when the input is empty is a reasonable UX decision, though it means users won't discover the feature until they enter a path. Consider if showing a disabled popout for empty inputs would improve discoverability, but the current approach is acceptable.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.phpemhttp/plugins/dynamix/javascript/jquery.filetree.js
🚧 Files skipped from review as they are similar to previous changes (1)
- emhttp/plugins/dynamix/javascript/jquery.filetree.js
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-05-31T00:04:34.204Z
Learnt from: zackspear
Repo: unraid/webgui PR: 0
File: :0-0
Timestamp: 2025-05-31T00:04:34.204Z
Learning: For the unraid/webgui legacy codebase, ARIA labels and accessibility concerns should not be addressed in individual PRs. These will be handled via a different solution in the future.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
📚 Learning: 2025-10-03T02:57:29.994Z
Learnt from: ljm42
Repo: unraid/webgui PR: 2414
File: etc/rc.d/rc.nginx:374-376
Timestamp: 2025-10-03T02:57:29.994Z
Learning: Repo unraid/webgui: In etc/rc.d/rc.nginx, maintainers prefer not to add explicit mv-failure checks or EXIT trap clearing around atomic writes in build_servers(), build_locations(), and build_ini(); treat mv failures (e.g., disk full/permissions) as non-recoverable and keep the implementation simple.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
📚 Learning: 2025-08-13T03:13:53.283Z
Learnt from: Squidly271
Repo: unraid/webgui PR: 2333
File: emhttp/plugins/dynamix/include/publish.php:28-36
Timestamp: 2025-08-13T03:13:53.283Z
Learning: In the unraid/webgui codebase, PR #2333 by Squidly271 supersedes earlier work that already addressed all legacy publish() calls using the old overloaded syntax. There are no remaining legacy calls that need backward compatibility handling in the publish() function.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
📚 Learning: 2025-02-27T21:52:42.541Z
Learnt from: dlandon
Repo: unraid/webgui PR: 2035
File: emhttp/plugins/dynamix/SyslogFilter.page:19-26
Timestamp: 2025-02-27T21:52:42.541Z
Learning: In the unraid/webgui repository, basic error handling is sufficient for file operations like checking file existence and using the @ operator to suppress errors, without additional error messages or notifications.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
📚 Learning: 2025-09-05T19:26:36.587Z
Learnt from: zackspear
Repo: unraid/webgui PR: 2354
File: emhttp/plugins/dynamix/ShareEdit.page:0-0
Timestamp: 2025-09-05T19:26:36.587Z
Learning: In emhttp/plugins/dynamix/ShareEdit.page, the clone-settings div was moved outside the form element and both are wrapped in a div.relative container to prevent event bubbling issues while maintaining proper positioning.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
📚 Learning: 2025-12-28T15:54:58.673Z
Learnt from: mgutt
Repo: unraid/webgui PR: 2496
File: emhttp/plugins/dynamix/Browse.page:901-906
Timestamp: 2025-12-28T15:54:58.673Z
Learning: In the unraid/webgui codebase, CSRF validation is centralized in the global auto_prepend_file (local_prepend.php) which runs before every PHP request. Do not add per-script CSRF checks in individual files like Browse.page or Control.php. If a script relies on global validation, ensure it does not duplicate CSRF logic; otherwise extend the central preface to cover the needed checks.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
⏰ 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: Analyze (javascript-typescript)
🔇 Additional comments (8)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php (8)
518-521: LGTM: Popout button correctly scoped to Path configurations.The conditional logic appropriately adds the file manager popout only for Path-type configurations, avoiding Device types that browse
/dev.
536-537: LGTM: Template integration is correct.The popout HTML is properly passed to the template format string and positioned correctly in the output.
842-842: LGTM: Sibling check correctly handles popout presence.Changing from
.next()to.siblings()properly accounts for the popout button that may exist between the input and fileTree.
846-846: LGTM: FileTree insertion point correctly accounts for popout.Using the
insertAftervariable ensures the fileTree is positioned after the popout button (if present), maintaining proper DOM structure.
862-875: LGTM: File detection heuristic is reasonable for this use case.The function correctly:
- Sanitizes input by trimming and encoding
- Strips filenames to open the parent directory
- Uses
noopenerfor securityThe file detection (checking for
.in the last path segment) is simplistic—it won't catch extension-less files or directories with dots—but opening the parent directory for file paths is acceptable UX for this feature.
877-883: LGTM: Input lookup handles multiple DOM contexts.The fallback logic correctly handles both template-generated configs (sibling) and popup dialogs (within closest span).
885-889: LGTM: Popout creation prevents duplicates.The sibling check correctly prevents duplicate popout buttons, and the DOM insertion order (after the input) is appropriate.
1595-1595: LGTM: Template placeholder correctly positioned.The
{14}placeholder is properly positioned to render the popout button adjacent to the input field within the flex container.
Adds a "pop out" button to Jquery file browser to allow viewing selected folder in the build in Unraid file browser
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.