-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Log Viewer: Enhances the donut chart to be responsive, link to log search, and show numbers directly #20928
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?
Conversation
- Import and use repeat directive for better performance - Add _logLevelKeys state property to track log level keys - Update setLogLevelCount() to populate _logLevelKeys - Replace .map() with repeat() in render method for legend and donut slices - Update willUpdate to observe both filter and response changes - Resolves TODO comment about using repeat directive 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add showInlineNumbers property to optionally display numbers inside slices - Implement #getTextPosition() method to calculate text position at slice center - Render SVG text elements when showInlineNumbers is enabled - Fix tooltip positioning to appear near cursor (changed from x-10, y-70 to x+10, y+10) - Recalculate container bounds on each mouse move to handle window resize - Add pointer-events: none to tooltip to prevent mouse interference - Add CSS styling for slice numbers (user-select: none) - Enable inline numbers by default in log viewer log types chart 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add href property to donut-slice element for clickable slices - Wrap SVG paths in <a> tags when href is provided - Update Circle interface to include href property - Add showDescription property to optionally display description text - Render description as visible text below the chart - Add CSS styling for description text - Update log-types-chart to build search URLs with log level and date range - Observe dateRange from context to build accurate search URLs - Enable clickable slices and visible description in log-types-chart Now clicking on a donut slice navigates to the search view filtered by that log level and the current date range. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add media query to switch from column to row layout on screens wider than 768px. This ensures the legend and donut chart are displayed side by side on desktop resolutions instead of stacked vertically. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://victorious-ground-017b08103-20928.westeurope.6.azurestaticapps.net |
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 enhances the Log Viewer's donut chart component to be more interactive and responsive. It adds clickable slices that navigate to filtered search results, displays counts inline within slices, and implements responsive layouts for different screen sizes. The router was also enhanced to support SVG anchor elements for SPA navigation.
Key Changes:
- Added clickable slices to the donut chart that link to filtered log search views with appropriate date ranges
- Implemented responsive layout using CSS Grid with container queries (312px breakpoint)
- Extended the core router to handle SVG
<a>elements for SPA navigation
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Umbraco.Web.UI.Client/src/packages/core/router/router-slot/util/anchor.ts | Added support for SVG anchor elements in the router's click handler, extracting href from SVGAnimatedString and handling URL resolution |
| src/Umbraco.Web.UI.Client/src/packages/log-viewer/components/donut-chart/donut-chart.element.ts | Added showInlineNumbers and showDescription properties, implemented text positioning for inline numbers, wrapped slices in SVG anchors when href is provided, made container responsive |
| src/Umbraco.Web.UI.Client/src/packages/log-viewer/components/donut-chart/donut-slice.element.ts | Added href property to enable clickable slices |
| src/Umbraco.Web.UI.Client/src/packages/log-viewer/workspace/views/overview/components/log-viewer-log-types-chart.element.ts | Added date range observer, implemented #buildSearchUrl() method, refactored template to use Lit's repeat directive, implemented responsive CSS Grid layout |
| src/Umbraco.Web.UI.Client/src/mocks/data/log-viewer.data.ts | Refactored getLevelCount() to use cleaner reduce pattern with proper TypeScript typing |
| override render() { | ||
| return html` | ||
| <uui-box id="types" headline="Log types"> | ||
| <p id="description">In the chosen date range you have this number of log message of type:</p> |
Copilot
AI
Nov 21, 2025
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.
The description text has a grammatical error. It should say "this number of log messages" (plural) since it refers to multiple messages.
Change to: "In the chosen date range you have this number of log messages of type:"
| <p id="description">In the chosen date range you have this number of log message of type:</p> | |
| <p id="description">In the chosen date range you have this number of log messages of type:</p> |
| #calculateDetailsBoxPosition = (event: MouseEvent) => { | ||
| // Recalculate bounds on each mouse move to handle window resize | ||
| this.#containerBounds = this._container.getBoundingClientRect(); | ||
| const x = this.#containerBounds ? event.clientX - this.#containerBounds?.left : 0; | ||
| const y = this.#containerBounds ? event.clientY - this.#containerBounds?.top : 0; | ||
| this._posX = x - 10; | ||
| this._posY = y - 70; | ||
| this._posX = x + 10; | ||
| this._posY = y + 10; | ||
| }; |
Copilot
AI
Nov 21, 2025
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.
[nitpick] The #calculateDetailsBoxPosition method is called on every mousemove event and recalculates getBoundingClientRect() each time. This could impact performance, especially with many rapid mouse movements.
Consider throttling or debouncing the position calculation, or only recalculating the bounds when the window resizes rather than on every mouse move. The bounds don't change during mouse movement unless the window is resized.
|
|
||
| // Change the history! | ||
| history.pushState(null, '', path); | ||
| history.pushState(null, '', fullUrl); |
Copilot
AI
Nov 21, 2025
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.
The history.pushState API expects a string for the URL parameter, not a URL object. While the URL object will be coerced to a string, it will include the full absolute URL (e.g., https://example.com/path) rather than just the path portion.
This differs from the previous implementation which passed pathname + search + hash, and may cause issues with the history state.
Recommend changing to:
history.pushState(null, '', fullUrl.pathname + fullUrl.search + fullUrl.hash);| history.pushState(null, '', fullUrl); | |
| history.pushState(null, '', fullUrl.pathname + fullUrl.search + fullUrl.hash); |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Summary
Enhances the donut chart on the Log Viewer. Chart slices are clickable and navigate to filtered search results. The router now supports SVGAElements as well.
Changes
Enhanced Components
Donut Chart Component (
umb-donut-chart):hrefpropertyshow-inline-numbersattribute to display counts inside slices<a>tags when href is providedDonut Slice Component (
umb-donut-slice):hrefproperty to make slices clickableLog Viewer Overview (
log-viewer-log-types-chart.element.ts):Core Router Enhancement
anchor.ts):<a>elements (previously only HTML anchors)SVGAnimatedStringtype for SVG href and target propertiesdocument.location.originas baseVisual Changes
Desktop Layout (container ≥ 312px):
Before:

After:

Mobile Layout (container < 312px):
Before:

After:

Technical Details
URL Structure
Clicking a slice navigates to:
section/settings/workspace/logviewer/view/search/?loglevels=Information&startDate=2025-11-19&endDate=2025-11-21
Router Changes
The router's
ensureAnchorHistory()function now handles bothHTMLAnchorElementandSVGAElement:.baseValfor SVG (handlesSVGAnimatedString)URLconstructor withdocument.location.originfor consistent URL resolutionfullUrl.origin !== location.originto distinguish internal/external linksResponsive Breakpoint
aspect-ratio: 1Test Steps
1. Basic Functionality
2. Chart Interactions
?loglevels=Information&startDate=...&endDate=...3. Responsive Layout
Desktop (≥312px width):
Mobile (<312px width):
4. Date Range Changes
5. External Link Test (Router regression)
https://google.comNotes
document.location.originto match existing Umbraco root-relative URL patterns