Skip to content

Conversation

@mgutt
Copy link
Contributor

@mgutt mgutt commented Dec 27, 2025

Prevent fileTree dropdown from closing when clicking inside the file manager dialog by stopping mousedown event propagation. This fixes the issue where clicking the scrollbar would inadvertently close the target folder selection dropdown.

Solution: Added preventFileTreeClose() function that prevents mousedown events inside .ui-dfm dialogs from bubbling to the document-level handler that closes the fileTree dropdown.

Fixes #2487

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where file-tree dropdowns would unexpectedly close when clicking inside action dialogs. The dialogs now consistently prevent this closure both when opened and after actions complete, improving file-management stability and usability.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 27, 2025

Walkthrough

Added a DOM event-prevention routine to keep the file manager's fileTree dropdown from closing when clicking inside action dialogs (e.g., on the scrollbar) and wired that routine into the dialog open/close flows for copy/move actions.

Changes

Cohort / File(s) Summary
File Manager Dialog Event Prevention
emhttp/plugins/dynamix/Browse.page
Added preventFileTreeClose() which binds a mousedown.dfmFileTree handler to .ui-dfm elements to stop propagation; ensured the handler is applied when opening action dialogs and unbound on dialog close; invoked after dialog close/finalize in doAction, doActions, and related flows.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I hopped in code to mend the fray,
The scrollbar click no longer runs away,
Now dialogs hold fast when users explore,
A quiet fix — click, scroll, and more! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main fix: preventing the fileTree dropdown from closing when the scrollbar is clicked, which directly addresses the issue reported.
Linked Issues check ✅ Passed The code changes directly address issue #2487 by implementing event propagation prevention to stop fileTree dropdown from closing when clicking inside the file manager dialog.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the fileTree dropdown closure issue and are directly scoped to the requirements in issue #2487.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 96b9351 and b6f3080.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix/Browse.page
🚧 Files skipped from review as they are similar to previous changes (1)
  • emhttp/plugins/dynamix/Browse.page

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.

@github-actions
Copy link

github-actions bot commented Dec 27, 2025

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2025.12.27.1159
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

  1. Go to Plugins → Install Plugin
  2. Copy and paste this URL:
https://preview.dl.unraid.net/pr-plugins/pr-2492/webgui-pr-2492.plg
  1. Click Install

Alternative: Direct Download

⚠️ Important Notes:

  • Testing only: This plugin is for testing PR changes
  • Backup included: Original files are automatically backed up
  • Easy removal: Files are restored when plugin is removed
  • Conflicts: Remove this plugin before installing production updates

📝 Modified Files:

Click to expand file list
emhttp/plugins/dynamix/Browse.page

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2492, or run:

plugin remove webgui-pr-2492

🤖 This comment is automatically generated and will be updated with each new push to this PR.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
emhttp/plugins/dynamix/Browse.page (2)

603-603: Optional: Consider calling only for actions with fileTree dropdowns.

The function is invoked for all actions, but the fileTree dropdown is only opened for copy/move operations (cases 3, 4, 8, 9 where fileTreeAttach is called). While calling it unconditionally is harmless, you could optimize by placing it inside the relevant case blocks or adding a conditional check.

🔎 Example optimization
   });
   dfm_close_button();
-  preventFileTreeClose();
+  if ([3, 4, 8, 9].includes(action)) preventFileTreeClose();
   if (action == 15) $('.ui-dfm .ui-dialog-buttonset button:eq(1)').prop('disabled',true);

874-874: Optional: Same optimization applies here as in doAction.

Similar to line 603, this invocation could be optimized to only run for actions that use fileTree dropdowns (cases 3 and 4 in the doActions function).

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 987fa42 and 96b9351.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix/Browse.page
🧰 Additional context used
🧠 Learnings (1)
📚 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/Browse.page

- Stop mousedown event propagation in .ui-dfm dialogs
- Use namespaced event (.dfmFileTree) for clean removal
- Cleanup automatically via jQuery UI Dialog close callback

Fixes unraid#2487
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.

Copy / Move Operation Hides Target in File Manager

1 participant