-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Add colorization based on health level for panels and toolbar #2178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
85b08f0
427ec23
0c15dde
2f8c3d4
aa96318
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,13 @@ | |
--djdt-button-border-color: var(--djdt-table-border-color); | ||
--djdt-pre-border-color: var(--djdt-table-border-color); | ||
--djdt-raw-border-color: var(--djdt-table-border-color); | ||
|
||
--djdt-health-background-color-1: #4b3f1b; | ||
--djdt-health-color-1: #ffe761; | ||
--djdt-health-border-color-1: #ffcc00; | ||
--djdt-health-background-color-2: #5a2327; | ||
--djdt-health-color-2: #ffb3b3; | ||
--djdt-health-border-color-2: #ff0000; | ||
} | ||
|
||
#djDebug[data-theme="dark"] { | ||
|
@@ -56,6 +63,13 @@ | |
--djdt-button-border-color: var(--djdt-table-border-color); | ||
--djdt-pre-border-color: var(--djdt-table-border-color); | ||
--djdt-raw-border-color: var(--djdt-table-border-color); | ||
|
||
--djdt-health-background-color-1: #4b3f1b; | ||
--djdt-health-color-1: #ffe761; | ||
--djdt-health-border-color-1: #ffcc00; | ||
--djdt-health-background-color-2: #5a2327; | ||
--djdt-health-color-2: #ffb3b3; | ||
--djdt-health-border-color-2: #ff0000; | ||
} | ||
|
||
/* Debug Toolbar CSS Reset, adapted from Eric Meyer's CSS Reset */ | ||
|
@@ -377,6 +391,29 @@ | |
animation: spin 2s linear infinite; | ||
} | ||
|
||
/* Panel and Toolbar hidden button health states */ | ||
#djDebug .djdt-health-1 { | ||
/* The background can be shadowed by #djDebugToolbar li.djdt-active a:hover */ | ||
background: var(--djdt-health-background-color-1) !important; | ||
color: var(--djdt-health-color-1); | ||
} | ||
|
||
#djDebug .djdt-health-2 { | ||
/* The background can be shadowed by #djDebugToolbar li.djdt-active a:hover */ | ||
background: var(--djdt-health-background-color-2) !important; | ||
color: var(--djdt-health-color-2); | ||
} | ||
|
||
#djDebug .djdt-toolbarhandle-health-1 { | ||
/* The border-color is shadowed by the default #djShowToolBarButton border */ | ||
border-color: var(--djdt-health-border-color-1) !important; | ||
} | ||
|
||
#djDebug .djdt-toolbarhandle-health-2 { | ||
/* The border-color is shadowed by the default #djShowToolBarButton border */ | ||
border-color: var(--djdt-health-border-color-2) !important; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment why all these |
||
|
||
@keyframes spin { | ||
0% { | ||
transform: rotate(0deg); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ | |
</ul> | ||
</div> | ||
<div class="djdt-hidden" id="djDebugToolbarHandle"> | ||
<div title="{% translate 'Show toolbar' %}" id="djShowToolBarButton"> | ||
<div title="{% translate 'Show toolbar' %}" id="djShowToolBarButton" class="{% if toolbar.health_level %} djdt-toolbarhandle-health-{{ toolbar.health_level }}{% endif %}"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I generally do not like uppercase letters much in ID and class attributes, but I wonder if we should keep capitalization for internal consistency here? Any thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are inconsistent right now. Some places use camelCase when starting class names with Rightnow, I can change them to camelcase if you want, but for consistency the changes in whole project will be required in future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, don't worry about it then. Being as consistent as possible is good enough. |
||
<span id="djShowToolBarD">D</span><span id="djShowToolBarJ">J</span>DT | ||
</div> | ||
</div> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
|
||
from debug_toolbar import APP_NAME, settings as dt_settings | ||
from debug_toolbar.store import get_store | ||
from debug_toolbar.utils import HealthLevel | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
@@ -72,6 +73,17 @@ def csp_nonce(self): | |
""" | ||
return getattr(self.request, "csp_nonce", None) | ||
|
||
@property | ||
def health_level(self): | ||
""" | ||
Return the maximum health level across all panels. | ||
This is used to color the toolbar hidden button. | ||
""" | ||
if not self.panels: | ||
return HealthLevel.NONE | ||
|
||
return max(panel.health_level for panel in self.panels) | ||
|
||
|
||
def get_panel_by_id(self, panel_id): | ||
""" | ||
Get the panel with the given id, which is the class name by default. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
import sys | ||
import warnings | ||
from collections.abc import Sequence | ||
from enum import IntEnum | ||
from pprint import PrettyPrinter, pformat | ||
from typing import Any | ||
|
||
|
@@ -401,3 +402,18 @@ def is_processable_html_response(response): | |
and content_encoding == "" | ||
and content_type in _HTML_TYPES | ||
) | ||
|
||
|
||
class HealthLevel(IntEnum): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @matthiask @hunzlahmalik do we want to bikeshed a bit on the name here? If we build out an API, this will likely be included in that and no longer be a private-ish thing. In my mind, "severity" may be more appropriate, but then it may lack the context of what is the severity in relation to. Not sure, but want to bring it up since it's likely to become a public interface in the near future (< 1 years). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we can change the name. I thought about it and was inclined to using the |
||
""" | ||
Represents the health or alert level for a panel or the toolbar as a whole. | ||
|
||
Used to indicate the severity of issues detected by panels, allowing the UI to reflect | ||
warning or critical states (e.g., via colorization). Panels should return one of these | ||
levels from their `health_level` property. The toolbar will aggregate the maximum level | ||
across all panels to determine the overall toolbar health state. | ||
""" | ||
|
||
NONE = 0 | ||
WARNING = 1 | ||
CRITICAL = 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use a descriptive word rather than 1 and 2 here please?