Skip to content

Fix: Buttons on plugins tab overlap#2388

Merged
limetech merged 2 commits into
masterfrom
fix/pluginButtons
Sep 24, 2025
Merged

Fix: Buttons on plugins tab overlap#2388
limetech merged 2 commits into
masterfrom
fix/pluginButtons

Conversation

@Squidly271

@Squidly271 Squidly271 commented Sep 20, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Style
    • Repositioned plugin action buttons (Check for Updates, Update All Plugins, Remove Selected Plugins) into the tabs container for a more consistent layout.
    • Improves visual alignment with the tab bar, reducing clutter and enhancing readability.
    • Ensures the controls remain grouped with related navigation elements, aiding discoverability.
    • No functional changes to the buttons; behavior remains the same.

@coderabbitai

coderabbitai Bot commented Sep 20, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

A single UI change updates the DOM insertion target for plugin control elements in emhttp/plugins/dynamix.plugin.manager/Plugins.page, switching from the .tabs selector to .tabs-container during initialization. The buttons and their behavior remain unchanged.

Changes

Cohort / File(s) Summary of changes
Plugin manager UI DOM target update
emhttp/plugins/dynamix.plugin.manager/Plugins.page
Updated initialization to append plugin control spans to .tabs-container instead of .tabs; no logic changes to the controls themselves.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant P as Plugins.page
  participant D as DOM (.tabs-container)

  U->>P: Load Plugins page
  activate P
  Note over P: Initialization
  P->>D: Append "Check For Updates", "Update All", "Remove Selected" controls
  deactivate P

  U->>D: Click a control
  D-->>P: Trigger existing handlers (unchanged)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

I hopped through tabs, then paused to stare,
Now buttons nest in containers fair.
Same clicky charms, a tidier lair—
Updates, removes, all handled with care.
Thump-thump, I nudge the UI square. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Fix: Buttons on plugins tab overlap" succinctly and accurately summarizes the primary UI change—fixing overlapping plugin-control buttons on the plugins tab by adjusting their insertion target. It is concise, specific to the affected area, and matches the changes described in the PR summary. A teammate scanning history will understand the main purpose from this title.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/pluginButtons

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Sep 20, 2025

Copy link
Copy Markdown

🔧 PR Test Plugin Available

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

Version: 2025.09.20.2202
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-2388/webgui-pr-2388.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.plugin.manager/Plugins.page

🔄 To Remove:

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

plugin remove webgui-pr-2025.09.20.2202

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
emhttp/plugins/dynamix.plugin.manager/Plugins.page (2)

148-150: Add a resilient fallback if .tabs-container isn’t present

Defensive selection avoids silently missing controls on layouts that don’t render .tabs-container (and aligns with prior guidance to use .title .right when tabs markup differs).

Apply this diff:

-  $('.tabs-container').append("<span id='checkall' class='status vhshift'><input type='button' value=\"_(Check For Updates)_\" onclick='openPlugin(\"checkall\",\"_(Plugin Update Check)_\",\":return\")' disabled></span>");
-  $('.tabs-container').append("<span id='updateall' class='status vhshift' style='display:none;margin-left:12px'><input type='button' value=\"_(Update All Plugins)_\" onclick='updateList()'></span>");
-  $('.tabs-container').append("<span id='removeall' class='status vhshift' style='display:none;margin-left:12px'><input type='button' value=\"_(Remove Selected Plugins)_\" onclick='removeList()'></span>");
+  const $dest = $('.tabs-container').length ? $('.tabs-container')
+                : ($('.title .right').length ? $('.title .right') : $('.tabs'));
+  $dest.append(
+    "<span id='checkall' class='status vhshift'><input type='button' value=\"_(Check For Updates)_\" onclick='openPlugin(\"checkall\",\"_(Plugin Update Check)_\",\":return\")' disabled></span>" +
+    "<span id='updateall' class='status vhshift' style='display:none;margin-left:12px'><input type='button' value=\"_(Update All Plugins)_\" onclick='updateList()'></span>" +
+    "<span id='removeall' class='status vhshift' style='display:none;margin-left:12px'><input type='button' value=\"_(Remove Selected Plugins)_\" onclick='removeList()'></span>"
+  );

149-150: Avoid inline left margins; prefer logical spacing or a CSS rule

Inline margin-left:12px can misbehave in RTL and at tight widths. Consider a CSS rule or logical property:

For example (in CSS): .tabs-container .status + .status { margin-inline-start: 12px; } and drop the inline styles here.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7caf24 and 7033b75.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix.plugin.manager/Plugins.page (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: zackspear
PR: unraid/webgui#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.
Learnt from: zackspear
PR: unraid/webgui#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.
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
📚 Learning: 2025-09-05T19:26:36.587Z
Learnt from: zackspear
PR: unraid/webgui#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.plugin.manager/Plugins.page
📚 Learning: 2025-05-22T20:15:34.297Z
Learnt from: zackspear
PR: unraid/webgui#2203
File: emhttp/plugins/dynamix/include/DefaultPageLayout/HeadInlineJS.php:345-356
Timestamp: 2025-05-22T20:15:34.297Z
Learning: Status updates from the showStatus function should be appended to $('.title .right') rather than $('.tabs') as this element is created by MainContentTabless. This ensures status messages appear in the correct location in the UI layout.

Applied to files:

  • emhttp/plugins/dynamix.plugin.manager/Plugins.page
⏰ 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 (1)
emhttp/plugins/dynamix.plugin.manager/Plugins.page (1)

148-150: LGTM: moving controls to .tabs-container fixes the overlap

This change should resolve the z-index/flow clash with the tab strip; IDs and downstream logic still work as before.

Please sanity-check in light and dark themes at 125–150% zoom and with long translated labels to confirm no wrapping/regression.

@Squidly271

Copy link
Copy Markdown
Contributor Author

@coderabbitai. tabs-container is always present. inline style removal is outside the scope of this quick fix

@coderabbitai

coderabbitai Bot commented Sep 20, 2025

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@Squidly271 Thank you for the clarification! I understand that tabs-container is always available in this context, which makes the DOM target change safe. I also appreciate you clarifying that inline style removal is outside the scope of this quick fix - that makes perfect sense for maintaining focus on the immediate button overlap issue.


✏️ Learnings added
Learnt from: Squidly271
URL: https://github.com/unraid/webgui/pull/2388

Timestamp: 2025-09-20T22:25:36.642Z
Learning: In the unraid/webgui project, the `.tabs-container` element is always present in the DOM for the plugins page, making it a reliable target for DOM manipulation.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.

@limetech limetech merged commit 3732eb2 into master Sep 24, 2025
5 checks passed
@limetech limetech deleted the fix/pluginButtons branch September 24, 2025 22:46
@github-actions

Copy link
Copy Markdown

🧹 PR Test Plugin Cleaned Up

The test plugin and associated files for this PR have been removed from the preview environment.


🤖 This comment is automatically generated when a PR is closed.

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.

2 participants