feat(web): Display preliminary instead of estimated when using TSA #6905
Conversation
| (day) => day.meta.estimationMethod || day.meta.estimatedPercentage === 100 | ||
| ); | ||
|
|
||
| const allTimeSlicerAverageMethod = chartData.every( | ||
| (day) => day.meta.estimationMethod === EstimationMethods.TSA | ||
| ); |
There was a problem hiding this comment.
I'm wondering if it doesn't start to make sense to create a custom function for these instead so we only loop over it once and set different Boolean flags instead?
Or at the very least re arrange these so the TSA is checked first and then the early TSA return then the rest of the function as it were.
| ? t(`estimation-card.aggregated_estimated.${field}`, { | ||
| percentage: estimatedPercentage, | ||
| }) | ||
| : t(`estimation-card.${estimationMethod?.toLowerCase()}.${field}`); |
There was a problem hiding this comment.
Should we change the enums to be lower case by default so we don't need to call toLowerCase()?
There was a problem hiding this comment.
Sometimes they are taken from the api object and are uppercase or lowercase, I'll make it so the translations and enums match the casing but I think in the future could improve this by matching this as one case between front and backend.
…-to-say-preliminary-for
… casing to translation keys
…-to-say-preliminary-for
| function analyzeChartData(chartData: AreaGraphElement[]) { | ||
| let estimatedCount = 0; | ||
| let tsaCount = 0; | ||
|
|
||
| for (const day of chartData) { | ||
| if (day.meta.estimationMethod === EstimationMethods.TSA) { | ||
| tsaCount++; | ||
| } | ||
| if (day.meta.estimationMethod || day.meta.estimatedPercentage === 100) { | ||
| estimatedCount++; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| allTimeSlicerAverageMethod: tsaCount === chartData.length, | ||
| allEstimated: estimatedCount === chartData.length, | ||
| hasEstimation: estimatedCount > 0, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Looks way better than all the redundat loops!
There was a problem hiding this comment.
The functional evangelist in me really struggled typing let 😆
…-to-say-preliminary-for
|
/review
" |
PR Reviewer Guide 🔍
Code feedback:
|
That was literally one things we fixed, not an issue to be reviewed... |
tonypls
deleted the
tonyvanswet/avo-282-update-estimation-pill-to-say-preliminary-for
branch
|
Was this a accidental change we made here? |
Good spotting, I'll look into it! |
Fix here: #6953 |
Issue
We have changed the estimation card to read Preliminary for time slicer average data but not the rest of the badges
Description
This PR updates the chart badges to display "Preliminary" instead of "Estimated" when the estimation method is Time Slicer Average.
The map tooltip data does not have the estimation method data to conditionally display here as it only gets estimated as a boolean. Setting this as preliminary for hour would not be accurate for mode reconstruct breakdown zones.