refactor: more flexibility with explorer #2548
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2548 +/- ##
==========================================
- Coverage 82.20% 81.93% -0.28%
==========================================
Files 927 928 +1
Lines 20872 20901 +29
Branches 3313 3321 +8
==========================================
- Hits 17158 17125 -33
- Misses 3577 3638 +61
- Partials 137 138 +1 ☔ View full report in Codecov by Sentry. |
| const intervalOptions = this.buildIntervalOptions(); | ||
|
|
||
| if (this.intervalSupported()) { | ||
| this.selectedInterval = this.getBestIntervalMatch(intervalOptions, this.selectedInterval ?? defaultInterval); | ||
| this.intervalOptions = intervalOptions; // The only thing this flag controls is whether options are available (and thus, the selector) | ||
| } else { | ||
| this.selectedInterval = this.getBestIntervalMatch(intervalOptions, defaultInterval); | ||
| // TODO: why do we need to set this if interval is not supported. |
There was a problem hiding this comment.
See comment above. It's poorly named, but this is really switching on whether the interval is selectable rather than supported.
There was a problem hiding this comment.
Yeah, so I want to keep it undefined if the interval is not supported.
There was a problem hiding this comment.
Maybe just terminology; there is an interval, it's just not selectable. Is it just no longer needed if not selectable? If so, is there any harm in sending it in?
There was a problem hiding this comment.
The problem is that this is set even when the data is not a time series. I am using the presence ofintervalData property to decide the legend rendering behavior since we are treating Grouped but not time series data case differently.
| return legendEntry; | ||
| } | ||
|
|
||
| private updateLegendClassesAndStyle(): void { | ||
| const legendElementSelection = select(this.legendElement!); | ||
| if (this.isGrouped) { | ||
| if (true) { |
There was a problem hiding this comment.
need to remove this one.
Description
Please include a summary of the change, motivation and context.
Testing
Please describe the tests that you ran to verify your changes. Please summarize what did you test and what needs to be tested e.g. deployed and tested helm chart locally.
Checklist:
Documentation
Make sure that you have documented corresponding changes in this repository or hypertrace docs repo if required.