Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugins: Fix sorting issue with expandable rows #75553

Merged
merged 10 commits into from
Oct 12, 2023

Conversation

fabrizio-grafana
Copy link
Contributor

@fabrizio-grafana fabrizio-grafana commented Sep 27, 2023

When rows are expandable and sortable, the two features currently clash, as reported in this escalation. This PR fixes such issue.

If we like this approach, we can then iterate to add tests.

Fixes #74443

@grafana-delivery-bot grafana-delivery-bot bot added this to the 10.2.x milestone Sep 27, 2023
@fabrizio-grafana fabrizio-grafana changed the title Fix sorting issue with expandable rows Plugins: Fix sorting issue with expandable rows Sep 27, 2023
@fabrizio-grafana fabrizio-grafana self-assigned this Sep 27, 2023
@fabrizio-grafana fabrizio-grafana requested a review from a team September 29, 2023 11:26
@fabrizio-grafana fabrizio-grafana marked this pull request as ready for review September 29, 2023 11:26
@fabrizio-grafana fabrizio-grafana requested review from a team, oscarkilhed and mdvictor and removed request for a team September 29, 2023 11:26
@oscarkilhed
Copy link
Contributor

Would be nice to have a test for this since manual testing with nested data is a bit of a hassle currently. Table.test.tsx has a test for nested data that would be fairly good as a starting point.

@fabrizio-grafana fabrizio-grafana requested review from aocenas and a team October 2, 2023 13:44
@fabrizio-grafana
Copy link
Contributor Author

@aocenas tried to clarify the code to address your comments. Let me know if you think it is enough!

@fabrizio-grafana
Copy link
Contributor Author

Added a test for the main scenario of expansion and sorting of rows, the one that was causing the bug fixed in this PR.

@fabrizio-grafana fabrizio-grafana merged commit 6e0825d into main Oct 12, 2023
18 checks passed
@fabrizio-grafana fabrizio-grafana deleted the tempo/fix-row-sorting-2 branch October 12, 2023 08:39
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tempo] Sorting columns with expanded trace breaks the UI
4 participants