fix(home/stats): drop /mo suffix from Period Savings total (revert #763 auto-fix) - #767
Conversation
Revert the change applied by PR #763 ("CodeRabbit auto-fixes"): it appended the unit suffix to Period Savings even though the line immediately above explicitly says Period Savings is a cumulative dollar total over the selected date range, NOT a per-unit rate. The auto-fix violated its own comment and broke 4 tests in frontend/src/__tests__/savings-history.test.ts, which assert plain "$X.XX" output (no suffix). Restoring the original behaviour so the tests and the comment agree again.
|
@coderabbitai review |
|
Warning Review limit reached
More reviews will be available in 8 minutes and 11 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Actions performedReview triggered.
|
Period Savings is a cumulative dollar total over the selected date range,
not a per-unit rate. Appending /mo, /hr, or /yr to the value misrepresents
it as a rate. Instead, the label becomes "Period Savings (monthly)" etc.,
reinforcing which view mode is active without implying a rate.
Added id="period-savings-label" to the <h4> in index.html, updated
renderSavingsStats to set its textContent to `Period Savings (${unit})`
using the already-available adjective variable (lowercased), and added
label assertions to the unit-toggle test for all three units.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
The label ("Period Savings") answers "what is this number" and should
stay plain across all unit modes. A new sub-line element
(#period-savings-unit, class stat-unit-context) below the dollar value
carries the view-mode context ("shown in monthly/hourly/yearly
equivalents"), making the semantic layering explicit: label = what,
value = number, sub-line = how it is displayed. Updated the DOM fixture
and three unit-toggle assertions in savings-history.test.ts to query
#period-savings-unit. Added .stat-unit-context CSS rule to charts.css.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Problem
PR #763 ("CodeRabbit auto-fixes") appended
' ' + suffix(e.g./mo) toperiodSavingsEl.textContent. That is semantically wrong: Period Savingsis a cumulative dollar total over the selected date range, not a per-unit
rate. Displaying
$500.00 /moimplies a monthly rate, not a total.Four tests failed as a result.
Update: moved unit indicator below the value for cleaner semantic separation.
Fix
Move the unit indicator from the label to a small sub-line below the value:
Period Savings$500.00#period-savings-unit):shown in monthly equivalents/shown in hourly equivalents/shown in yearly equivalentsThis separates the three concerns cleanly: label = what, value = number,
sub-line = how it is displayed. The sub-line uses the new
.stat-unit-contextCSS class (small, muted text) consistent with the project's
--cudly-text-mutedtoken already used throughout stat cards.
Changes
frontend/src/index.html: added<p class="stat-unit-context" id="period-savings-unit">shown in monthly equivalents</p>below#period-savings. Restored#period-savings-labelto plainPeriod Savings.frontend/src/modules/savings-history.ts: inrenderSavingsStats,set
#period-savings-labelto the constant'Period Savings'and drive#period-savings-unitwithshown in ${adjective.toLowerCase()} equivalents.Updated inline comment to reflect new "below the value" placement.
frontend/src/styles/charts.css: added.stat-unit-contextrule(
font-size: 0.85rem,color: var(--cudly-text-muted),margin-top: 0.25rem).frontend/src/__tests__/savings-history.test.ts: added<p id="period-savings-unit">to the DOM fixture; updated the threeunit-toggle assertions to query
#period-savings-unitinstead of#period-savings-label.Tests
79 tests pass. The originally-failing value assertions (
toBe('$500.00')etc.)are unchanged.