Repository navigation
Add theme-appropriate colors for graph-view threshold glyphs - #841
Merged
Merged
Conversation
The toggle-threshold-VRULEs and create-threshold action glyphs rendered on the graph view page had no color defined in any theme. Add a theme-appropriate color for each (.tholdVRules, .tholdEdit) across all packaged themes, and brighten the existing .tholdGlyph* action colors on the dark themes (dark, midwinter, sunrise, paper-plane) where the original values were too dark to read against the dark backgrounds.
xmacan
approved these changes
Oct 5, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The duplicated acknowledge selectors leave resume-action glyphs without their intended dark-theme color.
Review effort: Balanced
Findings: 4
Open (4)
What changed in this PR
Adds theme-specific colors for graph-view threshold actions and improves glyph contrast.
Changes:
- Adds colors for VRULE and create-threshold glyphs across all themes.
- Brightens threshold action glyphs in dark themes.
| File | Description |
|---|---|
| css/classic.css | Adds light-theme graph action colors. |
| css/modern.css | Adds light-theme graph action colors. |
| css/paw.css | Adds light-theme graph action colors. |
| css/dark.css | Adds and brightens dark-theme glyph colors. |
| css/midwinter.css | Adds and brightens dark-theme glyph colors. |
| css/sunrise.css | Adds and brightens dark-theme glyph colors. |
| css/paper-plane.css | Adds and brightens dark-theme glyph colors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| .tholdGlyphAcknowledge { | ||
| color: #336600; | ||
| color: #7cb342; |
|
|
||
| .tholdGlyphAcknowledge { | ||
| color: #336600; | ||
| color: #7cb342; |
|
|
||
| .tholdGlyphAcknowledge { | ||
| color: #336600; | ||
| color: #7cb342; |
|
|
||
| .tholdGlyphAcknowledge { | ||
| color: #336600; | ||
| color: #7cb342; |
TheWitness
added a commit
that referenced
this pull request
Oct 5, 2026
Each theme stylesheet carried a duplicated `.tholdGlyphAcknowledge` selector. thold_graph.php emits three distinct acknowledge action glyphs - tholdGlyphAcknowledge, tholdGlyphAcknowledgeSuspend, and tholdGlyphAcknowledgeResume (the "Resume Notifications" action) - so the second copy was clearly meant to target the Resume glyph, which was left unstyled. Rename the duplicate to .tholdGlyphAcknowledgeResume so the resume action receives its intended color in every theme. Remediates the GitHub Copilot review findings on PR #841 (dark, midwinter, sunrise, paper-plane) and fixes the identical latent duplicate in the light themes (classic, modern, paw).
TheWitness
added a commit
that referenced
this pull request
Oct 5, 2026
…elector) (#842) * thold: style tholdGlyphAcknowledgeResume glyph in all themes Each theme stylesheet carried a duplicated `.tholdGlyphAcknowledge` selector. thold_graph.php emits three distinct acknowledge action glyphs - tholdGlyphAcknowledge, tholdGlyphAcknowledgeSuspend, and tholdGlyphAcknowledgeResume (the "Resume Notifications" action) - so the second copy was clearly meant to target the Resume glyph, which was left unstyled. Rename the duplicate to .tholdGlyphAcknowledgeResume so the resume action receives its intended color in every theme. Remediates the GitHub Copilot review findings on PR #841 (dark, midwinter, sunrise, paper-plane) and fixes the identical latent duplicate in the light themes (classic, modern, paw). * thold: style tholdGlyphAcknowledgeResume glyph in light themes Apply the same duplicate-selector fix to the light themes (paper-plane, classic, modern, paw): rename the second .tholdGlyphAcknowledge copy to .tholdGlyphAcknowledgeResume so the "Resume Notifications" graph-view glyph receives its intended color instead of falling through unstyled.
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.
Description
When viewing a graph, thold injects two action glyphs through the
graph_buttons/graph_buttons_thumbnailshooks (seethold_graph_button()insetup.php):<i class="tholdVRules far fa-chart-bar"><i class="tholdEdit fas fa-wrench">Neither
.tholdVRulesnor.tholdEdithad a color rule in any of the packaged theme stylesheets, so the icons rendered in the inherited default color rather than a deliberate, theme-matched one. This adds a theme-appropriate color for both glyphs to every packaged theme.While doing so, the pre-existing
.tholdGlyph*action colors (used on the thresholds list inthold_graph.php) were found to use a single dark palette (#666666,#990000,#006600, …) across all themes. Those values are too dark to read on the dark themes, so they were brightened for the dark themes only; the light themes keep their existing values.Changes per theme
classic,modern,paw): add.tholdVRules(#1b6ca8) and.tholdEdit(#666666, matching the existing edit glyph). Existing glyph colors left unchanged — they already read well on light backgrounds.dark,midwinter,sunrise,paper-plane): add.tholdVRules(#4aa3df) and.tholdEdit(#b3b3b3), and brighten the existing.tholdGlyph*colors (edit#b3b3b3, disable#e06666, enable#4caf50, chart#d6a13a, log#c7c04a, acknowledge#7cb342) so they remain legible against the dark backgrounds (sunriseandpaper-planeare dark themes despite their names — both use near-blackbodybackgrounds).Semantic color coding is preserved across themes (gray = edit/create, red = disable, green = enable, amber = chart, blue = VRULE toggle); only brightness is tuned per theme family.
Related Issue
Motivation and Context
The two graph-view action glyphs had no defined color in any theme, leaving them visually inconsistent with the rest of the themed icons. This gives both glyphs an intentional, theme-matched color in every packaged theme and fixes the low-contrast threshold glyphs on the dark themes.
Because the glyphs are rendered by the plugin itself and the matching stylesheet is loaded by
thold_page_head()viaget_selected_theme(), the change works the same on both Cacti 1.2.x and the develop branch.How Has This Been Tested?
thold_graph_button()insetup.phpthat the injected<i>elements carry thetholdVRulesandtholdEditclasses being styled..tholdVRulesand.tholdEditexactly once and that all CSS braces balance.bodybackground in Cacti core (include/themes/*/main.css).Screenshots (if appropriate):
Types of changes
Checklist: