Conversation
…izer in DeviceMetricView inside api views to display correct response schema in API documentation
…self.serializer_class
…penwisp#716 Replaces direct use of MonitoringDeviceDetailSerializer with self.serializer_class to improve flexibility, allow overrides, and maintain consistent serializer usage across the view. Fixes openwisp#716
Replaces direct use of MonitoringDeviceDetailSerializer with self.serializer_class to improve flexibility, allow overrides, and maintain consistent serializer usage across the view. Fixes openwisp#716
Replaces direct use of MonitoringDeviceDetailSerializer with self.serializer_class to improve flexibility, allow overrides, and maintain consistent serializer usage across the view. Fixes openwisp#716
…p#716 Replaces direct use of MonitoringDeviceDetailSerializer with self.serializer_class to improve flexibility, allow overrides, and maintain consistent serializer usage across the view. Related to openwisp#716
…p#716 Replaces direct use of MonitoringDeviceDetailSerializer with self.serializer_class to improve flexibility, allow overrides, and maintain consistent serializer usage across the view. Related to openwisp#716
…p#716 Replaces direct use of MonitoringDeviceDetailSerializer with self.serializer_class to improve flexibility, allow overrides, and maintain consistent serializer usage across the view. Related to openwisp#716
- Updated the GitHub Actions `Install Dependencies` step to install Django before the project package in editable mode.
Replaced hardcoded color values in CSS files with the theme color variables defined in openwisp-utils, ensuring consistent styling across the OpenWISP project. Related to openwisp#720
Replaced hardcoded color values in CSS files with the theme color variables defined in openwisp-utils, ensuring consistent styling across the OpenWISP project. Related to openwisp#720
Replaced hardcoded color values in CSS files with the theme color variables defined in openwisp-utils, ensuring consistent styling across the OpenWISP project. Related to openwisp#720
…com:shakurah/openwisp-monitoring into issues/720-theme-colors-from-openwisp-utils
Replaced hardcoded color values in CSS files with the theme color variables defined in openwisp-utils, ensuring consistent styling across the OpenWISP project. Related to openwisp#720
…com:shakurah/openwisp-monitoring into issues/720-theme-colors-from-openwisp-utils
📝 WalkthroughWalkthroughThis PR updates frontend assets and theming for openwisp-monitoring: it replaces references to Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRsSuggested labelsenhancement Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_monitoring/device/static/monitoring/css/daterangepicker.css`:
- Line 141: The CSS rule in daterangepicker.css contains a syntax error: the
border declaration reads `border: 1px solidvar(--ow-color-white)` which will be
ignored; update the declaration (in the rule that contains this border property)
to insert a space between `solid` and `var()` so it becomes `1px solid
var(--ow-color-white)` to restore correct border rendering.
- Line 77: The CSS declaration for the border-top property is malformed: replace
the token "solidvar(--ow-color-white)" with a valid value by inserting a space
so it reads "solid var(--ow-color-white)"; update the border-top declaration
(the line using border-top with var(--ow-color-white)) to ensure the browser
recognizes the rule.
In `@openwisp_monitoring/device/static/monitoring/js/chart.css`:
- Around line 110-116: The CSS selector '#ow-chart-contents .circle > span' has
duplicate font-size declarations; remove the redundant 'font-size: 0.15em;' and
keep the explicit 'font-size: 13px;' so only a single font-size remains (update
the rule for '#ow-chart-contents .circle > span' to remove the earlier font-size
line).
- Around line 56-63: The .chart-heading rule has duplicate font-size
declarations; remove the obsolete font-size: 2em declaration so only font-size:
1.22em remains (update the .chart-heading selector in chart.css to eliminate the
dead/duplicated font-size line).
- Around line 44-55: The background's URL in the .chart-help CSS rule uses an
unquoted URL; update the background declaration in the .chart-help selector to
quote the URL argument (e.g. quote ../../admin/img/icon-unknown.svg) so the
background/shorthand uses background: url("...") 0 0 no-repeat; ensuring the URL
is wrapped in quotes to satisfy stylelint and CSS best practices.
- Around line 120-127: Remove the duplicate .chart-heading CSS rule block by
deleting the redundant declaration (the second .chart-heading block with margin,
font-size, text-align, font-weight, color) so only the original .chart-heading
rule remains; locate the duplicate using the selector .chart-heading in
chart.css and remove the later block to avoid repetition.
In `@openwisp_monitoring/device/static/monitoring/js/device-map.js`:
- Line 13: The deactivated color token is missing the leading hash causing
invalid CSS color parsing: update the deactivated value from "000" to "#000" in
the status color map (the object property named deactivated in device-map.js) so
marker rendering uses a valid hex color; ensure no other status tokens are
missing the "#" prefix.
In `@openwisp_monitoring/device/static/monitoring/js/floorplan.js`:
- Around line 420-421: The label color values currently use CSS variables
directly ("var(--ow-color-white)" and "var(--ow-color-black)") which
ECharts/canvas won't resolve; before building the chart options, read and trim
the computed values from the :root via
getComputedStyle(document.documentElement).getPropertyValue('--ow-color-white')
and '--ow-color-black', store them (e.g. whiteColor, blackColor) and replace
label.color and label.backgroundColor in the options object with those resolved
strings so ECharts receives actual hex/RGB colors.
In `@openwisp_monitoring/monitoring/static/monitoring/css/chart.css`:
- Around line 110-115: The CSS rule for selector "#ow-chart-contents .circle >
span" contains duplicate font-size declarations; remove the redundant
font-size:0.15em (or the 13px one) so only a single font-size remains (prefer
keeping font-size: 13px for clarity), leaving the rest of the properties intact
in the same rule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3618b05-aa93-4485-9ee5-1fd4aac18f9c
⛔ Files ignored due to path filters (3)
openwisp_monitoring/device/static/monitoring/js/lib/netjsongraph.echarts.min.jsis excluded by!**/*.min.jsopenwisp_monitoring/device/static/monitoring/js/lib/netjsongraph.min.jsis excluded by!**/*.min.jsopenwisp_monitoring/monitoring/static/monitoring/js/lib/plotly-cartesian.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (16)
.github/workflows/ci.ymlopenwisp_monitoring/device/admin.pyopenwisp_monitoring/device/apps.pyopenwisp_monitoring/device/static/monitoring/css/daterangepicker.cssopenwisp_monitoring/device/static/monitoring/css/device-map.cssopenwisp_monitoring/device/static/monitoring/css/leaflet.fullscreen.cssopenwisp_monitoring/device/static/monitoring/css/monitoring.cssopenwisp_monitoring/device/static/monitoring/css/netjsongraph.cssopenwisp_monitoring/device/static/monitoring/css/percircle.min.cssopenwisp_monitoring/device/static/monitoring/js/chart.cssopenwisp_monitoring/device/static/monitoring/js/device-map.jsopenwisp_monitoring/device/static/monitoring/js/floorplan.jsopenwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/device/tests/test_apps.pyopenwisp_monitoring/monitoring/static/monitoring/css/chart.cssopenwisp_monitoring/tests/test_selenium.py
📜 Review details
⏰ 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). (9)
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.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.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2026-02-25T17:20:05.287Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/device/templates/admin/map/base_map.html:25-26
Timestamp: 2026-02-25T17:20:05.287Z
Learning: In openwisp-monitoring, base_map.html unconditionally applies the `dedicated-map-page` class to `#device-map-container`, but CSS conflicts are avoided by conditionally loading `monitoring.css`. MapPageAdmin includes `monitoring.css` (which contains `.dedicated-map-page` rules), while the dashboard map does not load `monitoring.css`, so the class has no effect on the dashboard.
Applied to files:
openwisp_monitoring/device/static/monitoring/js/chart.cssopenwisp_monitoring/device/static/monitoring/css/leaflet.fullscreen.cssopenwisp_monitoring/device/static/monitoring/css/device-map.cssopenwisp_monitoring/device/static/monitoring/css/monitoring.cssopenwisp_monitoring/device/admin.pyopenwisp_monitoring/device/apps.pyopenwisp_monitoring/device/static/monitoring/js/device-map.js
📚 Learning: 2026-03-10T22:16:11.815Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 755
File: openwisp_monitoring/device/static/monitoring/js/floorplan.js:45-47
Timestamp: 2026-03-10T22:16:11.815Z
Learning: In openwisp-monitoring PR `#755`, the indoor map URL fragment separator was intentionally changed from ":" to "_" (format: `<locationId>_<floor>`). This is a new feature with no existing saved/bookmarked URLs, so no backward compatibility with the old ":" separator format is required. The `_` separator is the canonical format going forward.
Applied to files:
openwisp_monitoring/device/static/monitoring/css/device-map.cssopenwisp_monitoring/device/admin.pyopenwisp_monitoring/device/static/monitoring/js/floorplan.js
📚 Learning: 2026-02-21T18:44:28.852Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/device/api/views.py:263-281
Timestamp: 2026-02-21T18:44:28.852Z
Learning: In openwisp-monitoring, MonitoringIndoorCoordinatesList inherits organization scoping from the parent IndoorCoordinatesList (from openwisp-controller), which uses FilterByParentManaged mixin and filters by location_id in get_queryset(). The child class only overrides the queryset attribute to add monitoring-specific select_related fields; this pattern is safe as long as get_queryset() from the parent is not bypassed. During reviews, verify that MonitoringIndoorCoordinatesList continues to rely on the parent's get_queryset() and that any added select_related fields in the child do not alter the parent's filtering logic.
Applied to files:
openwisp_monitoring/device/admin.pyopenwisp_monitoring/device/apps.pyopenwisp_monitoring/device/tests/test_apps.pyopenwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/tests/test_selenium.py
📚 Learning: 2026-03-14T18:39:04.626Z
Learnt from: UltraBot05
Repo: openwisp/openwisp-monitoring PR: 766
File: openwisp_monitoring/utils.py:59-68
Timestamp: 2026-03-14T18:39:04.626Z
Learning: In this repository (openwisp/openwisp-monitoring), the project targets Python 3.10–3.13 as defined by CI. Do not flag backports.zoneinfo as a missing dependency; zoneinfo is a built-in module in all supported Python versions. When reviewing Python code, assume zoneinfo is available and avoid suggesting installation of backports.zoneinfo. If a file imports zoneinfo or uses it for time zone handling, treat it as standard library usage compatible with the supported CI matrix.
Applied to files:
openwisp_monitoring/device/admin.pyopenwisp_monitoring/device/apps.pyopenwisp_monitoring/device/tests/test_apps.pyopenwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/tests/test_selenium.py
📚 Learning: 2026-03-14T18:39:04.626Z
Learnt from: UltraBot05
Repo: openwisp/openwisp-monitoring PR: 766
File: openwisp_monitoring/utils.py:59-68
Timestamp: 2026-03-14T18:39:04.626Z
Learning: In the openwisp-monitoring project, targets are Linux-based environments. Do not flag a Windows-specific tzdata package as a missing dependency in code reviews for Python files (e.g., openwisp_monitoring/utils.py). If a platform-specific dependency is truly required, document the exception in review guidelines and ensure CI/packaging checks enforce platform constraints rather than manual review.
Applied to files:
openwisp_monitoring/device/admin.pyopenwisp_monitoring/device/apps.pyopenwisp_monitoring/device/tests/test_apps.pyopenwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/tests/test_selenium.py
📚 Learning: 2026-02-21T18:29:38.883Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/device/static/monitoring/js/floorplan.js:253-258
Timestamp: 2026-02-21T18:29:38.883Z
Learning: In openwisp-monitoring, the `floor` field in the indoor coordinates API is always an integer representing floor numbers, so it doesn't require escaping when used in jQuery attribute selectors.
Applied to files:
openwisp_monitoring/device/static/monitoring/js/floorplan.js
📚 Learning: 2026-02-21T01:03:37.822Z
Learnt from: nemesifier
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/tests/test_selenium.py:827-859
Timestamp: 2026-02-21T01:03:37.822Z
Learning: In Selenium tests (e.g., in openwisp_monitoring/tests/test_selenium.py and similar test files), when testing JS animations on dashboards or elements driven by JavaScript (such as real-time location updates), insert a short sleep (e.g., sleep(0.3)) before WebDriverWait assertions to allow animations to complete and reduce flakiness. Note: use this as a targeted workaround and prefer explicit waits or animation-end checks where possible to avoid relying on fixed delays.
Applied to files:
openwisp_monitoring/device/tests/test_apps.pyopenwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/tests/test_selenium.py
📚 Learning: 2026-02-25T18:42:08.825Z
Learnt from: dee077
Repo: openwisp/openwisp-monitoring PR: 738
File: openwisp_monitoring/tests/test_selenium.py:309-317
Timestamp: 2026-02-25T18:42:08.825Z
Learning: In Selenium tests that use ChannelsLiveServerTestCase (e.g., openwisp_monitoring/tests/test_selenium.py), override settings to configure CHANNEL_LAYERS with channels_redis.core.RedisChannelLayer instead of InMemoryChannelLayer. This is required because the live server runs in a separate process from the test process, and InMemoryChannelLayer is per-process only and cannot handle cross-process WebSocket broadcasting needed for real-time location update tests.
Applied to files:
openwisp_monitoring/device/tests/test_apps.pyopenwisp_monitoring/device/tests/test_admin.pyopenwisp_monitoring/tests/test_selenium.py
🪛 Biome (2.4.7)
openwisp_monitoring/device/static/monitoring/js/chart.css
[error] 60-60: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
(lint/suspicious/noDuplicateProperties)
[error] 114-114: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
(lint/suspicious/noDuplicateProperties)
[error] 124-124: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
(lint/suspicious/noDuplicateProperties)
openwisp_monitoring/monitoring/static/monitoring/css/chart.css
[error] 114-114: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
(lint/suspicious/noDuplicateProperties)
openwisp_monitoring/device/static/monitoring/css/daterangepicker.css
[error] 77-77: Unexpected unknown function: solidvar
(lint/correctness/noUnknownFunction)
[error] 141-141: Unexpected unknown function: solidvar
(lint/correctness/noUnknownFunction)
openwisp_monitoring/device/static/monitoring/css/percircle.min.css
[error] 1-1: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
(lint/suspicious/noDuplicateProperties)
🪛 Stylelint (17.4.0)
openwisp_monitoring/device/static/monitoring/js/chart.css
[error] 58-58: Unexpected duplicate "font-size" (declaration-block-no-duplicate-properties)
(declaration-block-no-duplicate-properties)
[error] 111-111: Unexpected duplicate "font-size" (declaration-block-no-duplicate-properties)
(declaration-block-no-duplicate-properties)
[error] 122-122: Unexpected duplicate "font-size" (declaration-block-no-duplicate-properties)
(declaration-block-no-duplicate-properties)
[error] 49-49: Expected quotes around "url" function argument (function-url-quotes)
(function-url-quotes)
openwisp_monitoring/device/static/monitoring/css/percircle.min.css
[error] 1-1: Unexpected deprecated property "clip" (property-no-deprecated)
(property-no-deprecated)
[error] 1-1: Unexpected deprecated property "clip" (property-no-deprecated)
(property-no-deprecated)
[error] 1-1: Unexpected deprecated property "clip" (property-no-deprecated)
(property-no-deprecated)
🔇 Additional comments (28)
.github/workflows/ci.yml (1)
85-85: Good dependency pinning order.Installing the matrix Django version at Line [85] before
requirements-test.txtis the right move to reduce resolver drift.openwisp_monitoring/device/admin.py (1)
592-592: MapPageAdmin asset swap looks correct.Line [592] correctly switches to the ECharts-only NetJSONGraph bundle.
openwisp_monitoring/device/static/monitoring/css/leaflet.fullscreen.css (1)
2-2: Fullscreen control theming change is solid.Line [2] cleanly replaces hardcoded white with a theme variable and keeps asset reference explicit.
openwisp_monitoring/device/apps.py (1)
382-382: Dashboard template asset update is consistent.Line [382] correctly switches the dashboard map to
netjsongraph.echarts.min.js.openwisp_monitoring/device/tests/test_apps.py (1)
34-34: Test expectation matches the new bundle name.Line [34] is correctly updated to assert
netjsongraph.echarts.min.js.openwisp_monitoring/device/tests/test_admin.py (2)
904-904: LGTM!The test correctly references the new ECharts-only build
netjsongraph.echarts.min.js, aligning with the PR objective to avoid bundling Leaflet inside netjsongraph.js.
1245-1245: LGTM!Consistent update to verify the new
netjsongraph.echarts.min.jsasset is loaded on the map page admin.openwisp_monitoring/device/static/monitoring/css/device-map.css (5)
1-5: LGTM!The CSS variable
var(--ow-color-black)for.health-deactivatedaligns with the project's theming approach using OpenWISP design tokens.
62-67: LGTM!Correct use of
var(--ow-color-white)for the no-data button text color.
234-245: LGTM!The floorplan container styling correctly uses CSS variables for theming (
--ow-overlay-40for shadows,--ow-color-whitefor background).
262-269: LGTM!The floorplan title color correctly uses
var(--ow-color-fg-darker)for consistent theming.
303-310: LGTM!Consistent use of
var(--ow-overlay-40)for the navigation body shadow.openwisp_monitoring/tests/test_selenium.py (5)
809-819: LGTM!Formatting change to improve readability of the multi-line JavaScript in the lambda. No functional change.
833-843: LGTM!Consistent formatting adjustment for the execute_script lambda.
896-907: LGTM!Formatting consistency maintained across similar test patterns.
935-947: LGTM!Formatting consistency maintained.
974-986: LGTM!Formatting consistency maintained across all similar lambda patterns in the test file.
openwisp_monitoring/device/static/monitoring/css/monitoring.css (3)
4-9: LGTM!The
#ow-device-status h2styling correctly uses theme variables for color and background, enabling consistent theming support.
32-35: LGTM!Table header background correctly uses
var(--ow-color-fg-lightest)for theme consistency.
41-43: LGTM!Consistent use of
var(--ow-color-black)for.health-deactivatedtext color, matching the same class indevice-map.css.openwisp_monitoring/device/static/monitoring/css/netjsongraph.css (3)
47-50: LGTM!The marker cluster text color correctly uses
var(--ow-color-white)for theme consistency.
76-79: LGTM!Tooltip background correctly uses
var(--ow-color-white)for consistent theming.
87-101: LGTM!Tooltip key and value text colors consistently use
var(--ow-color-black)for theme support.openwisp_monitoring/device/static/monitoring/css/percircle.min.css (1)
1-1: LGTM!The
.percircle.darkbackground-color correctly usesvar(--ow-color-fg-dark)for theme consistency. The static analysis warnings about duplicate properties and deprecatedclipare pre-existing issues in this minified third-party CSS file, not introduced by this change.openwisp_monitoring/device/static/monitoring/css/daterangepicker.css (4)
4-4: LGTM!Correct use of
var(--ow-color-white)for the daterangepicker background.
35-36: LGTM!Correct use of CSS variable for the border color.
361-379: LGTM!The active date and in-range styling correctly uses
var(--ow-color-primary)andvar(--ow-color-white)for theme consistency.
380-386: LGTM!Button styling correctly uses
var(--ow-color-fg-darker)andvar(--ow-color-fg-dark)for theme consistency.
| bottom: -6px; | ||
| border-bottom: initial; | ||
| border-top: 6px solid #fff; | ||
| border-top: 6px solidvar(--ow-color-white); |
There was a problem hiding this comment.
Fix CSS syntax error: missing space before var().
The declaration border-top: 6px solidvar(--ow-color-white) is malformed. There must be a space between solid and var(). This will cause the rule to be ignored by browsers.
🐛 Proposed fix
- border-top: 6px solidvar(--ow-color-white);
+ border-top: 6px solid var(--ow-color-white);📝 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.
| border-top: 6px solidvar(--ow-color-white); | |
| border-top: 6px solid var(--ow-color-white); |
🧰 Tools
🪛 Biome (2.4.7)
[error] 77-77: Unexpected unknown function: solidvar
(lint/correctness/noUnknownFunction)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/device/static/monitoring/css/daterangepicker.css` at line
77, The CSS declaration for the border-top property is malformed: replace the
token "solidvar(--ow-color-white)" with a valid value by inserting a space so it
reads "solid var(--ow-color-white)"; update the border-top declaration (the line
using border-top with var(--ow-color-white)) to ensure the browser recognizes
the rule.
| } | ||
| .daterangepicker .calendar-table { | ||
| border: 1px solid #fff; | ||
| border: 1px solidvar(--ow-color-white); |
There was a problem hiding this comment.
Fix CSS syntax error: missing space before var().
The declaration border: 1px solidvar(--ow-color-white) is malformed. There must be a space between solid and var(). This will cause the rule to be ignored by browsers.
🐛 Proposed fix
- border: 1px solidvar(--ow-color-white);
+ border: 1px solid var(--ow-color-white);📝 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.
| border: 1px solidvar(--ow-color-white); | |
| border: 1px solid var(--ow-color-white); |
🧰 Tools
🪛 Biome (2.4.7)
[error] 141-141: Unexpected unknown function: solidvar
(lint/correctness/noUnknownFunction)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/device/static/monitoring/css/daterangepicker.css` at line
141, The CSS rule in daterangepicker.css contains a syntax error: the border
declaration reads `border: 1px solidvar(--ow-color-white)` which will be
ignored; update the declaration (in the rule that contains this border property)
to insert a space between `solid` and `var()` so it becomes `1px solid
var(--ow-color-white)` to restore correct border rendering.
| .chart-help { | ||
| display: inline-block; | ||
| width: 22px; | ||
| height: 21px; | ||
| text-indent: -9999px; | ||
| background: url(../../admin/img/icon-unknown.svg) 0 0 no-repeat; | ||
| background-size: 80%; | ||
| background-position: 50% 50%; | ||
| opacity: 0.6; | ||
| cursor: pointer; | ||
| margin-left: 3px; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add quotes around URL function argument.
Per CSS best practices and stylelint rules, URL function arguments should be quoted.
♻️ Proposed fix
.chart-help {
display: inline-block;
width: 22px;
height: 21px;
text-indent: -9999px;
- background: url(../../admin/img/icon-unknown.svg) 0 0 no-repeat;
+ background: url("../../admin/img/icon-unknown.svg") 0 0 no-repeat;
background-size: 80%;
background-position: 50% 50%;
opacity: 0.6;
cursor: pointer;
margin-left: 3px;
}📝 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.
| .chart-help { | |
| display: inline-block; | |
| width: 22px; | |
| height: 21px; | |
| text-indent: -9999px; | |
| background: url(../../admin/img/icon-unknown.svg) 0 0 no-repeat; | |
| background-size: 80%; | |
| background-position: 50% 50%; | |
| opacity: 0.6; | |
| cursor: pointer; | |
| margin-left: 3px; | |
| } | |
| .chart-help { | |
| display: inline-block; | |
| width: 22px; | |
| height: 21px; | |
| text-indent: -9999px; | |
| background: url("../../admin/img/icon-unknown.svg") 0 0 no-repeat; | |
| background-size: 80%; | |
| background-position: 50% 50%; | |
| opacity: 0.6; | |
| cursor: pointer; | |
| margin-left: 3px; | |
| } |
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 49-49: Expected quotes around "url" function argument (function-url-quotes)
(function-url-quotes)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/device/static/monitoring/js/chart.css` around lines 44 -
55, The background's URL in the .chart-help CSS rule uses an unquoted URL;
update the background declaration in the .chart-help selector to quote the URL
argument (e.g. quote ../../admin/img/icon-unknown.svg) so the
background/shorthand uses background: url("...") 0 0 no-repeat; ensuring the URL
is wrapped in quotes to satisfy stylelint and CSS best practices.
| .chart-heading { | ||
| margin: 0; | ||
| font-size: 2em; | ||
| text-align: center; | ||
| font-size: 1.22em; | ||
| font-weight: normal; | ||
| color: #434b62; | ||
| } |
There was a problem hiding this comment.
Remove duplicate font-size declaration.
The font-size property is declared twice (lines 58 and 60). Only the last value (1.22em) takes effect, making the font-size: 2em declaration dead code.
🐛 Proposed fix
.chart-heading {
margin: 0;
- font-size: 2em;
text-align: center;
font-size: 1.22em;
font-weight: normal;
color: `#434b62`;
}📝 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.
| .chart-heading { | |
| margin: 0; | |
| font-size: 2em; | |
| text-align: center; | |
| font-size: 1.22em; | |
| font-weight: normal; | |
| color: #434b62; | |
| } | |
| .chart-heading { | |
| margin: 0; | |
| text-align: center; | |
| font-size: 1.22em; | |
| font-weight: normal; | |
| color: `#434b62`; | |
| } |
🧰 Tools
🪛 Biome (2.4.7)
[error] 60-60: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
(lint/suspicious/noDuplicateProperties)
🪛 Stylelint (17.4.0)
[error] 58-58: Unexpected duplicate "font-size" (declaration-block-no-duplicate-properties)
(declaration-block-no-duplicate-properties)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/device/static/monitoring/js/chart.css` around lines 56 -
63, The .chart-heading rule has duplicate font-size declarations; remove the
obsolete font-size: 2em declaration so only font-size: 1.22em remains (update
the .chart-heading selector in chart.css to eliminate the dead/duplicated
font-size line).
| #ow-chart-contents .circle > span { | ||
| font-size: 0.15em; | ||
| word-spacing: -0.75px; | ||
| color: var(--ow-overlay-60) !important; | ||
| font-size: 13px; | ||
| font-weight: bold; | ||
| } |
There was a problem hiding this comment.
Remove duplicate font-size declaration.
The font-size property is declared twice (lines 111 and 114). Only the last value (13px) takes effect.
🐛 Proposed fix
`#ow-chart-contents` .circle > span {
- font-size: 0.15em;
word-spacing: -0.75px;
color: var(--ow-overlay-60) !important;
font-size: 13px;
font-weight: bold;
}📝 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.
| #ow-chart-contents .circle > span { | |
| font-size: 0.15em; | |
| word-spacing: -0.75px; | |
| color: var(--ow-overlay-60) !important; | |
| font-size: 13px; | |
| font-weight: bold; | |
| } | |
| `#ow-chart-contents` .circle > span { | |
| word-spacing: -0.75px; | |
| color: var(--ow-overlay-60) !important; | |
| font-size: 13px; | |
| font-weight: bold; | |
| } |
🧰 Tools
🪛 Biome (2.4.7)
[error] 114-114: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
(lint/suspicious/noDuplicateProperties)
🪛 Stylelint (17.4.0)
[error] 111-111: Unexpected duplicate "font-size" (declaration-block-no-duplicate-properties)
(declaration-block-no-duplicate-properties)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/device/static/monitoring/js/chart.css` around lines 110 -
116, The CSS selector '#ow-chart-contents .circle > span' has duplicate
font-size declarations; remove the redundant 'font-size: 0.15em;' and keep the
explicit 'font-size: 13px;' so only a single font-size remains (update the rule
for '#ow-chart-contents .circle > span' to remove the earlier font-size line).
| .chart-heading { | ||
| margin: 0; | ||
| font-size: 2em; | ||
| text-align: center; | ||
| font-size: 1.22em; | ||
| font-weight: normal; | ||
| color: #434b62; | ||
| } |
There was a problem hiding this comment.
Remove duplicate .chart-heading rule block.
This entire rule block (lines 120-127) duplicates the .chart-heading rule at lines 56-63 with identical properties. This is redundant and should be removed.
🗑️ Proposed fix - remove duplicate block
- .chart-heading {
- margin: 0;
- font-size: 2em;
- text-align: center;
- font-size: 1.22em;
- font-weight: normal;
- color: `#434b62`;
- }📝 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.
| .chart-heading { | |
| margin: 0; | |
| font-size: 2em; | |
| text-align: center; | |
| font-size: 1.22em; | |
| font-weight: normal; | |
| color: #434b62; | |
| } |
🧰 Tools
🪛 Biome (2.4.7)
[error] 124-124: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
(lint/suspicious/noDuplicateProperties)
🪛 Stylelint (17.4.0)
[error] 122-122: Unexpected duplicate "font-size" (declaration-block-no-duplicate-properties)
(declaration-block-no-duplicate-properties)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/device/static/monitoring/js/chart.css` around lines 120 -
127, Remove the duplicate .chart-heading CSS rule block by deleting the
redundant declaration (the second .chart-heading block with margin, font-size,
text-align, font-weight, color) so only the original .chart-heading rule
remains; locate the duplicate using the selector .chart-heading in chart.css and
remove the later block to avoid repetition.
| critical: "#a72d1d", | ||
| unknown: "#353c44", | ||
| deactivated: "#000000", | ||
| deactivated: "000", |
There was a problem hiding this comment.
Fix invalid color token for deactivated status.
At Line [13], "000" should be "#000". Without #, color parsing is unreliable and deactivated markers can render incorrectly.
🐛 Proposed fix
- deactivated: "000",
+ deactivated: "#000",📝 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.
| deactivated: "000", | |
| deactivated: "#000", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/device/static/monitoring/js/device-map.js` at line 13,
The deactivated color token is missing the leading hash causing invalid CSS
color parsing: update the deactivated value from "000" to "#000" in the status
color map (the object property named deactivated in device-map.js) so marker
rendering uses a valid hex color; ensure no other status tokens are missing the
"#" prefix.
| color: "var(--ow-color-white)", | ||
| backgroundColor: "var(--ow-color-black)", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Apache ECharts support CSS custom properties like var(--ow-color-white)andvar(--ow-color-black)directly inlabel.colorandlabel.backgroundColor options (especially for canvas rendering)?
💡 Result:
No—not for canvas rendering.
Apache ECharts passes label.color, label.backgroundColor, etc. down to the underlying canvas drawing APIs, and var(--...) CSS custom properties are not resolved there, so ECharts won’t interpret var(--ow-color-white) / var(--ow-color-black) as actual colors. The ECharts team has explicitly marked CSS-variable color support as “not planned”, noting the underlying limitation and recommending resolving CSS variables yourself (e.g., via getComputedStyle(...).getPropertyValue(...)) before calling setOption. [1]
This aligns with the broader browser limitation that canvas color strings generally don’t support var(--...) (they expect a concrete computed color like #fff, rgb(...), etc.). [2]
Workaround: read the computed value of your CSS custom property in JS and feed the resolved color string into ECharts options. [1]
Sources:
[1] Apache ECharts issue “Support on CSS Variables for color property on charts #19743” (and linked maintainer guidance) apache/echarts#19743 (see also the referenced summary of the maintainer comment)
[2] Stack Overflow: canvas fillStyle and CSS variables var(--...) support https://stackoverflow.com/questions/71368314/can-i-set-a-canvas-fillstyle-with-a-variable-from-css
Resolve CSS custom properties in ECharts label colors before passing to options.
Lines 420–421 use var(--ow-color-white) and var(--ow-color-black) in label.color and label.backgroundColor. Apache ECharts does not resolve CSS custom properties in these options; the underlying canvas drawing APIs do not support var(--...) syntax. Labels will fall back to unexpected colors or fail to render correctly.
Compute the CSS variable values in JavaScript using getComputedStyle() and pass the resolved hex/rgb colors to ECharts instead:
const root = document.documentElement;
const whiteColor = getComputedStyle(root).getPropertyValue('--ow-color-white').trim();
const blackColor = getComputedStyle(root).getPropertyValue('--ow-color-black').trim();
// Then use whiteColor and blackColor in the option object🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/device/static/monitoring/js/floorplan.js` around lines
420 - 421, The label color values currently use CSS variables directly
("var(--ow-color-white)" and "var(--ow-color-black)") which ECharts/canvas won't
resolve; before building the chart options, read and trim the computed values
from the :root via
getComputedStyle(document.documentElement).getPropertyValue('--ow-color-white')
and '--ow-color-black', store them (e.g. whiteColor, blackColor) and replace
label.color and label.backgroundColor in the options object with those resolved
strings so ECharts receives actual hex/RGB colors.
| #ow-chart-contents .circle > span { | ||
| font-size: 0.15em; | ||
| word-spacing: -0.75px; | ||
| color: rgba(0, 0, 0, 0.6) !important; | ||
| color: var(--ow-overlay-60) !important; | ||
| font-size: 13px; | ||
| font-weight: bold; |
There was a problem hiding this comment.
Remove duplicate font-size declaration in the same rule.
font-size is declared twice (Lines [111] and [114]); keep only one to avoid ambiguous overrides and lint errors.
♻️ Proposed fix
`#ow-chart-contents` .circle > span {
- font-size: 0.15em;
word-spacing: -0.75px;
color: var(--ow-overlay-60) !important;
font-size: 13px;
font-weight: bold;
}📝 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.
| #ow-chart-contents .circle > span { | |
| font-size: 0.15em; | |
| word-spacing: -0.75px; | |
| color: rgba(0, 0, 0, 0.6) !important; | |
| color: var(--ow-overlay-60) !important; | |
| font-size: 13px; | |
| font-weight: bold; | |
| `#ow-chart-contents` .circle > span { | |
| word-spacing: -0.75px; | |
| color: var(--ow-overlay-60) !important; | |
| font-size: 13px; | |
| font-weight: bold; |
🧰 Tools
🪛 Biome (2.4.7)
[error] 114-114: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
(lint/suspicious/noDuplicateProperties)
🪛 Stylelint (17.4.0)
[error] 111-111: Unexpected duplicate "font-size" (declaration-block-no-duplicate-properties)
(declaration-block-no-duplicate-properties)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openwisp_monitoring/monitoring/static/monitoring/css/chart.css` around lines
110 - 115, The CSS rule for selector "#ow-chart-contents .circle > span"
contains duplicate font-size declarations; remove the redundant font-size:0.15em
(or the 13px one) so only a single font-size remains (prefer keeping font-size:
13px for clarity), leaving the rest of the properties intact in the same rule.
Test Failures in Selenium TestsHello @shakurah, There are test failures related to Selenium tests. Specifically, the Failure: The logs show multiple retries for the Analysis: This indicates that the Leaflet library, which is crucial for the map functionality, is not being loaded or is not available when the JavaScript code attempts to execute. This could be due to:
Recommendation:
The summary of slow tests indicates that many tests are exceeding the 0.3-second threshold, which might be a contributing factor to the timeouts. While not directly causing the Leaflet error, optimizing these slow tests could improve overall test stability. |
The dashboard map was loading netjsongraph.min.js which bundled both ECharts and Leaflet, while django-leaflet was already loading Leaflet separately on device/location detail pages. Switched to the ECharts-only build of netjsongraph.js and load Leaflet JS from django-leaflet instead, avoiding bundling it twice and reducing the page load size. Closes openwisp#709
shakurah
force-pushed
the
issues/709-reuse-django-leaflet
branch
from
52b6368 to
a232a42
Compare
1 participant
Checklist
Reference to Existing Issue
Closes #709 .
Description of Changes
The dashboard map was loading netjsongraph.min.js which bundled both ECharts and Leaflet together, while django-leaflet was already loading Leaflet separately on device/location detail pages — resulting in Leaflet being loaded twice.
This change switches the dashboard map to use the ECharts-only build of netjsongraph.js (netjsongraph.echarts.min.js) and loads Leaflet JS directly from django-leaflet, bringing it in line with how device/location detail pages already handle Leaflet and allowing the browser to reuse the cached Leaflet file across pages.
Files changed:
openwisp_monitoring/device/admin.py— added leaflet/leaflet.js and switched to echarts-only bundleopenwisp_monitoring/device/apps.py— same as aboveopenwisp_monitoring/device/tests/test_admin.py— updated assertions to reflect new JS loading orderopenwisp_monitoring/device/tests/test_apps.py— same as above