Enhanced Monthly chart to display cumulative and monthly data via UI toggle. Addresses issue #5. by antamy · Pull Request #7 · MarkT1065/wheeler

antamy · 2025-10-11T23:48:48Z

The original implementation displayed a cumulative gains chart by month. This enhancement adds a cumulative / monthly button group to the UI so that the chart either displays cumulative gains or gains by month. Graph and axis titles have been updated accordingly.

In addition, code that deals with the Totals pane at the top of the page has been refactored. It was originally embedded within the code to create the cumulatvive chart and has now been extracted as a separate updateTotalsPanelValues() function that's called from the chart creation function.

Variables have been renamed for clarity:

cumulativeGainsChart → gainsOverTimeChartInstance
cumulativeGainsCtx → gainsOverTimeCtx

Function renamed to createGainsOverTimeChart()

cumulativePutsMonthData → putsMonthlyData (now represents the raw monthly amounts)
putsCumulativeData stays as is (represents the running totals)

Similar changes for calls, cap gains, and dividends

Cleaner logic flow:

Load monthly data first
Calculate cumulative from monthly
Choose which dataset to display based on view mode
Update totals panel with final running totals

…toggle

…toggle (code and screenshots)

…/antamy/wheeler into feature/add-chart-monthly-toggle

MarkT1065 · 2025-10-13T06:12:59Z

● Pull Request #7 Evaluation

Title: Enhanced Monthly chart to display cumulative and monthly data via UI toggle

Status: OPEN | Author: antamy (aroby)

Summary: Adds a cumulative/monthly toggle to the Monthly page chart, allowing users to switch between viewing cumulative gains over time and monthly gains.

Changes Overview

Files Modified:

internal/web/templates/monthly.html (+265, -151)
Binary database file (test data)
New screenshots added

Code Quality Assessment

Strengths:

Clean Refactoring: Extracted totals panel logic into updateTotalsPanelValues() function
Smart Naming: Renamed variables for clarity (e.g., cumulativeGainsChart → gainsOverTimeChartInstance)
Proper Chart Management: Destroys existing chart before creating new one to prevent memory leaks
Consistent Data Flow: Loads monthly data first, calculates cumulative from it
Good UX: Toggle buttons with clear visual states (green for active, gray for inactive)
Maintains Functionality: Totals panel still shows cumulative YTD values regardless of view mode

Technical Implementation:

Uses Chart.js with proper instance management
Toggle buttons use inline styles (acceptable for this simple case)
Dynamic dataset switching based on viewMode parameter
Separate y-axis labels for cumulative vs monthly views
Color coding maintained for capital gains (red for losses, green for profits)

Minor Concerns:

Inline Styles: Toggle button styles are inline rather than in CSS file
DOMContentLoaded: Event listener wrapping might be redundant if script is already at page bottom
Binary File: data/test_db.db included in PR (should be gitignored)

Recommendation: APPROVE with minor suggestions

This is a well-executed enhancement that:

Directly addresses issue Monthly chart - different views #5
Improves code organization through refactoring
Adds valuable functionality without breaking existing features
Follows good coding practices (proper cleanup, clear naming)

Optional improvements for follow-up:

Move toggle button styles to styles.css
Consider removing test database from git tracking
Add CSS transition effects to the toggle buttons if not already present

The PR is ready to merge as-is, with the minor improvements being nice-to-haves rather than blockers.

MarkT1065 · 2025-10-13T06:30:25Z

I appreciate this new feature! Seeing results by monthly and cumulative over time is a nice improvement. Thanks for contributing! I'm heading out of town on PTO tomorrow. Mind if we merge when I get back?

antamy · 2025-10-13T13:06:17Z

I appreciate this new feature! Seeing results by monthly and cumulative over time is a nice improvement. Thanks for contributing! I'm heading out of town on PTO tomorrow. Mind if we merge when I get back?

Sure, that's fine. I'll look at the follow-ups in the meantime. This is the first time I've used git, Go and Claude plus my development experience was many years ago and more backend than frontend, so I'm quickly getting up to speed! Enjoy your time off.

antamy · 2025-10-23T15:56:35Z

Hi, when can we merge my contribution? I’m not entirely sure what I need to do … From: MarkT1065 ***@***.***> Sent: Monday, October 13, 2025 1:31 AM To: MarkT1065/wheeler ***@***.***> Cc: Anthony R. Roby ***@***.***>; Author ***@***.***> Subject: Re: [MarkT1065/wheeler] Enhanced Monthly chart to display cumulative and monthly data via UI toggle. Addresses issue #5. (PR #7) [https://avatars.githubusercontent.com/u/228252352?s=20&v=4]MarkT1065 left a comment (MarkT1065/wheeler#7)<#7 (comment)> I appreciate this new feature! Seeing results by monthly and cumulative over time is a nice improvement. Thanks for contributing! I'm heading out of town on PTO tomorrow. Mind if we merge when I get back? — Reply to this email directly, view it on GitHub<#7 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABUGFTTPTOYZ4VZ4W22TMKD3XNBJNAVCNFSM6AAAAACI56X4XKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGOJWGA2DIMRSGU>. You are receiving this because you authored the thread.Message ID: ***@***.******@***.***>>

MarkT1065 · 2025-10-23T16:31:50Z

On the way back home … I’ll be available in a day or two

…

On Thu, Oct 23, 2025 at 5:56 PM aroby ***@***.***> wrote: *antamy* left a comment (MarkT1065/wheeler#7) <#7 (comment)> Hi, when can we merge my contribution? I’m not entirely sure what I need to do … From: MarkT1065 ***@***.***> Sent: Monday, October 13, 2025 1:31 AM To: MarkT1065/wheeler ***@***.***> Cc: Anthony R. Roby ***@***.***>; Author ***@***.***> Subject: Re: [MarkT1065/wheeler] Enhanced Monthly chart to display cumulative and monthly data via UI toggle. Addresses issue #5. (PR #7) [https://avatars.githubusercontent.com/u/228252352?s=20&v=4]MarkT1065 left a comment (MarkT1065/wheeler#7)< #7 (comment)> I appreciate this new feature! Seeing results by monthly and cumulative over time is a nice improvement. Thanks for contributing! I'm heading out of town on PTO tomorrow. Mind if we merge when I get back? — Reply to this email directly, view it on GitHub< #7 (comment)>, or unsubscribe< https://github.com/notifications/unsubscribe-auth/ABUGFTTPTOYZ4VZ4W22TMKD3XNBJNAVCNFSM6AAAAACI56X4XKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGOJWGA2DIMRSGU>. You are receiving this because you authored the thread.Message ID: ***@***.******@***.***>> — Reply to this email directly, view it on GitHub <#7 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BWNNVQBTI64OWYBQELWCO4L3ZD3EVAVCNFSM6AAAAACI56X4XKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIMZXG44TQMJWHA> . You are receiving this because you commented.Message ID: ***@***.***>

MarkT1065 · 2025-10-26T11:11:54Z

@antamy the code LGTM. Thanks for the contribution! I left 2 comments above (fixing a 404 on the readme and reverting the db). I can merge after those two fixes.

MarkT1065

I think I missed this step before heading on PTO. I reviewed and left comments but didn't "submit request changes".

Thanks for your patience!

MarkT1065 · 2025-10-13T06:08:11Z

were any changes were made to this or was this the result of test runs?

we probably don't need to check in changes to this database instance except for schema changes. i don't really have this written in any contributors guide because this project has been single author to date. I really appreciate your Pull Request!

No changes, not sure why this was included. I figured out how to remove it.

MarkT1065 · 2025-10-13T06:28:10Z

New screenshots look good. The link on the README is to the old screenshot, so it would 404 now.

Claude didn't find that in review. I noticed it looking at your branch in github. so much for the AI overlords? :P

Good catch. I decided to remove the two screenshots and just have one, since both of them didn't really add anything to the README. I did update the README to address a spelling mistake.

MarkT1065 · 2025-10-26T20:44:04Z

LGTM. squashing and merging.

MarkT1065 · 2025-10-26T20:44:23Z

@antamy thank you for this contribution!

…toggle. Addresses issue MarkT1065#5. (MarkT1065#7) * Enhanced Monthly chart to display cumulative and monthly data via UI toggle * Enhanced Monthly chart to display cumulative and monthly data via UI toggle (code and screenshots) * Added screenshots * Update README.md, remove previous screenshots and add new screenshot * Remove test database from tracking and add to .gitignore

antamy added 4 commits October 11, 2025 18:37

Enhanced Monthly chart to display cumulative and monthly data via UI …

39ed271

…toggle

Enhanced Monthly chart to display cumulative and monthly data via UI …

2dc9872

…toggle (code and screenshots)

Added screenshots

8db1609

Merge branch 'feature/add-chart-monthly-toggle' of https://github.com…

7555874

…/antamy/wheeler into feature/add-chart-monthly-toggle

MarkT1065 requested changes Oct 26, 2025

View reviewed changes

antamy added 2 commits October 26, 2025 08:52

Update README.md, remove previous screenshots and add new screenshot

3d4dc19

Remove test database from tracking and add to .gitignore

7a51843

MarkT1065 merged commit 30d5672 into MarkT1065:main Oct 26, 2025

Conversation

antamy commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarkT1065 commented Oct 13, 2025

Uh oh!

MarkT1065 commented Oct 13, 2025

Uh oh!

antamy commented Oct 13, 2025

Uh oh!

antamy commented Oct 23, 2025 via email

Uh oh!

MarkT1065 commented Oct 23, 2025 via email

Uh oh!

MarkT1065 commented Oct 26, 2025

Uh oh!

MarkT1065 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarkT1065 Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

antamy Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

MarkT1065 Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

antamy Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

MarkT1065 commented Oct 26, 2025

Uh oh!

MarkT1065 commented Oct 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

antamy commented Oct 11, 2025 •

edited

Loading

MarkT1065 left a comment •

edited

Loading