Conversation
…utton.css, and default-dynamix.css
WalkthroughUpdates dialog sizing behavior and header rendering in Browse.page, introduces class-based fullscreen handling, and reorganizes jQuery UI dialog CSS. Adjusts filter UI markup and cleans related styles. Adds positioning tweaks for dialog-embedded file trees. No public API or signature changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrowseJS as Browse.page JS
participant DOM
participant CSS
User->>BrowseJS: Open file dialog / toggle fullscreen
BrowseJS->>DOM: Add/Remove .ui-dialog-content-full on .ui-dialog
DOM->>CSS: Recalculate applied selectors
CSS-->>User: Apply fullscreen or centered dialog styles
BrowseJS->>DOM: Render header (left nav, right controls/filter)
DOM->>CSS: Apply updated filter/control styles
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
emhttp/plugins/dynamix/styles/default-dynamix.css (1)
1504-1575: Fix Biome errors: duplicate background properties; keep WebKit fallback without lint violationsBiome flags duplicate background declarations. Replace the vendor-prefixed background with background-image (+position/repeat/size) and keep the standard background for modern browsers. This preserves fallback and satisfies the linter.
Proposed diff:
-.ui-dialog-buttonpane { +.ui-dfm .ui-dialog-buttonpane { .ui-dialog-buttonset { button { ... - background: - -webkit-gradient( - linear, - left top, - right top, - from(var(--dynamix-jquery-ui-button-background-start)), - to(var(--dynamix-jquery-ui-button-background-end)) - ) - 0 0 no-repeat, - -webkit-gradient( - linear, - left top, - right top, - from(var(--dynamix-jquery-ui-button-background-start)), - to(var(--dynamix-jquery-ui-button-background-end)) - ) 0 100% no-repeat, - -webkit-gradient( - linear, - left bottom, - left top, - from(var(--dynamix-jquery-ui-button-background-start)), - to(var(--dynamix-jquery-ui-button-background-start)) - ) 0 100% no-repeat, - -webkit-gradient( - linear, - left bottom, - left top, - from(var(--dynamix-jquery-ui-button-background-end)), - to(var(--dynamix-jquery-ui-button-background-end)) - ) 100% 100% no-repeat; + /* Legacy WebKit fallback without duplicating the background shorthand */ + background-image: + -webkit-gradient(linear, left top, right top, from(var(--dynamix-jquery-ui-button-background-start)), to(var(--dynamix-jquery-ui-button-background-end))), + -webkit-gradient(linear, left top, right top, from(var(--dynamix-jquery-ui-button-background-start)), to(var(--dynamix-jquery-ui-button-background-end))), + -webkit-gradient(linear, left bottom, left top, from(var(--dynamix-jquery-ui-button-background-start)), to(var(--dynamix-jquery-ui-button-background-start))), + -webkit-gradient(linear, left bottom, left top, from(var(--dynamix-jquery-ui-button-background-end)), to(var(--dynamix-jquery-ui-button-background-end))); + background-position: + 0 0, + 0 100%, + 0 100%, + 100% 100%; + background-repeat: + no-repeat, + no-repeat, + no-repeat, + no-repeat; background: linear-gradient(90deg, var(--dynamix-jquery-ui-button-background-start) 0, var(--dynamix-jquery-ui-button-background-end)) 0 0 no-repeat, linear-gradient(90deg, var(--dynamix-jquery-ui-button-background-start) 0, var(--dynamix-jquery-ui-button-background-end)) 0 100% no-repeat, linear-gradient(0deg, var(--dynamix-jquery-ui-button-background-start) 0, var(--dynamix-jquery-ui-button-background-start)) 0 100% no-repeat, linear-gradient(0deg, var(--dynamix-jquery-ui-button-background-end) 0, var(--dynamix-jquery-ui-button-background-end)) 100% 100% no-repeat; background-size: 100% 2px, 100% 2px, 2px 100%, 2px 100%; &:hover { color: var(--dynamix-jquery-ui-button-hover-color); - background: -webkit-gradient( - linear, - left top, - right top, - from(var(--dynamix-jquery-ui-button-background-start)), - to(var(--dynamix-jquery-ui-button-background-end)) - ); + /* Legacy WebKit fallback */ + background-image: -webkit-gradient( + linear, + left top, + right top, + from(var(--dynamix-jquery-ui-button-background-start)), + to(var(--dynamix-jquery-ui-button-background-end)) + ); background: linear-gradient(90deg, var(--dynamix-jquery-ui-button-background-start) 0, var(--dynamix-jquery-ui-button-background-end)); } } } }If retaining global button styling is intentional, keep the selector as .ui-dialog-buttonpane in the first line of this diff. Otherwise, scoping it to .ui-dfm confines changes to the file manager dialogs.
Also applies to: 1562-1572
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
emhttp/plugins/dynamix/Browse.page(2 hunks)emhttp/plugins/dynamix/sheets/Browse.css(1 hunks)emhttp/plugins/dynamix/sheets/BrowseButton.css(1 hunks)emhttp/plugins/dynamix/styles/default-dynamix.css(2 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
emhttp/plugins/dynamix/styles/default-dynamix.css
[error] 1551-1551: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 1551-1551: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 1571-1571: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
⏰ 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/sheets/BrowseButton.css (1)
166-181: LGTM: filter control alignment tweaksIcon offset and input line-height look consistent with the new header layout and improve vertical alignment without side effects.
| function fullWindow() { | ||
| // this class is used to determine if the dialog is sized via the default CSS in default-dynamix.css or by JS when the user clicks the "expand" button. | ||
| $('.ui-dialog').toggleClass('ui-dialog-content-full'); | ||
| if ($('.ui-dfm .ui-dialog-titlebar-close').html().indexOf('expand')>=0) { |
There was a problem hiding this comment.
Fullscreen toggle should target only the current dialog, not all dialogs
Toggling ui-dialog-content-full on all .ui-dialog elements risks affecting other open dialogs. Limit the class toggle to the current dialog widget.
Proposed diff:
-function fullWindow() {
- // this class is used to determine if the dialog is sized via the default CSS in default-dynamix.css or by JS when the user clicks the "expand" button.
- $('.ui-dialog').toggleClass('ui-dialog-content-full');
+function fullWindow() {
+ // Toggle only the current dialog's container to avoid side effects on other dialogs.
+ var $dlg = dfm.window.dialog && dfm.window.dialog('widget') || dfm.window.closest('.ui-dialog');
+ if ($dlg && $dlg.length) $dlg.toggleClass('ui-dialog-content-full');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function fullWindow() { | |
| // this class is used to determine if the dialog is sized via the default CSS in default-dynamix.css or by JS when the user clicks the "expand" button. | |
| $('.ui-dialog').toggleClass('ui-dialog-content-full'); | |
| if ($('.ui-dfm .ui-dialog-titlebar-close').html().indexOf('expand')>=0) { | |
| function fullWindow() { | |
| // Toggle only the current dialog's container to avoid side effects on other dialogs. | |
| var $dlg = dfm.window.dialog && dfm.window.dialog('widget') || dfm.window.closest('.ui-dialog'); | |
| if ($dlg && $dlg.length) $dlg.toggleClass('ui-dialog-content-full'); | |
| if ($('.ui-dfm .ui-dialog-titlebar-close').html().indexOf('expand')>=0) { |
🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/Browse.page around lines 273 to 276, the function
fullWindow() currently toggles the ui-dialog-content-full class on all
.ui-dialog elements and queries the titlebar of all dialogs; change the
selectors to affect only the dialog containing the clicked control (e.g. find
the closest .ui-dialog from the event target or this), then call toggleClass on
that single dialog element and scope the titlebar/close-button HTML check to
that dialog only so other open dialogs are not affected.
| $('.title .left').html(url.join('<i class="fa fa-chevron-right"></i>')); | ||
| $('.title .right').append('<span class="dfm_filter"><input type="text" id="dfm_filter" class="dfm_filter" oninput="filter(this.value)" autocomplete="off" spellcheck="false" placeholder="_(file type)_"><i class="fa fa-filter dfm_filter"></i></span><i class="fa fa-toggle-off" onclick="toggleTime()" style="cursor:pointer;" title="_(Toggle Time/Age display)_"></i>'); | ||
| table = $('table.indexer'); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Guard against duplicate #dfm_filter and missing .title .right container
If this init code runs more than once or the page structure differs, you can end up with duplicate IDs or no insertion point. Add a guard and fallback.
Proposed diff:
-$('.title .left').html(url.join('<i class="fa fa-chevron-right"></i>'));
-$('.title .right').append('<span class="dfm_filter"><input type="text" id="dfm_filter" class="dfm_filter" oninput="filter(this.value)" autocomplete="off" spellcheck="false" placeholder="_(file type)_"><i class="fa fa-filter dfm_filter"></i></span><i class="fa fa-toggle-off" onclick="toggleTime()" style="cursor:pointer;" title="_(Toggle Time/Age display)_"></i>');
+$('.title .left').html(url.join('<i class="fa fa-chevron-right"></i>'));
+var $right = $('.title .right');
+if ($right.length && $('#dfm_filter').length === 0) {
+ $right.append('<span class="dfm_filter"><input type="text" id="dfm_filter" class="dfm_filter" oninput="filter(this.value)" autocomplete="off" spellcheck="false" placeholder="_(file type)_"><i class="fa fa-filter dfm_filter"></i></span><i class="fa fa-toggle-off" onclick="toggleTime()" style="cursor:pointer;" title="_(Toggle Time/Age display)_"></i>');
+}#!/bin/bash
set -e
# Locate the Browse.page file in the repository
file=$(fd --hidden --glob "Browse.page" 2>/dev/null | head -n 1)
echo "Found file: $file"
# Search for existing references to dfm_filter in the file
echo "Occurrences of 'dfm_filter':"
rg -n "dfm_filter" "$file" || echo "No matches found"
# Check how often the init code is invoked (search for the surrounding function)
echo
echo "Context around the filter initialization:"
rg -C3 "\$\(\'\.title \.right\'\)" "$file"#!/bin/bash
set -e
# Locate the Browse.page file in the repository
file=$(fd --hidden --glob "Browse.page" 2>/dev/null | head -n 1)
echo "Found Browse.page at: $file"
# Show the context around lines 1000–1020 to see if this snippet is within a repeated init block
echo -e "\n--- Context (lines 990–1020) ---"
sed -n '990,1020p' "$file"
# Count occurrences of the dfm_filter ID across the repo
echo -e "\n--- Occurrences of '#dfm_filter' in repo ---"
rg -n "#dfm_filter" || echo "No other matches found"
# Count how many times $('.title .right') is used in this file
echo -e "\n--- Occurrences of $('.title .right') in Browse.page ---"
rg -n "\$\('\.title \.right'\)" "$file"Guard .title .right and prevent duplicate #dfm_filter
Ensure the filter input is only added if the right-hand title container exists and the #dfm_filter element isn’t already present. This avoids duplicate IDs and errors when the container is missing.
• File: emhttp/plugins/dynamix/Browse.page (around lines 1004–1006)
• Wrap the append in a check for the container and the absence of the filter element
Proposed diff:
- $('.title .left').html(url.join('<i class="fa fa-chevron-right"></i>'));
- $('.title .right').append(
- '<span class="dfm_filter">' +
- '<input type="text" id="dfm_filter" class="dfm_filter" ' +
- 'oninput="filter(this.value)" autocomplete="off" spellcheck="false" ' +
- 'placeholder="_(file type)_">' +
- '<i class="fa fa-filter dfm_filter"></i>' +
- '</span>' +
- '<i class="fa fa-toggle-off" onclick="toggleTime()" ' +
- 'style="cursor:pointer;" title="_(Toggle Time/Age display)_">' +
- '</i>'
- );
+ $('.title .left').html(url.join('<i class="fa fa-chevron-right"></i>'));
+ const $right = $('.title .right');
+ // only add filter once, and only if the container exists
+ if ($right.length && $('#dfm_filter').length === 0) {
+ $right.append(
+ '<span class="dfm_filter">' +
+ '<input type="text" id="dfm_filter" class="dfm_filter" ' +
+ 'oninput="filter(this.value)" autocomplete="off" spellcheck="false" ' +
+ 'placeholder="_(file type)_">' +
+ '<i class="fa fa-filter dfm_filter"></i>' +
+ '</span>' +
+ '<i class="fa fa-toggle-off" onclick="toggleTime()" ' +
+ 'style="cursor:pointer;" title="_(Toggle Time/Age display)_"></i>'
+ );
+ }
table = $('table.indexer');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $('.title .left').html(url.join('<i class="fa fa-chevron-right"></i>')); | |
| $('.title .right').append('<span class="dfm_filter"><input type="text" id="dfm_filter" class="dfm_filter" oninput="filter(this.value)" autocomplete="off" spellcheck="false" placeholder="_(file type)_"><i class="fa fa-filter dfm_filter"></i></span><i class="fa fa-toggle-off" onclick="toggleTime()" style="cursor:pointer;" title="_(Toggle Time/Age display)_"></i>'); | |
| table = $('table.indexer'); | |
| $('.title .left').html(url.join('<i class="fa fa-chevron-right"></i>')); | |
| const $right = $('.title .right'); | |
| // only add filter once, and only if the container exists | |
| if ($right.length && $('#dfm_filter').length === 0) { | |
| $right.append( | |
| '<span class="dfm_filter">' + | |
| '<input type="text" id="dfm_filter" class="dfm_filter" ' + | |
| 'oninput="filter(this.value)" autocomplete="off" spellcheck="false" ' + | |
| 'placeholder="_(file type)_">' + | |
| '<i class="fa fa-filter dfm_filter"></i>' + | |
| '</span>' + | |
| '<i class="fa fa-toggle-off" onclick="toggleTime()" ' + | |
| 'style="cursor:pointer;" title="_(Toggle Time/Age display)_"></i>' | |
| ); | |
| } | |
| table = $('table.indexer'); |
🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/Browse.page around lines 1004 to 1006, the code
appends a filter input unconditionally which can create duplicate #dfm_filter
IDs or throw if the right-hand title container is missing; wrap the append in a
guard that first checks the container exists (e.g. $('.title .right').length)
and then checks that no existing filter element is present (e.g. $('.title
.right').find('#dfm_filter').length === 0) before appending the input and toggle
elements so the element is only added once and only when the container is
present.
| .ui-dialog-content dd { | ||
| position: relative; | ||
|
|
||
| .fileTree { | ||
| top: 4rem; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Scope the new dialog dd positioning to file manager dialogs to avoid global impact
Setting position: relative on every dd inside any jQuery UI dialog can have unintended effects elsewhere. Since file manager dialogs already get the ui-dfm class, restrict these rules to that context.
Proposed diff:
-.ui-dialog-content dd {
- position: relative;
-
- .fileTree {
- top: 4rem;
- }
-}
+.ui-dfm .ui-dialog-content dd {
+ position: relative;
+}
+.ui-dfm .ui-dialog-content dd .fileTree {
+ top: 4rem;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .ui-dialog-content dd { | |
| position: relative; | |
| .fileTree { | |
| top: 4rem; | |
| } | |
| } | |
| .ui-dfm .ui-dialog-content dd { | |
| position: relative; | |
| } | |
| .ui-dfm .ui-dialog-content dd .fileTree { | |
| top: 4rem; | |
| } |
🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/sheets/Browse.css around lines 113 to 119, the rules
set position: relative on all dd elements inside .ui-dialog-content which
affects every jQuery UI dialog; restrict the selector to only file manager
dialogs by scoping it under the .ui-dfm class (e.g., target .ui-dfm
.ui-dialog-content dd and nested .fileTree) so only dialogs with ui-dfm receive
the positioning and top offset.
| .ui-dialog { | ||
| /* Center the dialog no matter what */ | ||
| box-sizing: border-box; | ||
|
|
||
| * { | ||
| box-sizing: border-box; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Limit new dialog sizing/centering rules to file manager dialogs (ui-dfm) to avoid app-wide regressions
These global .ui-dialog overrides (positioning, centering, width) can alter every jQuery UI dialog across the UI. Since file manager dialogs carry the ui-dfm class, consider scoping to that to meet the PR’s target while minimizing blast radius.
Proposed diff:
-.ui-dialog {
+.ui-dialog.ui-dfm {
box-sizing: border-box;
* {
box-sizing: border-box;
}
}
-.ui-dialog:not(.ui-dialog-content-full) {
+.ui-dialog.ui-dfm:not(.ui-dialog-content-full) {
/*
- If there is no .ui-dialog-content-full class, then we need to center the dialog no matter what the library JS sets on the element.
- Otherwise, if we do have .ui-dialog-content-full on the element, then we need to use the library JS to position the dialog at "full screen" size.
*/
top: 50% !important;
left: 50% !important;
transform: translate(-50%, -50%) !important;
box-sizing: border-box;
width: 100% !important;
max-width: 100rem;
...
}If you want to keep the button styles local to file manager dialogs as well, also prefix the buttonpane block:
-.ui-dialog-buttonpane {
+.ui-dfm .ui-dialog-buttonpane {
.ui-dialog-buttonset {
button {
...
}
}
}Also applies to: 1470-1502
🏁 Script executed:
#!/bin/bash
# Search for .ui-dfm usage across the repository
echo "=== .ui-dfm occurrences across repo ==="
rg '\.ui-dfm'
# Search for .ui-dfm in the default-dynamix.css file
echo "=== .ui-dfm in default-dynamix.css ==="
rg -n '\.ui-dfm' emhttp/plugins/dynamix/styles/default-dynamix.css
# Search for .ui-dialog selectors in default-dynamix.css
echo "=== .ui-dialog selectors in default-dynamix.css ==="
rg -n '\.ui-dialog' emhttp/plugins/dynamix/styles/default-dynamix.css
# Search for JS that adds the ui-dfm class to dialogs
echo "=== JS addClass ui-dfm ==="
rg -n "addClass.*ui-dfm" emhttp/plugins/dynamix
# Search for .ui-dfm in HTML templates or scripts
echo "=== .ui-dfm in HTML templates ==="
rg -n '\.ui-dfm' emhttp/plugins/dynamixLength of output: 7942
🏁 Script executed:
#!/bin/bash
# Inspect how .ui-dfm is applied in Browse.page via jQuery UI dialog initialization
echo "=== .dialog initialization in Browse.page ==="
rg -n '\.dialog' -n emhttp/plugins/dynamix/Browse.page -A 3 -B 3
# Check for dialogClass usage across dynamix plugin
echo "=== dialogClass occurrences across emhttp/plugins/dynamix ==="
rg -n 'dialogClass' emhttp/plugins/dynamixLength of output: 256878
Limit dialog overrides to File Manager dialogs (ui-dfm)
The global .ui-dialog rules at lines 1462–1468 and 1470–1502 will affect every jQuery UI dialog. Since File Manager dialogs are initialized with classes: { 'ui-dialog': 'ui-dfm' }, scope these rules to .ui-dialog.ui-dfm and adjust the button pane selector accordingly to avoid app-wide regressions:
• Prefix box-sizing and centering rules:
-.ui-dialog {
+.ui-dialog.ui-dfm {
box-sizing: border-box;
* {
box-sizing: border-box;
}
}
-.ui-dialog:not(.ui-dialog-content-full) {
+.ui-dialog.ui-dfm:not(.ui-dialog-content-full) {
/* centering and sizing rules */
top: 50% !important;
left: 50% !important;
transform: translate(-50%, -50%) !important;
width: 100% !important;
max-width: 100rem;
/* … */
}• Prefix the button pane block:
-.ui-dialog-buttonpane {
+.ui-dfm .ui-dialog-buttonpane {
.ui-dialog-buttonset {
button {
/* … */
}
}
}This ensures only File Manager dialogs get the new sizing/centering styles and preserves legacy fallbacks.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .ui-dialog { | |
| /* Center the dialog no matter what */ | |
| box-sizing: border-box; | |
| * { | |
| box-sizing: border-box; | |
| } | |
| } | |
| .ui-dialog.ui-dfm { | |
| box-sizing: border-box; | |
| * { | |
| box-sizing: border-box; | |
| } | |
| } |
🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/styles/default-dynamix.css around lines 1462–1468 and
1470–1502, the rules target global .ui-dialog and will affect every jQuery UI
dialog; change the selectors to scope only File Manager dialogs by prefixing
them with .ui-dialog.ui-dfm (e.g. replace .ui-dialog { ... } and nested * rules
with .ui-dialog.ui-dfm { ... } and .ui-dialog.ui-dfm * { ... }) and update the
button pane selector from .ui-dialog .ui-dialog-buttonpane (and similar
selectors in the later block) to .ui-dialog.ui-dfm .ui-dialog-buttonpane so only
dialogs initialized with classes: { 'ui-dialog': 'ui-dfm' } receive these
sizing/centering styles while leaving global legacy fallbacks untouched.
3 participants
Summary by CodeRabbit
New Features
Style
Refactor