Ganesh implements a loss tracking line graph#3876
Ganesh implements a loss tracking line graph#3876karnati007 wants to merge 9 commits intodevelopmentfrom
Conversation
…ighestGoodNetworkApp into Ganesh_impl_loss_tracking_line_graph
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
I navigated to Dashboard → Reports → Total Construction Summary → Financials (Loss Tracking Line Chart). Verified that all filters work correctly and the dark mode displays properly without issues. Everything looks good! PR_3876.mp4 |
ShravyaKudlu
left a comment
There was a problem hiding this comment.
Wow this looks great, and better then before, I have a few observations
- in dark mode the graph x-axis and yaxis and datapoints is black, which isnt seen clearly, if there is any way to change that
2. in light mode the reset filter button is not seen
3. from and to? what kind of datapoints were expected from user?
|
Navigated to to dashboard→ Reports→ Total Construction Summary→ Financials (Loss Tracking Line Chart); reviewed for both light and dark mode. Observations: |
Naveen-Kumar-Reddy-27
left a comment
There was a problem hiding this comment.
Tested PR #3876 locally against development. Verified UI and identified the following issues:
Dark Mode Text Visibility
Low contrast makes text under "Planned vs Actual Cost" (e.g., date fields, project filters) hard to read.

Light Mode Button Contrast
The button under "Loss Tracking Line Chart" blends into the light background.
Missing "Reset Filters" Button
"Planned vs Actual Cost" lacks a "Reset Filters" button, unlike the Loss Tracking section.

Padding/Alignment Issues
Adding some space or padding between the X-axis label "Time (months)" and the year legends (2022, 2023, 2024) would improve readability and overall look.
SallaguntaRaahul
left a comment
There was a problem hiding this comment.
I reviewed PR-3876.
In light mode, the Reset Filters button is not clearly visible and should have higher contrast.

For each material, only one year of data is currently being returned. This means a material shows data tied to a single year, but ideally each material should support data and tracking across multiple years, not just one.
Also, I noticed stray “📊 Card” and “📊 Big Card” labels at the bottom of the dashboard that look like leftover demo components—these should be removed to avoid UI clutter.
On the line chart, the x-axis label (“Time (months) – 2022”) overlaps with the tick values, which makes it hard to read. This should be repositioned or styled differently
In dark mode, the same issues persist (year–material restriction, x-axis label overlap, stray demo labels), and in addition the text labels in the Planned vs Actual Cost chart are rendered in dark colors, which makes them nearly invisible against the dark background.

aseemdeshmukh
left a comment
There was a problem hiding this comment.
Navigated to to dashboard→ Reports→ Total Construction Summary→ Financials (Loss Tracking Line Chart); reviewed for both light and dark mode.
Observations:
- For Light mode, "Reset Filters" button text is not clearly visible under Loss Tracking Line Chart
- For Dark mode, "Planned vs Actual Cost" text is not clearly readable
- There should be "Reset Filters" button for "Planned vs Actual Cost" like the existing button under "Loss Tracking Line Chart"
vishnu-ing
left a comment
There was a problem hiding this comment.
The PR has been reviewed for code changes and the functionality is working good which has been tested in local. As per the suggestions in previous comments in PR, there can be some changes in text color in dark and light modes to make the UI better. No new review ccomments, Everything has been covered in previous comments. Attaching screenshots for current UI with graph data and filters applied in different modes.
a5a22d0 to
2bb7cbd
Compare
RitzzzZ2021
left a comment
There was a problem hiding this comment.
I navigated to Dashboard → Reports → Total Construction Summary → Financials (Loss Tracking Line Chart) as required. When I applied filters for different materials, it instead filtered by year. There are also no tags available for materials. Additionally, the chart design could be improved to make it clearer for readers.
aryanrachala54
left a comment
There was a problem hiding this comment.
The core functionality looks good, and filters are mostly working as expected. Before merging, a few key issues still need to be addressed:
- Improve text contrast for better readability in both light and dark modes (e.g., axis labels, "Reset Filters" button).
- Fix material filter logic (currently filters by year).
- Add "Reset Filters" button to the "Planned vs Actual Cost" section.
- Remove stray “📊 Card” components.
- Add validation to ensure start date is before end date.
Fixes from PR #3876 review + code improvements: 1. Critical bug fix: mergedData key was `line.year` (number) but <Line dataKey> expected `${line.year}-${line.material}` — multiple materials of the same year were silently overwriting each other in the chart data. 2. Wrap mergedData/chartData computation in useMemo (deps: filteredLines, startDate, endDate) to avoid unnecessary recalculation on every render. 3. Expanded color palette to support 9 year-material combinations: Teal shades for 2022, Pink shades for 2023, Yellow/Orange for 2024. 4. Expanded mock rawData to include all 3 materials (Metal, Plastic, Glass) for each of the 3 years (2022-2024), giving 9 distinct lines. 5. Moved date-range validation error message out of the <label> element and added .dateRangeError CSS class (grid-column: 1 / -1 for full width). 6. Applied Prettier formatting (auto-fixed via eslint --fix). All dark mode axis/tick colors already handled via dynamic textColor; Reset Filters button visibility handled via CSS module variables. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Redone with this PR: #5016 |
1 similar comment
|
Redone with this PR: #5016 |

































Description
Related PRS (if any):
…
Main changes explained:
…
How to test:
npm installandnpm run start:localto run this PR locally.Screenshots or videos of change
Light Theme:
https://github.com/user-attachments/assets/20fc4c25-78b7-4f93-add5-b4274486596e
Dark Theme:
