Replace hardcoded colors with CSS variables and format CSS files #1164
Conversation
atif09
changed the title
…#442 Problem: Some tests in TestConfigApi assumed a fixed global Template count; which fail when other modules like openwisp-monitoring add new templates. Fix: Updated template-related assertions in test_api.py to compare counts relatively (eg: capture initial_count, assert +1). Fixes openwisp#442 * [tests] Remove unnecessary test settings file openwisp#442 The test_settings override is not required to fix the template counting issue and adds unrelated changes. Removing it to keep the fix minimal and focused. Fixes openwisp#442
📝 WalkthroughWalkthroughThis pull request replaces hard-coded color values with CSS custom properties across six CSS files in openwisp-controller. Affected selectors include form elements, JSON editors, overlays, dialogs, buttons, tables, preformatted text, and other UI components, switching explicit hex/RGBA values to theme variables (e.g., var(--ow-...)). No functional logic, control flow, or public API declarations were modified. Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openwisp_controller/config/static/config/css/lib/jsonschema-ui.css (1)
383-391: Missed hardcoded color value.Line 386 still contains
background-color: white;which should be replaced withvar(--ow-color-white)to align with the PR objective of eliminating hardcoded colors.Proposed fix
.jsoneditor-wrapper div.jsoneditor .modal { position: absolute; z-index: 10; - background-color: white; + background-color: var(--ow-color-white); border: 1px solid var(--ow-color-fg-light); padding-bottom: 10px; width: 340px; margin-left: -215px; }
🤖 Fix all issues with AI agents
In `@openwisp_controller/config/static/config/css/admin.css`:
- Around line 119-127: The hover rule for .djnjc-overlay .close currently sets
the same background as the base .djnjc-overlay .close so there is no visible
change; update the hover to produce a visible effect by either (a) using an
existing token like var(--ow-color-primary-light) (plus a contrasting color
change such as color or border) or (b) applying a visual tweak such as reduced
opacity (e.g. background-color with rgba/opacity) or a slight
transform/box-shadow, or (c) add and define a new token (e.g.
--ow-color-primary-dark) in your theme if you need a darker variant; modify the
.djnjc-overlay .close:hover selector accordingly and ensure contrast remains
accessible.
In `@openwisp_controller/connection/static/connection/css/command-inline.css`:
- Around line 121-134: The hover/focus rules for `#ow-command-confirm-yes` and
`#ow-command-confirm-no` currently reuse the same background variables and provide
no visual change; replace the nonexistent hover vars with existing tokens or
other visual feedback: update the hover/focus selectors for
`#ow-command-confirm-yes` to use a darker/contrasting variable (e.g.
var(--ow-color-fg-dark) or a semi-transparent overlay) or add a subtle
outline/box-shadow and do the same for `#ow-command-confirm-no` (use a darker
danger/contrast token or opacity change) so users get clear hover/focus feedback
without introducing undefined variables.
🧹 Nitpick comments (4)
openwisp_controller/config/static/config/css/lib/advanced-mode.css (4)
112-114: Semantic concern: Using danger color for number values.Similar to the URL hover issue,
--ow-btn-danger-bgis semantically meant for destructive actions. While JSON editors often use reddish colors for numbers, reusing the "danger" variable could cause confusion in theming. If a dedicated--ow-color-numberor similar doesn't exist, consider documenting this choice or using a more neutral variable.
426-431: Consider using an error/warning color for error display.The
.jsoneditor-text-errorselement displays validation errors, but uses--ow-color-fg-lighter(neutral) instead of an error-related variable. While this provides a subtle appearance, using--ow-color-error-bgor similar (if available) would better communicate the error state semantically.
612-617: Semantic concern: Using danger color for selected state.
--ow-btn-danger-bgfor.jsoneditor-selectedbutton background may confuse users, as red typically signals danger. Consider using--ow-color-primaryor a dedicated selection color variable for better semantic clarity.
75-78: Semantic mismatch: Using danger color for link hover state
--ow-btn-danger-bgis reserved for destructive actions (delete buttons, error states, validation errors). Using it for URL hover/focus states creates confusing semantics—users may interpret the red color as an error or warning. URL hover states should use a neutral or primary color variable instead to maintain proper visual hierarchy and UX clarity.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
openwisp_controller/config/static/config/css/admin.cssopenwisp_controller/config/static/config/css/lib/advanced-mode.cssopenwisp_controller/config/static/config/css/lib/jsonschema-ui.cssopenwisp_controller/config/static/import_export/import-openwisp.cssopenwisp_controller/connection/static/connection/css/command-inline.cssopenwisp_controller/subnet_division/static/subnet-division/css/subnet-division.css
⏰ 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). (11)
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
🔇 Additional comments (22)
openwisp_controller/config/static/config/css/lib/jsonschema-ui.css (2)
15-25: LGTM!Border and button background colors are appropriately replaced with semantic CSS variables (
--ow-color-fg-light,--ow-btn-danger-bg).
491-498: LGTM!The inline-related header styling correctly uses semantic variables for text color, background, and borders, providing consistent theming.
openwisp_controller/subnet_division/static/subnet-division/css/subnet-division.css (2)
1-4: LGTM!The readonly input styling correctly uses overlay variables for border and background, replacing the previous rgba values.
5-11: Verify if openwisp-utils provides warning-specific color variables.The class
.help-text-warninguses--ow-color-primary-lightfor its background. While using a primary brand color for warning context may lack semantic clarity, this concern depends on whether openwisp-utils (v1.3) provides dedicated warning or alert color variables. No warning-specific variables are used anywhere in openwisp-controller, so verify with the openwisp-utils documentation whether--ow-color-warning,--ow-color-alert, or similar variables exist and should be used here instead.openwisp_controller/config/static/config/css/admin.css (3)
5-23: LGTM!The preformatted and JSON editor text styling correctly uses semantic variables for foreground/background colors, maintaining the monospace code appearance.
30-33: LGTM!Readonly input styling matches the same pattern used in
subnet-division.css, ensuring consistency across the codebase.
249-279: LGTM!Tab styling correctly uses semantic variables for different states (default, hover, current), providing consistent theming for navigation elements.
openwisp_controller/connection/static/connection/css/command-inline.css (2)
41-52: LGTM!The command overlay and dialog styling appropriately uses semantic variables for backdrop, border, and shadow colors.
101-114: LGTM!The confirmation dialog styling correctly uses semantic variables for background and text colors, with appropriate font-family declaration.
openwisp_controller/config/static/import_export/import-openwisp.css (1)
15-38: The implementation is correct. Using CSS variables within a@media (prefers-color-scheme: dark)block is the recommended pattern for openwisp-utils, as the variables themselves are static light-mode values and are designed to be overridden in prefers-color-scheme media queries. The code correctly forces consistent light-theme colors when the browser requests dark mode, which aligns with the stated intent that dark theme is not yet fully supported in OpenWISP.openwisp_controller/config/static/config/css/lib/advanced-mode.css (12)
84-103: LGTM!Consistent use of
--ow-color-fg-lighterfor highlight and focus states provides visual coherence.
155-162: LGTM!Appropriate use of neutral foreground variables for button focus states.
167-179: LGTM!Good semantic usage—primary color for the container border establishes visual hierarchy, and darker foreground for text ensures readability.
289-299: LGTM!Appropriate variable choices for the popover: dark background with white text ensures good contrast, and overlay variable for shadows is semantically correct.
325-355: LGTM!Arrow border colors correctly match the popover background for visual consistency.
461-472: LGTM!Appropriate use of neutral border and overlay variables for the context menu.
704-714: LGTM!Primary color for the menu bar is semantically appropriate and establishes clear visual hierarchy.
730-740: LGTM!Consistent use of light overlay for interactive states on the dark menu background.
787-803: LGTM!Good contrast choices for the exit button with appropriate hover state feedback.
898-908: LGTM!White background for full-screen mode is appropriate.
915-924: LGTM!White text for menu labels maintains proper contrast against the primary-colored menu bar.
531-542: Formatting changes look acceptable.These multi-line selector reformats appear to be Prettier output. While the line-per-selector style increases line count, it improves readability for complex selectors.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…1162 Replaced all hardcoded color literals (gray, lightgray, green, yellow, white) in advanced-mode.css and jsonschema-ui.css with semantic CSS variables from openwisp-utils for consistent theming support. Changes: - advanced-mode.css: Replaced 14 hardcoded colors with appropriate CSS variables (--ow-color-fg-medium, --ow-color-fg-light, --ow-color-success, --ow-color-warning, --ow-color-white) - jsonschema-ui.css: Replaced 1 hardcoded white with --ow-color-white This completes the color variable migration started in the initial commit. Fixes openwisp#1162
Replaced hardcoded color values with OpenWISP CSS variables to support theming and maintain consistency with the design system. Fixes openwisp#1162
|
4 participants
Checklist
Reference to Existing Issue
Closes #1162 .
Description of Changes
Replaces hardcoded color literals (hex & rgba) in the UI CSS with the project's semantic CSS variables defined in openwisp-utils#516
Screenshot
N/A