Jaden taking over for Carlos/Shravan fix conflicts 4351 (DONE Jaden) - #5437
Jaden taking over for Carlos/Shravan fix conflicts 4351 (DONE Jaden)#5437Jaden300 wants to merge 15 commits into
Conversation
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
9abfb9e to
96d5252
Compare
AdiDubbs
left a comment
AdiDubbs
left a comment
There was a problem hiding this comment.
The cleanup makes this much easier to review, but there is one runtime blocker.
InjuryCategoryBarChart removes setChartKey from the state destructure while continuing to call it after loading, which throws before the chart can settle. The new shared select-style helper is also only wired into SupplierPerformanceGraph while the touched injury chart retains separate dark-mode styles.
The branch is currently 30 commits behind development, so please rebase and retest the chart after fixing the runtime error.
|
|
||
| // ✅ NEW: key to force Recharts remount when needed (fixes "renders only on hover") | ||
| const [chartKey, setChartKey] = useState(0); | ||
| const [chartKey] = useState(0); |
There was a problem hiding this comment.
setChartKey is still called on line 153, but the setter has been removed from this state destructure. Once loading finishes, the animation-frame callback throws ReferenceError: setChartKey is not defined. Please either restore the setter or remove the remount effect and key={chartKey} together.
| @@ -0,0 +1,78 @@ | |||
| /** | |||
There was a problem hiding this comment.
Nice abstraction, but only SupplierPerformanceGraph imports this helper so far. The touched InjuryCategoryBarChart still uses its own react-select control, menu, and option styles with different dark-mode colours. Either migrate that chart as part of this shared-style work or narrow the docstring so it does not describe a single source of truth that is not yet in use.
ShreyasB02
left a comment
ShreyasB02
left a comment
There was a problem hiding this comment.
Tested the changes locally; here are the suggested changes.
- In the Trend Summary Table’s risk-profile section, highlighted rows in dark mode render their text unreadable due to insufficient contrast; the highlighted text should remain clearly visible.
- Issues Breakdown Section: Under the Total Material Cost Per Project graph, the greyed-out text is not clearly visible and is hard to read in dark mode
- Under the financials section, the date field still has a white background, and the text is not visible in dark mode in those fields.
- Under the Global Distribution and Project Status Overview, the start date and end date fields do not support dark mode. The text is completely invisible.
- Under labor time and tracking, under the paid labor section, the background color of the date field is white. Needs to be changed to dark mode like the other fields.
Adit0717
left a comment
Adit0717
left a comment
There was a problem hiding this comment.
Tested on shravan-fix-conflicts-4351. Dark mode toggle works, but need fixes for some elements before merge:
-
Trend Summary Table - row hover breaks text contrast, hovering a row in the Trend Summary Table lights the row background white/light-blue, making the row unreadable.
-
Hover tooltips (e.g. on the Trend Summary Table) are either: - Cut off at the edge, truncating the tooltip text, making both the tooltip and the row behind it hard to read.
-
Date input boxes don't switch to dark mode - They stay white with black text even when the rest of the page is dark. This shows up on: Financials → Cost Variance Trend, Cost Breakdown by Category, Labor and Time Tracking → Paid Labor Cost
There was a problem hiding this comment.
Tested this locally. Found some issues
The map becomes illegible of sorts. The contrast in colors does not make it easy to read the text on the map.
The labour cost chart seems to be getting compressed and text and the graph itself is not easy to get insights from. Not sure if this is from some missed files during merge from main.
Some of the colors here make the text hard to make out. Using slightly darker shades of those colors like the yellow or some of the pinks could help with this.
Another instance of the chart getting compressed to a very small size resulting in making it hard to reach the chart.
Checked out the rest of the changes and they work well.
https://github.com/user-attachments/assets/ecc3e7a2-3f07-4986-83ed-7b1568cf4d9d
…ndering across BMDashboard
- restore setChartKey setter in InjuryCategoryBarChart (ReferenceError fix) - migrate InjuryCategoryBarChart select styles to buildChartSelectStyles - add !important to darkDateInput in PaidLaborCost and CostVarianceTrendGraph - add dark date picker class to QuantityOfMaterialsUsed JSX and CSS - add tr:hover contrast rules for dark mode in ProjectRiskProfileOverview
68d6337 to
11cc64c
Compare
|
linlin-husky
left a comment
linlin-husky
left a comment
There was a problem hiding this comment.
Tested locally on branch shravan-fix-conflicts-4351 in Dark Mode as an Admin user.
Confirmed multiple dark mode styling and contrast defects across different sections:
-
Calendar Date Picker Popups (Lessons Learned & Global Distribution):
- Lessons Learned (Injury Severity by Category of Worker Injured): Clicking on
Start date/End dateopens a calendar popup with a solid white background and invisible white text. - Global Distribution and Project Status Overview: Clicking on
Start Date/End Dateopens a calendar popup with a solid white background where the month header, weekday labels, and dates render in white text, making them completely unreadable.
- Lessons Learned (Injury Severity by Category of Worker Injured): Clicking on
-
Date Input Fields (Financials -> Cost Breakdown by Category):
- The
FromandTodate picker inputs retain a solid white background in Dark Mode with invisible/white text.
- The
-
Reset Filter Button & Filter Inputs Contrast (Financials -> Planned vs Actual Cost):
- The
Reset Filterbutton renders with a bright white background and light/white text, creating a severe contrast issue. - The filter input and select fields in this section also render with solid white backgrounds in Dark Mode, leading to text legibility issues.
- The
Please see the attached screenshots and ensure consistent dark mode styling across tables, input fields, popup calendars, and buttons.
9 participants
Jaden taking over for Carlos/Shravan fix conflicts 4351 (DONE Jaden)
This is a redo integrating the feedback on this closed PR: #4954
Description
Took over from Carlos/Shravan to bring PR #4351 (dark mode implementation for the Total Construction Summary page) to a mergeable state.
The previous branch had accumulated ~212 files due to repeated
git merge developmentcalls pulling in unrelated work from other PRs. This PR resets to a clean base offdevelopmentand cherry-picks only the 22 files belonging to Carlos's original dark mode work, then resolves all blocking issues.Related PRs
Changes made
Branch cleanup
development- removed ~190 passenger files from other PRs that had accumulated via merge commitsBug fixes
ENDPOINTS.BM_PROJECTS_LIST_FOR_MATERIALS_COST- did not exist - changed toENDPOINTS.BM_PROJECTS)projectId/projectName- changed to_id/nameto match actual API response)issueCharts.css,DarkMode.css,autoComplete.css) that pointed to deleted or non-existent filesStyling
word-break: break-word, deprecated@document, invalidprefers-contrast: highCode quality
typeattributes, array index keys, mutating.sort()replaced with.toSorted(), optional chaining, unused imports, nested ternaries,.removeChildreplaced with.remove()ToolStatusDonutChartcognitive complexity from 25 to under 15 by extractinggetChartDimensions,NoDataView, andDonutChartViewTests
TotalMaterialCostPerProject.test.jsxnow passLeaderboard.test.jsxpassHow to test
shravan-fix-conflicts-4351npm installandnpm run start:localbmdashboard/totalconstructionsummaryvid.mov