-
Notifications
You must be signed in to change notification settings - Fork 595
Fix dark theme issues in trace timeline view #3244
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?
Fix dark theme issues in trace timeline view #3244
Conversation
- Document the themes.enabled configuration for dark mode toggle - Add themes config to the example JSON file - Document common configuration options (dependencies, monitor, linkPatterns) - Clarify the nested object structure required for configuration Signed-off-by: Jonah Kowall <[email protected]>
- SpanTreeOffset.css: Use design tokens for icon and indent guide colors - TimelineCollapser.css: Use text tokens for collapse/expand button colors - TimelineHeaderRow.css: Use surface/border tokens for header background - TracePageHeader.css: Use design tokens for back button and borders - CanvasSpanGraph.css: Use surface token for minimap background - ViewingLayer.css: Use border/surface tokens for graph overlay - TickLabels.css: Use text-secondary token for label colors - Ticks.css: Use border/text tokens for tick marks and labels - Scrubber.css: Use interactive/text tokens for scrubber handles - GraphTicks.css: Use border token for tick stroke color Fixes visibility issues with expand arrows, timeline header, back button, and minimap in dark mode. Signed-off-by: Jonah Kowall <[email protected]>
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.
Pull request overview
This PR fixes dark theme visibility issues in the trace timeline view by replacing hardcoded color values with semantic design tokens from vars.css. This ensures all UI elements properly adapt to both light and dark themes.
Key changes:
- Converted hardcoded hex colors to CSS custom properties (design tokens) across 10 CSS files
- Added theme toggle configuration to the example config file
- Added comprehensive documentation for theme configuration and other common options
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
TimelineHeaderRow.css |
Updated header background, border, and text colors to use theme-aware tokens |
TimelineCollapser.css |
Converted collapse/expand button colors to semantic text tokens |
Ticks.css |
Updated tick marks and labels to use border and text tokens |
SpanTreeOffset.css |
Fixed expand/collapse icon colors and hover backgrounds with appropriate tokens |
TracePageHeader.css |
Updated back button, borders, and overview items to use surface and border tokens |
ViewingLayer.css |
Converted graph border and inactive overlay to semantic tokens |
TickLabels.css |
Updated timeline label color to use text-secondary token |
Scrubber.css |
Converted scrubber handle and line colors to interactive and text tokens |
GraphTicks.css |
Updated tick stroke color to use border-strong token |
CanvasSpanGraph.css |
Changed minimap background to use surface-tertiary token |
jaeger-ui.config.example.json |
Added theme configuration example |
README.md |
Added comprehensive documentation for theme toggle and other configuration options |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3244 +/- ##
==========================================
- Coverage 97.83% 97.79% -0.04%
==========================================
Files 261 261
Lines 8209 8219 +10
Branches 2178 2180 +2
==========================================
+ Hits 8031 8038 +7
- Misses 175 178 +3
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Fix canvas span graph minimap to use theme-aware background color - Add MutationObserver to re-render canvas when theme changes - Make ViewingLayer SVG background transparent so canvas shows through - Fix search bar clear icon visibility in dark mode - Fix theme toggle button icon visibility - Add Ant Design input/button overrides for dark mode - Fix span bar row hover states and error icon colors - Fix JSON markup colors in trace timeline viewer Signed-off-by: Jonah Kowall <[email protected]>
|
Part of #1911 Post-merge tasks:
|
| background-color: var(--surface-tertiary); | ||
| border-color: var(--border-default); | ||
| color: var(--text-primary); |
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.
I think this is the wrong direction again. If anything we should be trying to align with AntD theme support, not work against it. A change like this ^ undermines AntD's styling.
The ideal situation would be that we do not override so much in ThemeConfig in ThemeProvider.tsx, but instead:
- define minimum set in the theme provider where by setting up the Javascript properties we effectively override standard
var(--ant-...)variables that are used by all standard AntD components - we define our own variables by referencing the corresponding
--ant-xxxvariables wherever possible - we use our own variables in our own components
Summary
Fixes visibility issues in the trace timeline view when using dark mode.
Issues Fixed
#000which was invisible on dark backgrounds#ececec) that didn't adapt to dark modergba(0, 0, 0, 0.5)which was invisible in dark modeChanges Made
Converted hardcoded colors to design tokens from
vars.css:SpanTreeOffset.cssTimelineCollapser.cssTimelineHeaderRow.cssTracePageHeader.cssCanvasSpanGraph.cssViewingLayer.cssTickLabels.cssTicks.cssScrubber.cssGraphTicks.cssTesting