Repository navigation
Skin ApexCharts from the active Cacti theme's rrdtheme colors - #53
Merged
Merged
Conversation
The By Method/By Table charts and the import upload donut hard-coded a single light palette and a two-theme light/dark switch that mislabeled several core themes (e.g. paper-plane and deepness are dark but rendered with light text/grid). Derive the ApexCharts foreColor, gridline color and light/dark mode from the active theme's own include/themes/<theme>/rrdtheme.php - mirroring Cacti core's rrdtheme loading, honoring the CactiColorMode cookie's dark/light variants, and inferring mode from the theme font luminance so every current and future theme is covered.
TheWitness
requested review from
bmfmancini and
xmacan
and
a balanced review from Copilot
October 8, 2026 20:15
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Theme inclusion should fail fast, and the new theme-resolution branches need unit coverage.
2 open findings
What changed in this PR
Updates ApexCharts to inherit colors from the active Cacti theme.
Changes:
- Adds theme color resolution and normalization helpers.
- Applies theme-derived foreground and grid colors across charts.
- Infers ApexCharts light/dark mode from font luminance.
| File | Description |
|---|---|
slowlog.php |
Applies resolved theme colors to bar, box, and donut charts. |
includes/slowlog_functions.php |
Adds theme loading, variant selection, and color normalization. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Address review: use require() with a cacti_path_is_within() containment guard (defense-in-depth on top of cacti_validate_theme) so a poisoned theme name can never steer the include outside include/themes - closing a CVE-class path-traversal vector - and fail fast instead of silently using default colors. Add tests/Unit/ApexThemeTest.php covering 3/6/8-digit and invalid color normalization, luminance-based light/dark mode, CactiColorMode dark/light variant selection, and the missing-file fallback; stub get_selected_theme() in the unit bootstrap. Fixes the 100% changed-line coverage gate.
Match the Cacti plugin naming convention. Updates the require in slowlog.php/import_log.php, the Pest coverage <source> list in phpunit.xml, every TestCase::loadPluginSource()/source-scan reference across the test suite, and the .github/copilot-instructions.md tree. Adds an includes/slowlog_functions.php tombstone to manifest.json so slowlog_prune_files() removes the old file from existing installs on upgrade (subdirectory tombstones are supported - the prune joins the relative path under the plugin dir and realpath-confines it).
bmfmancini
approved these changes
Oct 8, 2026
bmfmancini
approved these changes
Oct 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The ApexCharts views (By Method / By Table bar + box charts, and the import
upload-progress donut) were skinned with a single hard-coded light palette and a
two-case
light/darktheme switch. That switch only recognized a handful ofthemes and mislabeled several of the core themes it did list — e.g.
paper-planeanddeepnessare dark themes but were rendered with dark textand light gridlines, so chart labels were effectively invisible.
This change drives the chart surface colors from the active Cacti theme's own
include/themes/<theme>/rrdtheme.php, the same source core uses for RRDtoolgraph theming, so the charts match whatever theme (and dark/light color mode)
the user has selected.
Details
slowlog_apex_theme()helper validates the selected theme against thethemes allowlist (
cacti_validate_theme()when available), includes thattheme's
rrdtheme.php, and honors theCactiColorModecookie'srrdcolors_dark/rrdcolors_lightvariants when a theme defines them —mirroring
lib/rrd.php.light/darkmode is inferred from the luminance of thetheme's own
fontcolor, so all 11 current core themes (and any future ones)are covered without a hand-maintained theme-name list.
fontandgridcolors are normalized (RRDtool alpha suffixes / shorthandstripped) and applied as the chart
foreColorand gridlineborderColorforthe bar charts, box charts, and the donut (track + value label).
#e0e0e0donut track and the brittleswitch($selected_theme)mode mapping.No behavioral change to data, imports, or chart contents — colors only.
Testing
php -lclean on both modified files.