Skip to content

fix(#1240): fix overall balance to stay anchored to today across calendar navigation#1350

Merged
tupaschoal merged 7 commits into
TTLApp:mainfrom
rbalogic:fix/1240-compute-overall-balance
Jul 17, 2026
Merged

fix(#1240): fix overall balance to stay anchored to today across calendar navigation#1350
tupaschoal merged 7 commits into
TTLApp:mainfrom
rbalogic:fix/1240-compute-overall-balance

Conversation

@rbalogic

Copy link
Copy Markdown
Contributor

Summary

Fixes #1240.

Overall Balance now stays anchored to the real current day, even when navigating to past or future dates in Month/Day views.

What Changed?

  • Anchored Overall Balance calculations to today in both Month and Day views
  • Preserved existing count-today behavior
  • Updated English tooltip copy to match implemented behavior
  • Added renderer test coverage for past/future navigation in both calendar views
  • Stabilized the calendar window wait-mode renderer test

Why This Change?

Navigating the calendar previously made Overall Balance appear to shift with the viewed date.
This update keeps the metric fixed to today for consistent, predictable behavior.

Testing

npm run test:electron-mocha

npx eslint \
  renderer/classes/BaseCalendar.js \
  renderer/classes/MonthCalendar.js \
  renderer/classes/DayCalendar.js \
  __tests__/__renderer__/calendar.mjs \
  __tests__/__renderer__/classes/MonthCalendar.mjs \
  __tests__/__renderer__/classes/DayCalendar.mjs \
  --config configs/eslint.config.mjs

npx prettier -c locales/en/translation.json --config configs/.prettierrc.json

Notes

  • No screenshots included: behavior change only, plus a tooltip text update.

@rbalogic rbalogic changed the title fix(#1240): fix overall balance to stay anchored to today across cale… fix(#1240): fix overall balance to stay anchored to today across calendar navigation Apr 25, 2026
@rbalogic
rbalogic force-pushed the fix/1240-compute-overall-balance branch 2 times, most recently from b774338 to d533cdb Compare May 2, 2026 10:10
@codecov

codecov Bot commented May 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rbalogic
rbalogic force-pushed the fix/1240-compute-overall-balance branch from d533cdb to a937a4e Compare May 3, 2026 10:34
@rbalogic

rbalogic commented May 3, 2026

Copy link
Copy Markdown
Contributor Author

@tupaschoal Please review this PR when you can and approve workflow if required, Thanks.

@tupaschoal

Copy link
Copy Markdown
Collaborator

Sorry for the delay, I'm currently very busy, so it will still be a couple weeks until I can review it

@tupaschoal tupaschoal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @rbalogic, sorry it took me so long to review this, but I'm finally here :D

It looks to be working just fine and thanks for adding tests. Maybe we need an additional test with some actual values just to trust that the math is expected?

Also I noticed that when we switch months/days (depending on calendar view), the overall balance flickers as if it just recalculated, but now we know it doesn't, so there is probably logic somewhere that doesn't need to be called everytime the view changes.

Comment on lines +27 to +30
waitUntilHandlersSetup = new Promise(resolve =>
{
window.__resolveCalendarHandlersSetup = resolve;
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why did you have to add this piece of code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was added to make the test deterministic.

src/calendar.js registers the IPC handlers during async setup after import, and in the full suite this test could sometimes assert before handleToggleMainWindowWait had been assigned. This small wait just synchronizes the test with that setup.

@rbalogic

Copy link
Copy Markdown
Contributor Author

@tupaschoal I have addressed the comments. Added a real-value regression test, removed the redundant month-view overall balance refresh that was causing the extra recalculation, and explained the calendar.mjs synchronization code in the inline reply.

Please review when you can, Thanks.

@rbalogic
rbalogic requested a review from tupaschoal July 13, 2026 07:55
@tupaschoal

Copy link
Copy Markdown
Collaborator

I tested and I still see a small flicker, I recorded ahigh fps gif
flicker

Note that it sets the balance to ... because of its placeholder text

image

I'm ok if you think this should be handled in a separate PR, but I think it becomes a bit weirder now that the balance itself is not changing.

@rbalogic

Copy link
Copy Markdown
Contributor Author

@tupaschoal Thanks, I handled this in the same PR.

The remaining flicker was coming from the month view redraw recreating the overall-balance field with the ... placeholder before the async overall-balance refresh completed. I changed the calendar to restore the last known overall balance immediately after redraw, and then let the async refresh update it if needed.

So this should keep the field stable during navigation while preserving the existing async calculation flow.

Comment thread renderer/classes/BaseCalendar.js
@rbalogic
rbalogic requested a review from tupaschoal July 16, 2026 16:30
@tupaschoal
tupaschoal merged commit fa52e83 into TTLApp:main Jul 17, 2026
4 checks passed
@tupaschoal

Copy link
Copy Markdown
Collaborator

\changelog-update
Message: Fix #1240: Overall balance updates when changing months/days

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.

Overall balance updates when changing months

2 participants