545 graph bug #552
545 graph bug #552
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
egsch
left a comment
egsch
left a comment
There was a problem hiding this comment.
Hey @OddEyed45 sorry for the late review--this looks good! Thank you and good job finding a way to prevent the errors despite some of the weirder constraints of ApexCharts. I did leave a couple of little questions so if you could check those out that would be fantastic. But great work overall!
| calculateGrades(course.grades, chosenSemesters), | ||
| )[0], | ||
| ) | ||
| ? [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] |
There was a problem hiding this comment.
Just fyi to make sure we are communicating all possible grade data including cr/nc and pass/fail grades we will soon expand the grade data returned by CalculateGrades to include more than 14 entries. Do you think you could fill to an adaptive length array to prevent assuming we have 14 entries here?
| item.subtitle?.toLowerCase().includes(newInputValue.toLowerCase()) || | ||
| item.title?.toLowerCase().includes(newInputValue.toLowerCase()) | ||
| ) | ||
| searchQueryLabel(item) |
There was a problem hiding this comment.
Was removing the ! here intentional?
Overview
List the GitHub issues containing the issues relevant to this pull request.
Resolves issue #545
Describe your changes here at a high level, describing how this PR fits into the
rest of the project.
It fixed a simple error where the bar chart does not show up properly on the screen under Compare.
What Changed
The issue is fixed by correcting the grade data that is being displayed by the bar graph (under src/components/compare/Compare.tsx). The problem was that if the size of the grade data were 0, the convertNumbersToPercents function would output a list with NaNs since it would not go into the map function. The most direct way to fix this was to check the output of this method (and manually assign an array of 14 zeroes if NaN is found).
Other Notes
At one point, Vercel failed to deploy because the project was using a version of Next.js with a security vulnerability. I have fixed it on my end (hence, Vercel is working) but I am unsure about changing package.json & package-lock.json for the entire project.
Please make sure to fix that (there is a merge conflict here).