Closed ajmenca closed 2 months ago
The changes enhance the TimeSeriesChart
component by introducing a new story that features a bar chart with visual thresholds, improving data representation. The rendering logic for color handling has been streamlined by removing certain functions and modifying attribute definitions for greater clarity. Additionally, chart helper functions have been updated to better manage gradient colors, and the MultiSeriesBarChartOptions
interface has been adjusted to accommodate new threshold data structures, increasing the component's flexibility and usability.
Files | Change Summary |
---|---|
src/components/presentational/TimeSeriesChart/... |
Added a new story barChartWithThresholds showcasing a bar chart with thresholds and dynamic data. |
src/components/presentational/TimeSeriesChart/... |
Removed color handling functions and modified fill/stroke logic for clarity in chart rendering. |
src/helpers/chartHelpers.tsx |
Updated gradient color definitions in createBarChartDefs and createAreaChartDefs for better styling. |
src/helpers/chartOptions.ts |
Changed thresholds property type in MultiSeriesBarChartOptions from BarChartThreshold to ChartThreshold . |
src/helpers/chartOptions.ts (1)
`73-73`: **Update the type of `thresholds` property.** The type of `thresholds` in the `MultiSeriesBarChartOptions` interface has been updated to `ChartThreshold[]`. Ensure that all instances of `MultiSeriesBarChartOptions` are compatible with this change.src/helpers/chartHelpers.tsx (2)
`83-84`: **Update the gradient definitions for bar charts.** The `overThresholdBarColor` property has been replaced with `overThresholdColor` to define the gradient stops. This change ensures consistency in color usage for thresholds in bar charts. --- `98-107`: **Enhance gradient definitions for area charts.** Separate linear gradients are now created for the fill and stroke of each series in area charts. This allows for distinct styling of the area and stroke, improving the chart's appearance.src/components/presentational/TimeSeriesChart/TimeSeriesChart.tsx (1)
`257-258`: **Update fill and stroke attributes for area charts.** The fill and stroke attributes now use modified gradient keys (`-fill` and `-stroke`). This change enhances the differentiation between fill and stroke styles.
Overview
I noticed two coloring bugs in the Time Series Chart and Daily Data Chart.
The first is that this story for the TimeSeriesChart was clearly inverted:
The second is that this threshold story on the Daily Data Chart doesn't line up either; things are defaulting to the warning color instead of taking the "green" and "red" values for
overThresholdBarColor
The first problem was fixed by correcting two small things. The first was that the code was simply setting the stroke color based on the area color, and the code that was generating the fill gradient was using the
color
property instead of theareaColor
property to define it. I think this bug crept in all the way back when I implemented the TimeSeriesChart in the first place. I also made it so that it actually makes gradient definitions for both (the way that even a normal Line Chart technically uses a gradient), so that if we try and ever put Thresholds into the Area Chart we've got the ground work.The second problem was caused by an improper type definition in the
MultiSeriesBarChartOptions
declaring thethresholds
property asBarChartThreshold
instead ofChartThreshold
. TheDailyDataChart
was doing this conversion already (renamingoverThresholdBarColor
tooverThresholdColor
) but since the type was wrong (and strangely the code inDailyDataChart
doesn't cause a compilation error even though it attempts to explicitly state the return type and there should have been a mismatch), the code creating the gradient definitions was looking for the wrong property. We also didn't have a story on the baseTimeSeriesChart
demonstrating this behavior so it slipped past. This slipped in here somehow https://github.com/CareEvolution/MyDataHelpsUI/pull/247/files#diff-a68632464648485a258a95a49e5707d140e4d6c1ee3d52c7ffd6fe751d89f840These stories now look like this:
and this:
Security
REMINDER: All file contents are public.
Describe briefly what security risks you considered, why they don't apply, or how they've been mitigated.
Checklist
Testing
Documentation
Consider "Squash and merge" as needed to keep the commit history reasonable on
main
.Reviewers
Assign to the appropriate reviewer(s). Minimally, a second set of eyes is needed ensure no non-public information is published. Consider also including:
Summary by CodeRabbit
New Features
TimeSeriesChart
component showcasing a bar chart with thresholds for better visual representation.Improvements
TimeSeriesChart
.Changes
thresholds
inMultiSeriesBarChartOptions
for better data structure alignment.