Skip to content

Add theme-appropriate colors for QuickTree graph action glyphs - #34

Merged
TheWitness merged 2 commits into
mainfrom
feature/glyph-theme-colors
Oct 5, 2026
Merged

TheWitness merged 2 commits into
mainfrom
feature/glyph-theme-colors

Conversation

@TheWitness

Copy link
Copy Markdown
Member

Description

QuickTree renders two action glyphs:

  • Add this graph to QuickTree — printed on the graph view page via the graph_buttons / graph_buttons_thumbnails hooks (quicktree_graph_buttons() in setup.php).
  • Remove This Graph From QuickTree — printed on quicktree.php, plus the matching legend icons in the Information/Directions box.

These glyphs reused Cacti's core deviceUp / deviceDown classes, so their color was entirely dependent on core and QuickTree had no say in it. This gives QuickTree its own glyph classes with theme-appropriate colors, mirroring the approach used in plugin_thold.

Changes

  • New glyph classes .quicktreeAdd (green, "add") and .quicktreeRemove (red, "remove") replace the use of core deviceUp / deviceDown in all four markup locations (setup.php graph button, the quicktree.php remove link, and the two legend icons).
  • Default colors for both classes are added to the base css/quicktree.css so any theme (including custom themes that ship no QuickTree stylesheet) still gets a sensible color.
  • Per-theme stylesheets are added for each packaged Cacti theme:
    • Light (classic, modern, paw): add #2e7d32, remove #c62828.
    • Dark (dark, midwinter, sunrise, paper-plane): add #4caf50, remove #e06666 — brightened so they read against the dark backgrounds (sunrise and paper-plane are dark themes despite their names).
  • quicktree_page_head() now loads the base stylesheet and the selected theme's stylesheet (when present) on every page, not just quicktree.php. This is required because the add glyph is rendered on the graph view page, where the stylesheet was previously not loaded.

Related Issue

Motivation and Context

The add/remove glyphs borrowed their color from core and were not owned or tunable by QuickTree. This gives both glyphs an intentional, theme-matched color in every packaged theme (and a graceful default elsewhere), consistent with how plugin_thold themes its glyphs.

Because the glyphs are rendered by the plugin itself and the matching stylesheet is loaded by quicktree_page_head() via get_selected_theme(), the change works the same on both Cacti 1.2.x and the develop branch.

How Has This Been Tested?

  • php -l passes on setup.php and quicktree.php.
  • Verified no deviceUp / deviceDown references remain and that all four glyph sites now use .quicktreeAdd / .quicktreeRemove.
  • Confirmed every stylesheet's CSS braces balance and that each theme file defines the two classes.
  • Cross-checked light vs. dark classification against each theme's body background in Cacti core (include/themes/*/main.css).

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation repository accordingly.

The add-to-QuickTree and remove-from-QuickTree action glyphs reused the core deviceUp/deviceDown classes and were only styled via core. Give QuickTree its own .quicktreeAdd/.quicktreeRemove glyph classes with default colors in the base stylesheet and per-theme overrides for each packaged Cacti theme (classic, modern, paw as light; dark, midwinter, sunrise, paper-plane brightened for dark backgrounds). Load the base and theme stylesheets on every page via page_head so the glyph colors also apply on the graph view page where the add glyph is rendered.
Copilot AI balanced review requested due to automatic review settings October 5, 2026 12:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The behavior needs automated coverage and a required changelog entry.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Adds theme-aware colors for QuickTree’s add/remove graph glyphs.

Changes:

  • Replaces core glyph classes with QuickTree-specific classes.
  • Loads base and selected-theme stylesheets globally.
  • Adds light and dark theme color variants.
File Description
setup.php Updates add glyph and stylesheet loading.
quicktree.php Updates remove and legend glyph classes.
css/​quicktree.css Adds default glyph colors.
css/​classic.css Adds light-theme colors.
css/​modern.css Adds light-theme colors.
css/​paw.css Adds light-theme colors.
css/​dark.css Adds dark-theme colors.
css/​midwinter.css Adds dark-theme colors.
css/​sunrise.css Adds dark-theme colors.
css/​paper-plane.css Adds dark-theme colors.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread setup.php
xmacan
xmacan previously approved these changes Oct 5, 2026
The theme-glyph change touched quicktree_graph_buttons() (setup.php:219) and quicktree_page_head() (setup.php:265-268), which the patch-coverage gate flagged as uncovered. Add QuicktreeGraphButtonsTest and QuicktreePageHeadTest, plus get_md5_include_js/get_md5_include_css/get_selected_theme stubs in the unit bootstrap, to exercise both hooks.
@TheWitness
TheWitness merged commit 759f0d7 into main Oct 5, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants