feat: responsive DefaultPageLayout first pass#2188
Conversation
- Added new styles for #displaybox to enhance layout across themes. - Removed complex old media queries for better clarity and maintenance.
- Changed viewport meta tag from fixed width to device-width for better adaptability across devices.
- Changed media query breakpoint from 1281px to 1921px to enhance layout responsiveness for larger screens.
- Updated #menu and .nav-tile styles for better adaptability on mobile and larger screens. - Adjusted footer positioning based on screen size to maintain layout integrity. - Improved overall responsiveness by implementing media queries for different screen sizes.
- Added flexbox properties to div.title for improved layout and alignment of child elements. - Updated span.status to display as inline-block and added margin for better spacing.
- Commented out the styles for table.tablesorter.shift to prevent margin adjustments that may affect layout
- Changed .nav-tile to use flexbox with a column direction for better alignment of child elements. - Updated .usage-bar positioning to relative and adjusted width for enhanced layout consistency.
…tency - Commented out the margin-right property in span.status.vhshift to prevent potential layout issues.
- Added flexbox properties to body for improved layout structure. - Adjusted div.title.shift by commenting out z-index for better stacking context. - Updated footer positioning and z-index for consistent layout across screen sizes. - Modified styles for a.list and .tabs for better alignment and spacing.
…ability - Reformatted the table structure in ArrayDevices.page for better clarity. - Changed PHP short tags to standard syntax for consistency. - Enhanced the layout by separating the header and body sections for better organization.
- Removed commented-out styles for .Theme--black and .Theme--white to streamline the CSS. - Eliminated redundant margin-top adjustment in span.status.vhshift for improved layout clarity.
- Commented out height property in body to prevent layout shift on scrollable pages. - Restored z-index in div.title.shift for improved stacking context.
- Removed unnecessary media query styles to streamline the CSS. - Added margin reset for <p> elements containing links, scripts, and styles to improve spacing consistency.
- Commented out unnecessary styles in div.last to clean up the CSS. - Introduced a new CSS variable for grid column width to enhance flexibility. - Adjusted grid layout for better responsiveness on larger screens with a media query.
- Adjusted grid layout to have 1 column by default for mobile screens. - Implemented 2 columns for medium screens and 3 columns for larger screens using media queries for enhanced responsiveness.
…layout on smaller screen sizes - Wrapped tables in a <div> with class "TableContainer" for better responsiveness. - Ensured consistent styling and structure across ArrayDevices.page, BootDevice.page, DiskList.page, ShareList.page, DockerContainers.page, Plugins.page, and VMMachines.page. - Updated CSS to support new table container classes for enhanced layout control.
|
""" WalkthroughThis update refactors the HTML and CSS structure of various pages and components to enhance layout flexibility and readability. Tables are now wrapped in scrollable containers, flexbox and grid layouts are introduced for responsiveness, and several legacy or redundant styles are removed or replaced. Some PHP templates now evaluate embedded variables in page titles dynamically without using Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Server
User->>Browser: Request page (e.g., DockerContainers, ArrayDevices)
Browser->>Server: HTTP GET request
Server-->>Browser: HTML with tables wrapped in scrollable containers and updated CSS
Browser->>Browser: Render page with responsive layout and flexbox/grid styles
User->>Browser: Interact with UI (scroll tables, click buttons)
Browser->>Server: Submit forms, trigger actions
Server-->>Browser: Processed responses with consistent layout and styling
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. 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: 0
♻️ Duplicate comments (1)
emhttp/plugins/dynamix/UserList.page (1)
34-38: Duplicate title block — see previous comment
This shares the same structure (and potential icon-class conflict) as the Management Access title above. Please refer to the feedback on lines 18–22.
🧹 Nitpick comments (8)
emhttp/plugins/dynamix/sheets/DashStats.css (2)
21-23: Remove commented-out legacy rules
Leaving dead code commented out can clutter the stylesheet. If the.lastpadding-bottom is no longer needed, consider removing these lines completely or adding a clear TODO explaining why they're retained.
364-366: Enhance grid gaps
You’re currently usingcolumn-gap: 20px;but no row gap. For clarity and consistency, consider using the shorthandgap: 20px;(orrow-gap+column-gap) so both axes are addressed.div.grid { - display: grid; - grid-template-columns: 1fr; /* 1 column by default (mobile) */ - column-gap: 20px; + display: grid; + grid-template-columns: 1fr; + gap: 20px; }emhttp/plugins/dynamix/UserList.page (3)
18-22: Consistent title block markup
Both title blocks use<div class="title">…<span class="left">but you’ve also assignedclass="title"to the<i>icon element. That risks conflicting with the container styles in CSS. Consider using a more specific icon class (e.g.icon-title) to avoid unintended styling side effects.
23-33: Remove inlinestyle="text-align:center"
The.user-listCSS now handles centering. Inline styles should be removed in favor of your stylesheet to keep separation of concerns and reduce redundancy.
43-45: Avoid PHP short open tags
You’re using<?instead of<?php. Ifshort_open_tagis ever disabled, this will break. Please switch to full<?phptags for portability.emhttp/plugins/dynamix/ArrayDevices.page (1)
24-51: Wrap table in scrollable container
Excellent use of the new.TableContainerto enable horizontal scrolling. This aligns perfectly with your CSS additions indefault-base.css.Tip: If you find the disk-status table needs a larger minimum width than 900px (it has 10 columns), consider using
.TableContainer--widestwhich setsmin-width: 1345px.emhttp/plugins/dynamix/styles/default-base.css (2)
10-14: Review commented-out body styles
The block disablingheight: 100vh; display: flex; overflow-y: auto;is understandable to prevent layout shifts, but consider removing it or adding a clear comment explaining why it’s needed for future maintainability.
1114-1115: Remove or document commented rule
The commentedmargin-right: -9px;in.status.vhshiftshould either be removed or accompanied by an explanatory comment to clarify why it’s retained.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
emhttp/plugins/dynamix.docker.manager/DockerContainers.page(1 hunks)emhttp/plugins/dynamix.plugin.manager/Plugins.page(1 hunks)emhttp/plugins/dynamix.plugin.manager/sheets/Plugins.css(0 hunks)emhttp/plugins/dynamix.vm.manager/VMMachines.page(1 hunks)emhttp/plugins/dynamix/ArrayDevices.page(1 hunks)emhttp/plugins/dynamix/BootDevice.page(1 hunks)emhttp/plugins/dynamix/DiskList.page(1 hunks)emhttp/plugins/dynamix/ShareList.page(1 hunks)emhttp/plugins/dynamix/UserList.page(1 hunks)emhttp/plugins/dynamix/include/DefaultPageLayout.php(1 hunks)emhttp/plugins/dynamix/include/DefaultPageLayout/MainContent.php(1 hunks)emhttp/plugins/dynamix/include/DefaultPageLayout/MainContentTabless.php(0 hunks)emhttp/plugins/dynamix/sheets/DashStats.css(2 hunks)emhttp/plugins/dynamix/sheets/UserList.css(1 hunks)emhttp/plugins/dynamix/styles/default-base.css(14 hunks)emhttp/plugins/dynamix/styles/default-dynamix.css(2 hunks)
💤 Files with no reviewable changes (2)
- emhttp/plugins/dynamix.plugin.manager/sheets/Plugins.css
- emhttp/plugins/dynamix/include/DefaultPageLayout/MainContentTabless.php
🔇 Additional comments (38)
emhttp/plugins/dynamix/include/DefaultPageLayout.php (1)
83-83: Great viewport meta tag update for responsive designUpdating the viewport meta tag from fixed width to device-width is a critical first step for responsive web design. This change allows the content to adapt to various screen sizes rather than being fixed at 1300px width.
emhttp/plugins/dynamix/ShareList.page (1)
36-51: Good implementation of scrollable table containerWrapping the table in a
.TableContainerdiv enables horizontal scrolling for responsive behavior while maintaining the table's structure. The table formatting improvements also enhance code readability.emhttp/plugins/dynamix/DiskList.page (1)
18-33: Consistent table container implementationCorrectly implements the same pattern of wrapping tables in
.TableContainerdivs for horizontal scrolling on smaller screens. The consistent approach across files will help maintain a unified responsive experience.emhttp/plugins/dynamix.vm.manager/VMMachines.page (1)
489-505: Appropriate use of wider container for complex tableThe VM machines table contains more columns of data, so using the
.TableContainer--widestclass variant is appropriate here. This maintains consistency with the responsive approach while accommodating the wider content needs.emhttp/plugins/dynamix/styles/default-dynamix.css (2)
467-469: Appropriate removal of fixed margin adjustment for tablesCommenting out this CSS rule removes the hard-coded negative top margin on tables with the
.tablesorter.shiftclass. This change supports the move toward container-based layouts that handle positioning and responsiveness more effectively.
1538-1540: Appropriate removal of fixed margin adjustment in sidebar themeSimilar to the global rule, commenting out this CSS rule for the sidebar theme context improves layout consistency. This aligns with the modern approach of using container elements rather than fixed margins for positioning.
emhttp/plugins/dynamix.plugin.manager/Plugins.page (1)
155-169: Improved table structure with container wrapperThe table has been appropriately wrapped in a
TableContainerdiv and reformatted with proper indentation. This change enables:
- Horizontal scrolling on smaller screens
- Consistent styling through the container class
- Better code maintainability with clean, indented markup
This follows modern web development best practices for responsive table layouts.
emhttp/plugins/dynamix.docker.manager/DockerContainers.page (1)
31-49: Improved table structure with wider container wrapperThe Docker containers table has been wrapped in a
TableContainer--widestdiv, which is appropriate given the multiple columns and potential for wider content. The code is also properly formatted with consistent indentation, improving readability and maintainability.This change aligns with the responsive layout improvements throughout the UI.
emhttp/plugins/dynamix/BootDevice.page (1)
22-44: Improved table structure and PHP conditional formattingThis change includes two good improvements:
- Wrapping the table in a
TableContainerdiv for consistent responsive behavior- Reformatting the PHP conditional to use cleaner, more readable syntax with proper indentation
These changes improve both the frontend rendering and code maintainability.
emhttp/plugins/dynamix/include/DefaultPageLayout/MainContent.php (1)
88-88: Enhanced panel layout with container wrapperThe returned panel HTML is now properly wrapped in a
<div class="Panels">container. This enables:
- Using flex layouts instead of legacy float-based approaches
- Consistent spacing and alignment between panels
- Better responsive behavior across different screen sizes
This is a clean, non-intrusive improvement that maintains the function's behavior while enhancing the structure of its output.
emhttp/plugins/dynamix/sheets/DashStats.css (2)
367-371: Mobile-first 2-column breakpoint
The media query atmin-width: 768pxcorrectly switches to a two-column layout, aligning with the rest of the plugin. This matches your mobile-first approach.
372-376: Wide-screen 3-column breakpoint
Themin-width: 1600pxrule to expand to three columns is appropriate for large displays. Good match with your overall responsive strategy.emhttp/plugins/dynamix/UserList.page (3)
39-42: Approve conditional no-user message
Switching to an HTML<p>for the empty-state message is semantically correct and will pick up your global paragraph styles.
47-52: Verify URL encoding logic
You wrapurlencode()insidehtmlspecialchars(), which may double-encode certain characters. Please confirm that the resulting URL is correct in all cases (e.g., names with spaces or special characters).
58-59: Form buttons integration
Using standard<input type="submit">and<input type="button">is fine, and your CSS will style these appropriately. No changes needed here.emhttp/plugins/dynamix/sheets/UserList.css (4)
1-13: Solid base for.user-list
The flexbox column layout, centered alignment, and size constraints look great. This will give each user card a consistent appearance.
15-29: Scoped.info spanpositioning
Your nested rule for&.info spanis clear, but remember that plain.cssfiles require a preprocessor or PostCSS plugin to handle nesting. Please verify your build pipeline supports the CSS Nesting spec or tools likepostcss-nested.
31-35: Avatar sizing rules
Constrained max-width and fixed height ensure consistent icon display. Good use of responsive units.
38-47: Theme override for sidebar
The.Theme--sidebar .user-listbackground and border overrides with a hover state are well scoped. Nice alignment with the theming system.emhttp/plugins/dynamix/styles/default-base.css (19)
32-36: Use of:has()selector
Thep:has(> link), p:has(> script), p:has(> style)rules rely on the CSS relational pseudo-class, which still has inconsistent browser support. Please verify that your target browsers fully support:has().
439-443: Flex layout fordiv.title
Switching the title block to a flex row with aligned items modernizes the header and pairs nicely with your updated markup.
451-453: Remove top margin for consecutive titles
Suppressing margin on successive.titleelements prevents unwanted gaps and supports stacked headings.
473-475: Mobile-first menu grid
Defining a single-column grid for#menuon mobile is spot-on for a responsive layout.
479-486: Desktop/tablet menu media query
Switching#menuto a two-column layout atmin-width: 768pxmaintains the original behavior on larger screens.
489-492: Flexible.nav-tilesizing
Allowing.nav-tileto grow in height withheight: autoand amin-heightensures consistent tap targets without truncating content.
515-519: Right-aligned nav-tile on desktop
The.nav-tile.rightjustify adjustments look correct and preserve the right-side alignment on larger viewports.
588-593:#displayboxcontainer
Introducing#displayboxas a full-width, padded container will help center content and apply consistent box-sizing.
595-597: Sidebar displaybox override
Adding a left padding for.Theme--sidebar #displayboxsupports sidebar navigation layouts. Looks good.
614-625: Sticky footer adjustments
Usingmargin-top: autoin a flex context and switching to a fixed position at larger breakpoints ensures the footer remains at the bottom without overlaying content.
626-633: Desktop footer media query
Fixed positioning for the footer atmin-width: 768pxis consistent with the header/menu behavior.
1109-1110:span.statusalignment
Switchingdisplay: inline-blockwithmargin-left: automay not always right-align within non-flex containers. Please verify this behaves as intended across different table cell contexts.
1237-1238: Gap for tabs
Adding a gap between.tabschildren improves spacing in the tab list — good use of thegapproperty.
1244-1244: Tab button color reset
Explicitly settingcolor: var(--text-color)ensures your tab buttons inherit the correct text color in all themes.
1259-1259: Reset margin on tab buttons
Definingmargin: 0removes default button margins for a tighter layout. This is necessary given your new flex arrangement.
1286-1293:.Panelscontainer
Introducing the.Panelsflex container with wrapping, row/column gaps, and box-sizing creates a solid foundation for panel layouts.
1295-1300:.Panelbase styling
Your.Panelrules for relative positioning, centering, and transparent background are clean — great scaffolding for panel items.
1301-1309: Nested anchor styling in.Panel
Styling the anchor as a grid with gap and centered content supports flexible icon+text layouts. This is a robust approach for panel links.
1319-1323:.PanelImgconstraints
Limiting.PanelImgto 33px ensures consistent icon sizing across panels. Well scoped.
…nContentTabless for improved variable usage - Replaced direct htmlspecialchars usage with eval to ensure variables in .page titles are correctly processed. - This change enhances the flexibility of title rendering across multiple layout files.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
emhttp/plugins/dynamix/include/DefaultPageLayout/MainContent.php(2 hunks)emhttp/plugins/dynamix/include/DefaultPageLayout/MainContentTabbed.php(1 hunks)emhttp/plugins/dynamix/include/DefaultPageLayout/MainContentTabless.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- emhttp/plugins/dynamix/include/DefaultPageLayout/MainContentTabless.php
- emhttp/plugins/dynamix/include/DefaultPageLayout/MainContent.php
- Changed column-gap to gap in the grid layout to enhance spacing consistency across different screen sizes.
- Eliminated inline style for text alignment in user list divs to allow for more flexible styling through CSS. - This change enhances the overall layout consistency and responsiveness of the user list display.
- Added a comment in span.status.vhshift to indicate that the margin-right adjustment is likely no longer needed, improving code clarity for future reference.
- Changed the class attribute from "content" to "tab-content content" for better semantic clarity and improved styling consistency in the tabbed layout.
- Added a new class for the tabs container to enable horizontal scrolling and set its width to 100%. - Introduced a class for tab content to manage visibility and added padding for forms within it. - Adjusted padding and margin for various elements to improve layout consistency and responsiveness.
- Changed background color of radio buttons to transparent for a cleaner look. - Adjusted opacity of list items for better visibility. - Removed unnecessary CSS rules related to tab layout to simplify the stylesheet and enhance maintainability.
…ed, and MainContentTabless - Introduced a new function `processTitle` to safely handle page titles by replacing PHP variables with their values without using eval. - Updated title handling in MainContent, MainContentTabbed, and MainContentTabless to utilize the new `processTitle` function, enhancing security and maintainability.
- Wrapped input elements in spans for improved visual structure. - This change continues the effort to enhance layout consistency across the plugin.
- Wrapped the devices table in a div for improved layout structure. - This change continues the effort to enhance visual consistency across the plugin.
Good callout. IMO I believe we're okay with leaving the nesting in place. In a couple of these suggestions, it also mentions "The nested CSS syntax [...] requires a CSS preprocessor or recent browser support for CSS nesting" That last part is the part is what we're relying on here. Browser support for CSS Nesting https://caniuse.com/css-nesting – |
- Removed unnecessary 'class="narrow"' from input elements for improved layout consistency. - This change continues the effort to enhance visual structure across the plugin.
- Added missing line breaks in form elements for improved layout consistency. - This change continues the effort to enhance visual structure across the plugin.
- Wrapped URL links in a span for improved layout structure. - Removed unused ManagementAccess.css file as its styles are no longer needed. - This change continues the effort to enhance visual consistency across the plugin.
- Wrapped the devices table in a div for improved layout structure. - This change continues the effort to enhance visual consistency across the plugin.
- Wrapped input elements in a span for improved layout structure. - This change continues the effort to enhance visual consistency across the plugin.
- Wrapped the Docker vDisk size input element in a span for improved layout structure. - This change continues the effort to enhance visual consistency across the plugin.
…eButton.css - Wrapped the table in Browse.page within a div for improved layout structure. - Removed unnecessary margin-top from autoheight class in Browse.css and BrowseButton.css for better visual consistency. - This change continues the effort to enhance layout consistency across the plugin.
…vms.page - Wrapped tables in CPUisol.page, CPUpin.page, and CPUvms.page within divs for improved layout structure. - This change continues the effort to enhance visual consistency across the plugin.
…less.php - Wrapped elements in spans within PageBuilder.php to improve layout structure. - Added a span for layout consistency in MainContentTabless.php. - This change continues the effort to enhance visual consistency across the plugin.
- Updated layout structure in WG0.page and WGX.page by wrapping elements in spans for improved visual consistency. - Adjusted CSS in WG0.css to streamline styles and enhance layout consistency across the plugin. - This change continues the effort to enhance visual structure across the plugin.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
emhttp/plugins/dynamix/styles/default-base.css (4)
147-157: Review global input styling scope
You’ve setpadding: .5rem; width: 100%; margin: 0; box-sizing: border-box;on all text-like inputs and.textarea. While this enhances consistency and responsiveness, it may override form layouts expecting custom widths or spacing. Consider scoping these overrides under a layout-specific container class (e.g.,.DefaultPageLayout input[type="text"]) or documenting that child components should adjust or override these defaults as needed.
160-163: Improve UX for input instructions
The.input-instructionsrule withopacity: .75; font-style: italic;is clear, but consider adding a transition for smoother appearance when focused (paired with the adjacent selector below).
1149-1150: Remove commented-out legacy margin
The commented reference tomargin-right: -9px;likely no longer applies. Consider deleting this dead code to reduce clutter.
1732-1872: Bulk utility classes introduced
This PR adds a large set of helper classes (.hidden,.flex,.gap-*,.justify-*,.items-*, etc.), resembling a utility-first approach. While useful for ad-hoc layouts, consider extracting these into a separate utilities file or namespace (e.g.,.u-flex,.u-gap-1) to avoid global scope pollution and naming conflicts. Document their usage and the decision to avoid a utility framework like Tailwind.emhttp/plugins/dynamix/sheets/WG0.css (1)
38-40: Remove stale commented CSS
The commented block/* i.fa-fw { margin-left: 5px; } */appears obsolete. Consider deleting it to clean up the stylesheet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (6)
emhttp/plugins/dynamix/WG0.page(8 hunks)emhttp/plugins/dynamix/WGX.page(6 hunks)emhttp/plugins/dynamix/include/DefaultPageLayout/MainContentTabless.php(1 hunks)emhttp/plugins/dynamix/include/PageBuilder.php(1 hunks)emhttp/plugins/dynamix/sheets/WG0.css(2 hunks)emhttp/plugins/dynamix/styles/default-base.css(17 hunks)
✅ Files skipped from review due to trivial changes (1)
- emhttp/plugins/dynamix/include/PageBuilder.php
🚧 Files skipped from review as they are similar to previous changes (3)
- emhttp/plugins/dynamix/include/DefaultPageLayout/MainContentTabless.php
- emhttp/plugins/dynamix/WGX.page
- emhttp/plugins/dynamix/WG0.page
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (24)
emhttp/plugins/dynamix/styles/default-base.css (23)
10-14: Confirm removal of body flex layout
The commented-outheight: 100vh; display: flex; flex-direction: column; overflow-y: auto;block on thebodymay help avoid layout shifts but could reintroduce scrolling issues or break full-height layouts elsewhere. Please verify that this change doesn’t cause regressions in pages that rely on a flex-based root layout (e.g., sticky footers or sidebars).
126-128: Addition of.resize-noneutility class
The new.resize-none { resize: none; }rule provides a useful utility for disabling textarea resizing. This aligns with the responsive layout strategy.
165-170: Adjacent focus styling for input instructions
The adjacent sibling selectorinput:focus, textarea:focus, select:focus + .input-instructions { opacity: 1; }is well-targeted. Ensure that all form controls are followed by.input-instructionselements to avoid orphaned selectors.
300-304: Select element full-width adjustments
Settingwidth: 100%; padding: .5rem .8rem .5rem .5rem; margin: 0;on allselectelements improves form consistency but may conflict with inline dropdowns or side-by-side fields. Verify that this doesn’t break layouts in constrained mix-and-match form sections.
364-366: New.slots-amountsizing utility
.slots-amount { width: 7rem; }gives a specific width to slot-count selectors. Confirm that this value accommodates localization or dynamic content lengths.
368-369: Fine-tuneinput.narrowconstraints
Applyingmax-width: 166px;is a reasonable cap for numeric or short text inputs. Ensure this doesn’t truncate longer values unexpectedly (e.g., IP addresses).
483-484: Remove top margin between consecutive titles
The rule.title + .title { margin-top: 0; }ensures tight grouping of titles. This looks good for multi-panel layouts.
496-498: Mobile-first grid for#menu
Switching to a single-column, two-row grid on mobile (grid-template-columns: 1fr; grid-template-rows: auto auto;) enhances vertical stacking. Approved.
502-508: Desktop grid adjustments for#menu
The media query restores the original horizontal layout viagrid-template-columns: auto max-content; grid-template-rows: auto; position: sticky; top: 0;. This aligns with responsive best practices.
512-513: Allow.nav-tileheight to adjust
Changingheight: auto; min-height: 4rem;enables flexible wrapping of navigation tiles. Looks good.
528-531: Enable wrapping for nav tiles
Addingflex-wrap: nowrap; height: 4rem;in the desktop breakpoint preserves the horizontal scroll behavior. Approved.
539-542: Right-align large-screen navigation
The.nav-tile.right { justify-content: flex-end; }tweak keeps the UI consistent on desktop. Approved.
611-616:#displayboxpadding and box-sizing
The new padding andbox-sizing: border-box;ensure consistent spacing and prevent overflow issues. Approved.
631-631: Ensureposition: relative;on#footer
Addingposition: relative;provides a containing block for absolutely positioned children and maintains stacking context. Approved.
637-640: Footer grid layout for mobile
Implementingdisplay: grid; grid-template-columns: 1fr; grid-gap: 1rem; margin-top: auto;creates a stacked footer on narrow screens. Approved.
642-651: Sticky footer on desktop
The desktop breakpoint sets the footer toposition: fixed; bottom: 0; left: 0; justify-content: space-between; grid-template-columns: minmax(0, auto) minmax(0, auto);. Confirm no content gets obscured.
655-660: Responsive.footer-leftflex container
Grouping the left footer items with flex-wrap and gap improves small-screen layout. Approved.
664-667: Responsive.footer-rightflex container
Matching the right footer items to the left styling keeps symmetry. Approved.
670-677: Desktop footer item justification
Switching.footer-lefttojustify-content: flex-startand.footer-righttojustify-content: flex-endaligns with the overall grid. Looks good.
1144-1145: Makespan.statusa flex item
Applyingdisplay: inline-block; margin-left: auto;on.statushelps it right-align within flex containers. Approved.
851-858: Horizontal scrolling for tables
The.TableContainerclass withoverflow-x: auto; -webkit-overflow-scrolling: touch; table { min-width: 1280px; }ensures wide tables scroll gracefully. Approved.
860-867: Viewport-widest table scrolling
Similarly,.TableContainer--widestincreases the min-width threshold to1345pxfor even wider tables. Approved.
1889-1899:.Theme--sidebarnested rule checks
The sidebar-specific overrides (e.g., resettingi.spacingmargins, hidingi.title, adjustingi.favo) leverage CSS nesting. Ensure that the production build supports native CSS nesting or includes a PostCSS nesting plugin; otherwise these nested selectors won’t apply.emhttp/plugins/dynamix/sheets/WG0.css (1)
17-23: Modernize.presetlayout with flex
Replacing floats withdisplay: flex; flex-wrap: wrap; align-items: center; justify-content: space-between;in.presetimproves responsiveness and alignment. Validate that legacy browsers in Unraid’s environment support this flex usage.
…e.css - Added utility classes for width, height, and overflow to improve layout structure in PluginInstall.page. - Removed the obsolete Plugins.css file as its styles are no longer needed. - This change continues the effort to enhance visual consistency across the plugin.
- Removed commented-out CSS rules to clean up the stylesheet. - This change continues the effort to enhance visual consistency across the plugin.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
emhttp/plugins/dynamix/styles/default-base.css (3)
1518-1520: Nested CSS requires build support
You’ve introduced nested rules inside theddselector forp { margin: 0 0 4px 0; }. This depends on a CSS nesting plugin; verify your pipeline handles this or flatten the selector.
1342-1353: Confirm CSS nesting support for.tab-content form
The nestedform { … }block under.tab-contentdepends on CSS nesting. Verify your build system includes a nesting plugin or flatten these rules.
1365-1386: Validate nested selectors in.Panelblock
The nesteda { … }and&:hover .PanelText { … }rules require a CSS nesting plugin. Confirm your toolchain handles this or rewrite to flat selectors (e.g.,.Panel a {}and.Panel:hover .PanelText {}).
🧹 Nitpick comments (8)
emhttp/plugins/dynamix/styles/default-base.css (8)
10-14: Remove commented-out body layout rules
The legacyheight: 100vh,display: flex, andoverflow-y: autodeclarations are commented out on thebodyselector. They should be removed to reduce noise and rely on version control for history.
126-128: Scope or consolidate the.resize-noneutility
Adding.resize-none { resize: none; }is fine, but it may overlap with other utilities. Consider grouping all resize/overflow utilities under a single naming convention or documenting these helpers to avoid duplication.
157-157: Movebox-sizingto a global reset
Applyingbox-sizing: border-box;per-input is effective, but it’s more maintainable to include it in a root-level reset (*, *::before, *::after { box-sizing: border-box; }).
160-163: Improve input instruction visibility
The.input-instructionsutility is a nice addition to guide users. Consider linking it viaaria-describedbyon inputs to improve accessibility rather than relying solely on visual state.
165-170: Use:focus-withinor wrapper classes for instruction toggles
The adjacent sibling selector (input:focus + .input-instructions) works, but if markup changes, it could break. Consider using:focus-withinon a parent container or a dedicated utility class to toggle instruction visibility more robustly.
368-368: Document theinput.narrowutility usage
Addinginput.narrow { max-width: 166px; }is helpful, but consider naming consistency with your other utilities (e.g., prefixing with.u-or grouping under a utility namespace) to avoid conflicts.
1294-1297: Enable horizontal scroll for tab containers
Making.tabs-container { overflow-x: auto; width: 100%; }ensures tabs remain accessible on narrow screens. Consider adding-webkit-overflow-scrolling: touch;for smoother scrolling on iOS.
1732-1901: Audit newly introduced utility classes
A large set of utility classes (e.g.,.hidden,.flex,.gap-4, etc.) has been added. To prevent CSS bloat and naming collisions, consider centralizing them in a dedicated utilities file or adopting a utility-first framework (e.g., Tailwind). Also ensure these classes are documented for developer reference.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
emhttp/plugins/dynamix.plugin.manager/PluginInstall.page(1 hunks)emhttp/plugins/dynamix.plugin.manager/sheets/Plugins.css(0 hunks)emhttp/plugins/dynamix/sheets/WG0.css(1 hunks)emhttp/plugins/dynamix/styles/default-base.css(17 hunks)
💤 Files with no reviewable changes (1)
- emhttp/plugins/dynamix.plugin.manager/sheets/Plugins.css
🚧 Files skipped from review as they are similar to previous changes (2)
- emhttp/plugins/dynamix.plugin.manager/PluginInstall.page
- emhttp/plugins/dynamix/sheets/WG0.css
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
emhttp/plugins/dynamix/styles/default-base.css (12)
28-39: Ensure browser support for the:has()pseudo-class
You’ve added a global rule usingp:has(…), which is powerful but still lacks support in some major browsers (e.g., older Firefox). Consider wrapping it in a feature query or polyfill to avoid dropped rules in unsupported environments:@supports(selector(p:has(> link))) { p:has(> link), p:has(> script), … { margin: 0; } }
147-147: Standardize input padding for touch targets
Settingpadding: .5rem;on form fields improves usability on mobile devices. This aligns well with your responsive layout objectives.
152-153: Enable full-width inputs for responsiveness
Replacing the commented-out fixed width withwidth: 100%;ensures inputs span their container on narrow viewports. This change supports your mobile-first strategy.
300-303: Full-width select styling is responsive-friendly
Addingwidth: 100%; padding: .5rem .8rem .5rem .5rem; margin: 0;to<select>elements ensures they adapt to container widths on small screens. Good for your mobile-first layout.
355-358: Fine-tune narrow and auto select widths
The utilities for.select.narrow { width: 76px; }and.select.auto { width: auto; }give precise control over select sizing. Ensure these classes are documented for developers to apply them correctly.
1499-1505: Leverage CSS Grid for definition lists
Switchingdltodisplay: gridwith responsive columns and gaps enhances readability on all viewports. Modern browsers support this well.
1542-1558: Limit form field widths at larger breakpoints
The media query that setsmax-width: 400px;on various input types and selects keeps forms from growing too wide on desktops. This is a solid UX improvement.
1287-1287: Add consistent gap between tabs
Thegap: .5rem;on.tabscontainer improves spacing between tabs and matches your overall spacing scale.
1356-1363: Approve flex layout for Panels container
Using.Panels { display: flex; flex-wrap: wrap; column-gap: 3rem; row-gap: 2rem; }cleanly handles panel grouping and spacing. Modern browsers support flex gap properties.
2313-2315: Adjust.usage-barsizing for consistency
Changing.usage-bartoposition: relative; width: 6.4rem;standardizes its footprint. Test this width against varied data to ensure the bar never overflows.
611-616: Optimize#displayboxfor responsive layout
Wrapping content in#displayboxwithwidth: 100%; margin: 0 auto; padding: 1rem 1rem 4rem; box-sizing: border-box;provides a solid mobile-first container.
631-641: Review footer grid and positioning
Moving#footerto a grid layout on mobile and fixing it on desktop is good, but verify it doesn’t overlap content on very short viewports. Consider addingbottommargin to your main container.
- Deleted the Apps.css file as its styles are no longer needed. - This change continues the effort to enhance visual consistency across the plugin.
- Updated button and status message structure in DeviceInfo.page for improved layout consistency. - This change continues the effort to enhance visual consistency across the plugin.
…are.page, and CreateDocker.php - Updated button and status message structure in ArrayOperation.page, Disk.page, and Share.page for improved layout consistency. - Adjusted the placement of elements in CreateDocker.php to enhance visual structure. - This change continues the effort to enhance visual consistency across the plugin.
- Adjusted the placement of the control element in DeviceInfo.page to improve layout consistency. - This change continues the effort to enhance visual consistency across the plugin.
- Added a new selector for .ui-dropdownchecklist-selector-wrapper to maintain consistent max-width styling across form elements. - This change continues the effort to enhance visual consistency across the plugin.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
emhttp/plugins/dynamix/styles/default-base.css (1)
464-481: Duplicate: ensure CSS nesting support or flatten nested selectors
Multiple blocks use the CSS nesting syntax (e.g.,&:first-childwithindiv.title, nestedformunder.tab-content, nested rules in.Panels, nested&:emptyunder.content, and numerous nested selectors in.Theme--sidebar). Native CSS nesting is not fully supported across all browsers. Ensure your build pipeline includes a CSS nesting plugin (PostCSS Nesting or Sass) or flatten these rules into standard selectors to guarantee they aren’t dropped.Also applies to: 1272-1280, 1342-1353, 1365-1380, 1913-2066
🧹 Nitpick comments (4)
emhttp/plugins/dynamix/styles/default-base.css (4)
10-14: Remove commented-out code from production CSS
Commented-out rules forbodyheight and flex layout increase file size and reduce readability. If these styles are no longer needed, please remove them entirely or move experimental styles behind a feature flag or separate file.
147-154: Scope global input styling to avoid unintended overrides
Applyingpadding,width: 100%,margin: 0, andbox-sizing: border-boxto all text inputs and textareas can conflict with components that expect fixed dimensions. Consider scoping these rules to a form utility class (e.g.,.DefaultPageLayout input) or movingbox-sizingto the universal selector (*, *::before, *::after) to minimize side effects.Also applies to: 157-157
296-304: Scope global select styling
Similar to the input fields, settingwidth: 100%,padding, andmargin: 0on allselectelements may override intended layouts elsewhere. Consider scoping or using a utility class for full-width selects.
1733-1895: Consider extracting or modularizing utility classes
The file adds many Tailwind-like utilities (.flex,.gap-1,.justify-between, etc.). While powerful, this bloat may be better managed by importing a utility-first library or extracting these into a separate utilities stylesheet to keep the base styles concise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (6)
emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php(2 hunks)emhttp/plugins/dynamix/ArrayOperation.page(2 hunks)emhttp/plugins/dynamix/DeviceInfo.page(17 hunks)emhttp/plugins/dynamix/Disk.page(2 hunks)emhttp/plugins/dynamix/Share.page(1 hunks)emhttp/plugins/dynamix/styles/default-base.css(17 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php
- emhttp/plugins/dynamix/DeviceInfo.page
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (20)
emhttp/plugins/dynamix/ArrayOperation.page (2)
87-87: CSS class simplified for consistencyThe conditional CSS class assignment has been removed from the
ctrlspan element, keeping only the "status" class. This change aligns with the responsive layout improvements being implemented throughout the codebase.
317-317: Improved DOM selection specificityThe jQuery selector has been refined to target
.rightchild elements withindiv.titleelements, rather than appending directly todiv.title. This improves layout consistency and creates a more structured DOM hierarchy.emhttp/plugins/dynamix/Disk.page (2)
35-35: CSS class simplified for consistencyThe conditional CSS class assignment has been removed, using fixed classes 'ctrl2 status' for the navigation controls. This aligns with the responsive layout improvements across the UI.
54-54: Improved DOM selection specificityThe jQuery selector now targets the
.rightchild element of the firstdiv.titleelement, providing more precise control placement within the page structure.emhttp/plugins/dynamix/Share.page (2)
56-56: CSS class simplified for consistencyThe conditional CSS class assignment has been removed, with the span now using fixed classes 'ctrl1 status'. This change is consistent with the standardization of control elements across the UI.
62-62: Improved DOM selection specificityThe jQuery selector now appends to the
.rightchild element of the firstdiv.titleelement, which creates a more consistent structure for layout control elements across different page types.emhttp/plugins/dynamix/styles/default-base.css (14)
126-128: Utility class.resize-nonelooks good
This class provides a simple way to disable resizing on text areas. No issues spotted.
160-170: Approve.input-instructionsbehavior
The adjacent sibling selector to fade in instructions on focus is a good UX pattern. No changes needed here.
354-359: Utility classesselect.narrowandselect.auto
The.narrowand.autovariants allow flexibility in select widths. Implementation is straightforward.
364-366: Approve.slots-amountselect width
This class correctly sizes slot-amount selects to7rem. Good for consistency.
367-369: Approveinput.narrowmax-width
The narrow input variation is implemented as expected.
483-485: Approve sibling-title margin reset
Removing the top margin on consecutive.titleelements helps maintain consistent spacing. No issues here.
496-498: Approve mobile grid layout for#menu
Switching to a single-column stacked grid on small screens is appropriate for responsive design.
503-508: Approve desktop grid layout for#menuin media query
Reverting to the original horizontal layout atmin-width: 768pxlooks correct.
512-513: Approve.nav-tileheight adjustments
Usingheight: autoand amin-heightof4remallows tiles to adapt to content.
529-532: Approve desktop.nav-tileadjustments
Reinstatingflex-wrap: nowrapand a fixed height at larger breakpoints is fine.
611-615: Approve#displayboxcontainer styling
Setting full width, centered margins, padding, andbox-sizing: border-boxmeets layout needs.
631-641: Approve responsive#footerlayout
Defining a grid layout for the footer with a stacked mobile view and fixed positioning plus two columns at larger breakpoints enhances usability.Also applies to: 669-677
655-662: Approve.footer-leftand.footer-rightflex utilities
The wrapping, alignment, and gap settings support responsive footer content.
851-858: Approve.TableContainerand.TableContainer--widest
The scrolling wrappers withmin-widthon nested tables are correctly implemented to enable horizontal scrolling on narrow viewports.Also applies to: 860-867
4 participants
Summary by CodeRabbit
Style
Refactor
Chores