Skip to content

fix: adjust SmoothieChart minValue and scaling for improved chart acc…#2382

Merged
limetech merged 1 commit into
masterfrom
fix/chart-overflow-clipping
Sep 18, 2025
Merged

fix: adjust SmoothieChart minValue and scaling for improved chart acc…#2382
limetech merged 1 commit into
masterfrom
fix/chart-overflow-clipping

Conversation

@elibosley

@elibosley elibosley commented Sep 18, 2025

Copy link
Copy Markdown
Member

…uracy in DashStats page

Summary by CodeRabbit

  • Bug Fixes

    • Prevented negative values from appearing on the CPU usage chart.
  • Style

    • Applied a consistent 2% margin to the top and bottom of CPU and Network charts for improved readability.
    • Standardized chart scaling to use symmetrical margins, ensuring clearer visualization across varying data ranges.

@coderabbitai

coderabbitai Bot commented Sep 18, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Updates chart configuration in DashStats.page: sets CPU chart minValue to 0, removes yRangeFunction, and applies minValueScale/maxValueScale of 1.02 to CPU and network charts, introducing symmetrical 2% margins and preventing negative CPU values.

Changes

Cohort / File(s) Summary
Chart scale adjustments
emhttp/plugins/dynamix/DashStats.page
CPU: minValue set to 0; removed yRangeFunction; added minValueScale: 1.02 and maxValueScale: 1.02. Network: replaced maxValueScale: 1.0 with minValueScale: 1.02 and maxValueScale: 1.02.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A nibble of charts, a hop of delight,
I trimmed the lows and set scales right.
Two percent margins, neat and tight—
CPU won’t dip into the night.
Network hums with balanced sight.
Thump-thump, commit—looks just right! 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "fix: adjust SmoothieChart minValue and scaling for improved chart acc…" clearly and concisely identifies the main change (adjusting SmoothieChart minValue and scaling) and directly reflects the DashStats.page chart-range edits in the diff, making it specific and relevant for reviewers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/chart-overflow-clipping

Comment @coderabbitai help to get the list of available commands and usage tips.

@elibosley elibosley requested a review from ljm42 September 18, 2025 21:07
@github-actions

Copy link
Copy Markdown

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2025.09.18.2107
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

  1. Go to Plugins → Install Plugin
  2. Copy and paste this URL:
https://preview.dl.unraid.net/pr-plugins/pr-2382/webgui-pr-2382.plg
  1. Click Install

Alternative: Direct Download

⚠️ Important Notes:

  • Testing only: This plugin is for testing PR changes
  • Backup included: Original files are automatically backed up
  • Easy removal: Files are restored when plugin is removed
  • Conflicts: Remove this plugin before installing production updates

📝 Modified Files:

Click to expand file list
emhttp/plugins/dynamix/DashStats.page

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2025.09.18.2107, or run:

plugin remove webgui-pr-2025.09.18.2107

🤖 This comment is automatically generated and will be updated with each new push to this PR.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
emhttp/plugins/dynamix/DashStats.page (2)

1466-1468: minValueScale/maxValueScale are no-ops with fixed min/max; pick one approach

With minValue: 0 and maxValue: 100 set, Smoothie’s min/max scaling factors won’t change the range. If you want visible headroom, drop the fixed max; if you want a fixed 0–100 scale, remove the scales to avoid confusion.

Option A (fixed range 0–100; remove scales):

-  minValueScale: 1.02,
-  maxValueScale: 1.02,

Option B (dynamic headroom; drop fixed max so maxValueScale applies):

-  maxValue: 100,
+  // Let Smoothie compute dynamic headroom near 100%
+  // maxValue: 100,

Please confirm the intended behavior (fixed 0–100 vs. padded headroom). If choosing Option B and you want labels capped at 100, we can also clamp yMaxFormatter to 100.


1493-1495: Net chart: minValueScale is ineffective with minValue=0; keep only maxValueScale

Because minValue is fixed at 0, minValueScale won’t create a lower margin. Keeping maxValueScale=1.02 provides the desired 2% headroom at the top.

Apply:

-  minValueScale: 1.02,
   maxValueScale: 1.02,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a2d323 and 4499335.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix/DashStats.page (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T21:22:09.359Z
Learnt from: elibosley
PR: unraid/webgui#2378
File: emhttp/plugins/dynamix/DashStats.page:1440-1446
Timestamp: 2025-09-17T21:22:09.359Z
Learning: For SmoothieCharts millisPerPixel calculation, use element.offsetWidth (CSS pixels) rather than canvas.width or devicePixelRatio adjustments. The library appears to handle DPI scaling internally, and using canvas pixel width gives incorrect timing on HiDPI displays.

Applied to files:

  • emhttp/plugins/dynamix/DashStats.page
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
emhttp/plugins/dynamix/DashStats.page (2)

1450-1452: Setting CPU chart minValue=0 is correct to prevent negative CPU values

Keeps the axis grounded at zero and matches the intent to avoid sub‑zero CPU.


1448-1474: Smoothie options validated — minValueScale/maxValueScale supported

plugins/dynamix/javascript/smoothie.js documents and implements minValueScale/maxValueScale (comments at 305–307), sets defaults (421–422) and uses them in calculations (810, 817).

@limetech limetech merged commit 2c57119 into master Sep 18, 2025
5 checks passed
@limetech limetech deleted the fix/chart-overflow-clipping branch September 18, 2025 21:22
@github-actions

Copy link
Copy Markdown

🧹 PR Test Plugin Cleaned Up

The test plugin and associated files for this PR have been removed from the preview environment.


🤖 This comment is automatically generated when a PR is closed.

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.

2 participants