feat(LabelsView): update main time series when a "group by" label is selected #341
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 46b9147.
# Conflicts: # src/pages/ProfilesExplorerView/components/SceneExploreServiceLabels/components/SceneGroupByLabels/components/SceneLabelValuesGrid/SceneLabelValuesGrid.tsx
# Conflicts: # src/pages/ProfilesExplorerView/domain/actions/SelectAction.tsx
# Conflicts: # docker-compose.e2e.yaml # docker-compose.local.yaml # docker-compose.remote.yaml # docker-compose.yaml # package.json # src/pages/ProfilesExplorerView/components/SceneLabelValuesTimeseries/SceneLabelValuesTimeseries.tsx # src/plugin.json # src/shared/infrastructure/tracking/faro/__tests__/faro.spec.ts # yarn.lock
# Conflicts: # package.json # yarn.lock
grafakus
changed the base branch from
main
to
perf/limit-series-breakdown-charts
…rrentOption" method
grafakus
commented
Jan 22, 2025
| @@ -66,6 +74,32 @@ export class GroupByVariable extends QueryVariable { | |||
| this.changeValueTo(newValue); | |||
| }; | |||
|
|
|||
| findCurrentOption(): OptionWithIndex { | |||
There was a problem hiding this comment.
Added this new method to DRY in SceneGroupByLabels and SceneMainServiceTimeseries
grafakus
commented
Jan 22, 2025
| .map(({ value, text }, index) => { | ||
| return { | ||
| // TODO: check if there's a better way | ||
| value: JSON.stringify({ ...value, index }), |
There was a problem hiding this comment.
Adding the index (after sorting) is the reason behind this change
|
|
ifrost
previously approved these changes
Jan 31, 2025
ifrost
reviewed
Jan 31, 2025
| LabelsDataSource.MAX_TIMESERIES_LABEL_VALUES | ||
| ).state; | ||
|
|
||
| const queryRunner = body.state.$data?.state.$data as SceneQueryRunner; |
There was a problem hiding this comment.
Do we need to drill down to children props? Could we have the runner at SceneLabelValuesTimeseries/SceneLabelValuesTimeseries. The viz should to pull it as the closest anyway.
There was a problem hiding this comment.
That's a good idea. We'd also have to change some code in SceneComparePanel but it will simplify the access to the query runner. Let's do this in a future refactoring PR.
ifrost
previously approved these changes
Jan 31, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✨ Description
Related issue(s): resolves #197, is blocked by #219
This PR makes the main time series in the Labels view responsive to "group by" changes, for example:
And after interacting with the "Include"/"Exclude" buttons (including 3 out of the 52 nodes available):
📖 Summary of the changes
General overview:
SceneExploreServiceLabelsSceneMainServiceTimeseriesreacts to thegroupByvariable changes by updating the item stored inSceneLabelValuesTimeseries's stateSceneLabelValuesTimeseriesrun new queries whenever the item'squeryRunnerParams.groupByvalue changesSee diff tab for specific comments.
🧪 How to test?