fix: update HTML structure in Templates.php to improve formatting and…#2402
fix: update HTML structure in Templates.php to improve formatting and…#2402
Conversation
… consistency of warning messages
WalkthroughRefactors HTML in emhttp/plugins/dynamix/include/Templates.php to replace inline warning divs with dt/dd notice pairs, adding dt class "dfm_noticeLabel" and dd class "dfm_notice". Inserts non-breaking spaces in some places. Message content and icons remain the same; only DOM structure and spacing change. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
emhttp/plugins/dynamix/include/Templates.php (9)
24-25: Unify notice layout: wrap this warning in dt/dd like others.Align with the new dfm_noticeLabel/dfm_notice structure for consistency.
- -: <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This creates a folder at the current level)_</div> +<dt class="dfm_noticeLabel"> </dt> +<dd class="dfm_notice"> + <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This creates a folder at the current level)_</div> +</dd>
35-36: Unify notice layout: use dt/dd wrapper.Match the new notice pattern used elsewhere.
- -: <div class="dfm_info"><i class="fa fa-warning dfm"></i><?=_("This deletes the folder and all its content")?></div> +<dt class="dfm_noticeLabel"> </dt> +<dd class="dfm_notice"> + <div class="dfm_info"><i class="fa fa-warning dfm"></i><?=_("This deletes the folder and all its content")?></div> +</dd>
52-53: Unify notice layout: use dt/dd wrapper.Keep warning rows consistent.
- -: <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This renames the folder to the new name)_</div> +<dt class="dfm_noticeLabel"> </dt> +<dd class="dfm_notice"> + <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This renames the folder to the new name)_</div> +</dd>
121-122: Unify notice layout: use dt/dd wrapper.Bring delete file notice in line with others.
- -: <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This deletes the selected file)_</div> +<dt class="dfm_noticeLabel"> </dt> +<dd class="dfm_notice"> + <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This deletes the selected file)_</div> +</dd>
138-139: Unify notice layout: use dt/dd wrapper.Align rename file notice with the new structure.
- -: <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This renames the selected file)_</div> +<dt class="dfm_noticeLabel"> </dt> +<dd class="dfm_notice"> + <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This renames the selected file)_</div> +</dd>
207-208: Unify notice layout: use dt/dd wrapper.Align delete object notice with new structure.
- -: <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This deletes all selected sources)_</div> +<dt class="dfm_noticeLabel"> </dt> +<dd class="dfm_notice"> + <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This deletes all selected sources)_</div> +</dd>
221-222: Unify notice layout: use dt/dd wrapper.Align rename object notice with new structure.
- -: <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This renames the selected source)_</div> +<dt class="dfm_noticeLabel"> </dt> +<dd class="dfm_notice"> + <div class="dfm_info"><i class="fa fa-warning dfm"></i>_(This renames the selected source)_</div> +</dd>
36-36: Normalize i18n usage for warning messages.Mixed use of and bare _(…) across notices. Confirm both are equivalent in this template context or standardize on one.
If standardizing, prefer a single approach project‑wide for translator tooling consistency.
Also applies to: 78-78, 122-122, 164-164, 193-193, 247-247, 276-276
76-79: Optional a11y: mark notices as live regions.Consider role="note" with aria-live="polite" on dd.df m_notice so screen readers announce these contextual warnings.
Example:
<dd class="dfm_notice" role="note" aria-live="polite"> … </dd>Also applies to: 105-108, 162-165, 191-194, 245-248, 274-277
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix/include/Templates.php(13 hunks)
⏰ 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 (5)
emhttp/plugins/dynamix/include/Templates.php (5)
105-108: LGTM: consistent notice block for move folderMatches the new pattern.
162-165: LGTM: notice block for copy fileConsistent with the new pattern.
191-194: LGTM: notice block for move fileConsistent usage.
245-248: LGTM: notice block for copy objectConsistent with the new pattern.
274-277: LGTM: notice block for move objectConsistent with the new pattern.
| <dt class="dfm_noticeLabel"> </dt> | ||
| <dd class="dfm_notice"> | ||
| <div class="dfm_info"><i class="fa fa-warning dfm"></i><?=_("This copies the folder and all its content to another folder")?></div> | ||
| </dd> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
❓ Verification inconclusive
LGTM: new dt/dd notice block
The dfm_noticeLabel/dfm_notice structure looks good and improves alignment of warnings.
Please verify these classes are styled in the CSS. You can check with:
🏁 Script executed:
#!/bin/bash
fd -t f -e css | xargs rg -n --color=never -e '\.dfm_notice(Label)?\b'Length of output: 0
Define CSS for .dfm_notice and .dfm_noticeLabel
No matching rules found in any stylesheet; add definitions to the plugin’s CSS so the notice block renders with the intended styling.
🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/include/Templates.php around lines 76-79 the markup
uses classes .dfm_notice and .dfm_noticeLabel but there are no CSS rules; add
definitions to the plugin stylesheet (e.g., dynamix.css) to style the notice
block — create .dfm_noticeLabel to reserve a fixed width/align for the label and
.dfm_notice to display as a block with padding, subtle background, border-radius
and border, appropriate font-size/line-height and icon color; include
spacing/margin rules and ensure the stylesheet is loaded by the plugin so the
notice renders with the intended appearance.
🧹 PR Test Plugin Cleaned UpThe 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. |
This PR addresses multiple issues and feature requests from unraid#2500: 1. Fix PR unraid#2402 Markdown Parser regression (literal colon bug): - Changed all dialog templates from <div class="dfm_info"> to <span class="dfm_text"> - Markdown Extra parser can now handle inline elements without blank line - Removed obsolete <dt>/<dd> wrapper structure in Copy/Move templates - Cleaned up unused dfm_info CSS rules 2. Show total size of running transfer: - Enhanced file_manager rsync parser to calculate total transfer size - Averages 5 measurements at different progress points for ~2% accuracy - Displays 'Transferring X of Y' during operations 3. Show last N used destination paths in FileTree: - New PopularDestinations.php with frequency-based scoring system - Displays top 5 recent paths at FileTree root level - Automatic FUSE conflict prevention (/mnt/user vs /mnt/diskX) - Persistent storage in /boot/config/filemanager.json 4. Manually typing destination path updates FileTree: - New setupTargetNavigation() function - Navigate FileTree via arrow keys and Enter/Escape - Automatic FileTree open/close on input interaction - Prevents FileTree from closing when clicking inside dialog 5. Add 'Open Terminal here' button: - OpenTerminal.php now accepts path parameter via 'more' GET param - Creates custom startup script to set working directory - File Manager can open terminal at selected folder Additional fixes: - Fixed job queue numbering calculation (was off by one) - Improved dialog layout with proper flexbox constraints
This PR addresses multiple issues and feature requests from unraid#2500: 1. Fix PR unraid#2402 Markdown Parser regression (literal colon bug): - Changed all dialog templates from <div class="dfm_info"> to <span class="dfm_text"> - Markdown Extra parser can now handle inline elements without blank line - Removed obsolete <dt>/<dd> wrapper structure in Copy/Move templates - Cleaned up unused dfm_info CSS rules 2. Show total size of running transfer: - Enhanced file_manager rsync parser to calculate total transfer size - Averages 5 measurements at different progress points for ~2% accuracy - Displays 'Transferring X of Y' during operations 3. Show last N used destination paths in FileTree: - New PopularDestinations.php with frequency-based scoring system - Displays top 5 recent paths at FileTree root level - Automatic FUSE conflict prevention (/mnt/user vs /mnt/diskX) - Persistent storage in /boot/config/filemanager.json 4. Manually typing destination path updates FileTree: - New setupTargetNavigation() function - Navigate FileTree via arrow keys and Enter/Escape - Automatic FileTree open/close on input interaction - Prevents FileTree from closing when clicking inside dialog 5. Add 'Open Terminal here' button: - OpenTerminal.php now accepts path parameter via 'more' GET param - Creates custom startup script to set working directory - File Manager can open terminal at selected folder
This PR addresses multiple issues and feature requests from unraid#2500: 1. Fix PR unraid#2402 Markdown Parser regression (literal colon bug) - Changed dialog templates from <div> to <span> for inline processing - Cleaned up unused dfm_info CSS rules 2. Show total size of running transfer - Enhanced rsync parser to calculate total transfer size - Displays 'Transferring X of Y' format 3. Show last N used destination paths in FileTree - New PopularDestinations.php with frequency-based scoring - FUSE conflict prevention for /mnt/user vs /mnt/diskX 4. Manually typing destination path updates FileTree - Keyboard navigation with arrow keys and Enter/Escape - Automatic FileTree open/close on input interaction 5. Add 'Open Terminal here' button - OpenTerminal.php accepts path parameter - File Manager can open terminal at selected folder Note: This PR depends on unraid#2491 and unraid#2490 being merged first.
3 participants
… consistency of warning messages
Summary by CodeRabbit