fix(Calendar Heatmap): Add Back chart options for Calendar Heatmap #26230
Conversation
Mattc1221
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #26230 +/- ##
==========================================
- Coverage 69.18% 69.18% -0.01%
==========================================
Files 1945 1945
Lines 75948 75950 +2
Branches 8458 8458
==========================================
Hits 52546 52546
- Misses 21217 21219 +2
Partials 2185 2185
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Thanks @Mattc1221 for the change. Could you help provide some more context, i.e., regarding the PR title you state "Add Back ..." which would imply that these features previously existed. If this is the case would you mind referencing the PR(s) where this was removed? |
|
Note ideally we hope to migrate the legacy calendar heatmap chart to ECharts, however currently there isn't feature parity. The concern is if we continue augmenting the legacy chart, the changes of eventually migrating to ECharts further diminishes. |
There was a problem hiding this comment.
Thanks for the PR @Mattc1221 and for writing the SIP. I'm just requesting changes to block this PR from being merged before the SIP is approved.
|
Just noting that the SIP is open for voting. Hoping we can get some votes, and unblock this PR! |
|
Hold label has been removed... @michael-s-molina you can release your hold if it's just for procedural reasons (though the PR still needs review). |
| renderTrigger: true, | ||
| default: undefined, | ||
| label: t('Color Range Start'), | ||
| description: t('test'), |
There was a problem hiding this comment.
Please provide a meaningful description.
| renderTrigger: true, | ||
| default: undefined, | ||
| label: t('Color Range End'), | ||
| description: t('test'), |
| } = formData; | ||
|
|
||
| const { verboseMap } = datasource; | ||
| const timeFormatter = ts => getFormattedUTCTime(ts, xAxisTimeFormat); | ||
| const valueFormatter = getNumberFormatter(yAxisFormat); | ||
|
|
||
| // Get the color range if both are specified | ||
| const useCustomColorRange = |
There was a problem hiding this comment.
Shouldn't we validate for integers instead?
const useCustomColorRange =
isNumber(colorRangeStart) && isNumber(colorRangeEnd);
SUMMARY
SIP: #26229
Adds chart options back to the Calendar Heatmap. The new list of Chart options is:
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION