Skip to content
Draft
17 changes: 17 additions & 0 deletions docs/developer/admin-theme.rst
Original file line number Diff line number Diff line change
Expand Up @@ -286,3 +286,20 @@ Similarly, you can customize the HTML of the template by overriding the
``body`` block. See `email_template.html
<https://github.com/openwisp/openwisp-utils/blob/master/openwisp_utils/admin_theme/templates/openwisp_utils/email_template.html>`_
for reference implementation.

Shared NetJSONGraph Assets

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Shared NetJSONGraph Assets
Shared Netjsongraph.js Assets

~~~~~~~~~~~~~~~~~~~~~~~~~~

``openwisp-utils`` provides the unbundled ECharts variant of
``netjsongraph.js`` (and its base CSS themes) so it can be reused across

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of flagging this as code, please link the github repo and call this "the netjsongraph.js library", let's also write that other modules use this library: monitoring (for the geographic and indoor map) and network-topology for the topology graph, let's also add a link to those modules, eg:

:doc:`/monitoring/geo-indoor-maps`   # this should add a link to https://openwisp.io/docs/dev/monitoring/user/geo-indoor-maps.html

:doc:`/network-topology/index`  # this should add a link to https://openwisp.io/docs/dev/network-topology/
```.

different OpenWISP modules without redundantly bundling Leaflet JS.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
different OpenWISP modules without redundantly bundling Leaflet JS.
different OpenWISP modules without redundantly bundling Leaflet JS (which is already shipped and loaded by django-leaflet).


These files are served via the ``openwisp_utils.admin_theme`` app. You can
include them in other OpenWISP modules using the ``static`` template tag:

.. code-block:: html+django

{% load static %}
<link rel="stylesheet" href="{% static 'lib/netjsongraph/css/netjsongraph.css' %}">
<link rel="stylesheet" href="{% static 'lib/netjsongraph/css/netjsongraph-theme.css' %}">
<script src="{% static 'lib/netjsongraph/js/netjsongraph.echarts.min.js' %}"></script>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
.njg-container .leaflet-control-zoom-in,
.njg-container .leaflet-control-zoom-out {
pointer-events: auto;
opacity: 1;
cursor: pointer;
background: rgba(217, 79, 52, 0.85) !important;
color: #fff !important;
Comment on lines +6 to +7

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Document or verify necessity of !important declarations.

Multiple rules use !important to override Leaflet styles. While this might be necessary for overriding external library defaults, consider whether increased specificity could achieve the same result without !important, which makes future overrides more difficult.

If !important is required for Leaflet overrides, consider adding a comment explaining this:

/* Using !important to override Leaflet default styles */
.njg-container .leaflet-control-zoom-in,
.njg-container .leaflet-control-zoom-out {
  background: rgba(217, 79, 52, 0.85) !important;
  color: `#fff` !important;
}

Also applies to: 17-18, 70-71, 81-81

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openwisp_utils/static/netjsongraph/css/netjsongraph-theme.css` around lines 6
- 7, Review the use of !important in the theme rules (e.g., selectors like
.njg-container .leaflet-control-zoom-in and .njg-container
.leaflet-control-zoom-out and the other rules referenced) and either remove the
!important by increasing selector specificity or scope (for example, prefix with
.njg-container and more specific leaf node selectors) so Leaflet defaults are
overridden without !important; if removal isn’t possible because Leaflet injects
inline or higher-specificity styles, keep the declarations but add a short
comment above those blocks stating that !important is required to override
Leaflet default styles and list the affected selectors for future maintainers.

}

.njg-container .leaflet-control-zoom-in.leaflet-disabled,
.njg-container .leaflet-control-zoom-out.leaflet-disabled {
opacity: 0.7;
cursor: default;
}

.njg-container .leaflet-control-layers {
background: rgba(217, 79, 52, 0.85) !important;
color: #fff !important;
}

.njg-container .switch-wrap {
color: #fff;
}

.njg-container .switch-wrap label {
border: 1px solid #dfdfdf;
background: #ffffff;
}

.njg-container .switch-wrap label::before {
box-shadow: 0 1px 3px rgba(0, 0, 0, 1);
background-color: #fff;
}

.njg-container .switch-wrap input[type="checkbox"]:checked + label {
background: grey;
}
Comment on lines +35 to +37

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check WCAG color contrast for grey background with white text

python3 << 'EOF'
def relative_luminance(rgb):
    """Calculate relative luminance for RGB color"""
    rgb = [x / 255.0 for x in rgb]
    rgb = [(x / 12.92 if x <= 0.03928 else ((x + 0.055) / 1.055) ** 2.4) for x in rgb]
    return 0.2126 * rgb[0] + 0.7152 * rgb[1] + 0.0722 * rgb[2]

def contrast_ratio(rgb1, rgb2):
    """Calculate contrast ratio between two colors"""
    l1 = relative_luminance(rgb1)
    l2 = relative_luminance(rgb2)
    lighter = max(l1, l2)
    darker = min(l1, l2)
    return (lighter + 0.05) / (darker + 0.05)

# grey (`#808080`) vs white (`#ffffff`)
grey = [128, 128, 128]
white = [255, 255, 255]
ratio = contrast_ratio(grey, white)

print(f"Contrast ratio: {ratio:.2f}:1")
print(f"WCAG AA (normal text): {'PASS' if ratio >= 4.5 else 'FAIL'}")
print(f"WCAG AA (large text): {'PASS' if ratio >= 3 else 'FAIL'}")
print(f"WCAG AAA (normal text): {'PASS' if ratio >= 7 else 'FAIL'}")
EOF

Repository: openwisp/openwisp-utils

Length of output: 174


🏁 Script executed:

# First, let's read the CSS file to understand the context around line 36
cat -n openwisp_utils/static/netjsongraph/css/netjsongraph-theme.css | head -50

Repository: openwisp/openwisp-utils

Length of output: 1458


🏁 Script executed:

# Check if qa-checks script exists and run it as required by coding guidelines
if [ -f "./run-qa-checks" ]; then
  ./run-qa-checks -- openwisp_utils/static/netjsongraph/css/netjsongraph-theme.css
else
  echo "run-qa-checks script not found"
fi

Repository: openwisp/openwisp-utils

Length of output: 131


🏁 Script executed:

# Verify the proposed fix color (`#555555`) contrast ratio
python3 << 'EOF'
def relative_luminance(rgb):
    """Calculate relative luminance for RGB color"""
    rgb = [x / 255.0 for x in rgb]
    rgb = [(x / 12.92 if x <= 0.03928 else ((x + 0.055) / 1.055) ** 2.4) for x in rgb]
    return 0.2126 * rgb[0] + 0.7152 * rgb[1] + 0.0722 * rgb[2]

def contrast_ratio(rgb1, rgb2):
    """Calculate contrast ratio between two colors"""
    l1 = relative_luminance(rgb1)
    l2 = relative_luminance(rgb2)
    lighter = max(l1, l2)
    darker = min(l1, l2)
    return (lighter + 0.05) / (darker + 0.05)

# Proposed fix: `#555555` vs white (`#ffffff`)
proposed = [85, 85, 85]
white = [255, 255, 255]
ratio = contrast_ratio(proposed, white)

print(f"Proposed color (`#555555`) vs white:")
print(f"Contrast ratio: {ratio:.2f}:1")
print(f"WCAG AA (normal text): {'PASS' if ratio >= 4.5 else 'FAIL'}")
print(f"WCAG AA (large text): {'PASS' if ratio >= 3 else 'FAIL'}")
print(f"WCAG AAA (normal text): {'PASS' if ratio >= 7 else 'FAIL'}")
EOF

Repository: openwisp/openwisp-utils

Length of output: 209


Use a darker background color to meet WCAG AA contrast requirements.

The checked checkbox label uses grey (#808080) as the background with white text, which produces a contrast ratio of 3.95:1. This fails to meet WCAG AA level requirements for normal text (4.5:1 minimum). Change to a darker color such as #555555, which achieves a 7.46:1 contrast ratio and satisfies WCAG AA and AAA standards.

Proposed fix
.njg-container .switch-wrap input[type="checkbox"]:checked + label {
-  background: grey;
+  background: `#555555`;
}
📝 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.

Suggested change
.njg-container .switch-wrap input[type="checkbox"]:checked + label {
background: grey;
}
.njg-container .switch-wrap input[type="checkbox"]:checked + label {
background: `#555555`;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openwisp_utils/static/netjsongraph/css/netjsongraph-theme.css` around lines
35 - 37, The checked checkbox label background uses grey that fails WCAG AA
contrast; update the CSS rule for the selector .njg-container .switch-wrap
input[type="checkbox"]:checked + label to use a darker color (e.g. `#555555`) so
the white text meets at least 4.5:1 contrast (WCAG AA); modify that background
property value accordingly and run a contrast check to confirm compliance.


.njg-container .njg-date {
color: #fff;
}

.njg-container .njg-selectIcon {
background: rgba(217, 79, 52, 0.85);
color: #ffffff;
}

.njg-container .njg-searchBtn {
background-color: white;
}

.njg-container .njg-sideBar {
background-color: #fff;
}

.njg-container .sideBarHandle {
background: rgba(217, 79, 52, 0.85);
}

.njg-container .sideBarHandle::before {
color: #fff;
}

.njg-container .njg-metaData,
.njg-container .njg-infoContainer {
background-color: inherit;
}

.njg-container .njg-tooltip {
background: #fff !important;
border: none !important;
}

.njg-container .njg-tooltip-key,
.njg-container .njg-tooltip-value {
color: #000;
}

.njg-container .marker-cluster div {
color: #fff;
background-color: rgba(21, 102, 169, 0.8) !important;
}
Comment on lines +1 to +82

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Consider extracting repeated colors to CSS custom properties.

The primary theme color rgba(217, 79, 52, 0.85) is repeated 4 times (lines 6, 17, 44, 57). Using CSS custom properties would improve maintainability and make theme updates easier.

♻️ Refactor with CSS custom properties

Add at the top of the file:

.njg-container {
  --theme-primary: rgba(217, 79, 52, 0.85);
  --theme-white: `#ffffff`;
  --theme-grey: `#555555`;
}

Then replace the hardcoded values:

 .njg-container .leaflet-control-zoom-in,
 .njg-container .leaflet-control-zoom-out {
   pointer-events: auto;
   opacity: 1;
   cursor: pointer;
-  background: rgba(217, 79, 52, 0.85) !important;
-  color: `#fff` !important;
+  background: var(--theme-primary) !important;
+  color: var(--theme-white) !important;
 }

Apply similar changes to lines 17, 44, and 57.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openwisp_utils/static/netjsongraph/css/netjsongraph-theme.css` around lines 1
- 82, Extract the repeated theme color into CSS custom properties on the
.njg-container selector and use those variables where the hardcoded rgba(217,
79, 52, 0.85) and repeated whites/greys are used; specifically add properties
like --theme-primary and --theme-white to .njg-container and replace occurrences
in selectors referenced in the diff (.leaflet-control-zoom-in,
.leaflet-control-zoom-out, .leaflet-control-layers, .sideBarHandle,
.njg-selectIcon, etc.) to reference the new variables so future theme changes
only require updating the custom properties.


@media only screen and (max-width: 850px) {
.njg-container .njg-sideBar {
background: rgba(255, 255, 255, 0.96);
color: #000;
}
}
Loading
Loading