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.

Sign up for GitHub

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

Jump to bottom

QueryVariable: Use correct option property for variable options sorting #1015

Merged
merged 4 commits into from
Jan 2, 2025
Merged

QueryVariable: Use correct option property for variable options sorting #1015

merged 4 commits into from
Jan 2, 2025

Conversation

Sergej-Vlasov
Copy link
Contributor

@Sergej-Vlasov Sergej-Vlasov commented Dec 24, 2024 •
edited by scenes-repo-bot-access-token bot
Loading

sorting was carried out agains option.text instead of option.label

📦 Published PR as canary version: 5.36.2--canary.1015.12584248290.0

✨ Test out this PR locally via:

npm install @grafana/[email protected]
npm install @grafana/[email protected]
# or 
yarn add @grafana/[email protected]
yarn add @grafana/[email protected]

@Sergej-Vlasov Sergej-Vlasov added type/bug Something isn't working patch Increment the patch version when merged release Create a release when this pr is merged area/variables Issues related to variables system in scenes labels Dec 24, 2024
@Sergej-Vlasov Sergej-Vlasov requested a review from a team December 24, 2024 15:18
@Sergej-Vlasov Sergej-Vlasov self-assigned this Dec 24, 2024
@Sergej-Vlasov Sergej-Vlasov requested review from axelavargas and bfmatei and removed request for a team December 24, 2024 15:18
@Sergej-Vlasov Sergej-Vlasov linked an issue Dec 24, 2024 that may be closed by this pull request
Dashboards: Numerical ascending sort of query variable values is broken grafana/grafana#97881
Closed
@leeoniya
Copy link
Contributor

leeoniya commented Dec 31, 2024 •
edited
Loading

also, for numeric sort you can just subtract the values from each other and skip the if/conditions.

EDIT: actually if your labels are strings you can add numeric: true to the collator, and it will handle proper numeric sort without anything extra

@Sergej-Vlasov Sergej-Vlasov requested a review from leeoniya January 2, 2025 13:56
@leeoniya
Copy link
Contributor

leeoniya commented Jan 2, 2025

i think a lot of the sorting funcs can be removed and/or simplified, like sortByNumeric can likely just rely on the same numeric: true collator. but not in this PR :)

@Sergej-Vlasov Sergej-Vlasov merged commit a5f78be into main Jan 2, 2025
5 checks passed
@Sergej-Vlasov Sergej-Vlasov deleted the serge/fix-numeric-variable-sort branch January 2, 2025 15:45
@scenes-repo-bot-access-token
Copy link

🚀 PR was released in v5.36.2 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leeoniya leeoniya leeoniya approved these changes

@torkelo torkelo torkelo approved these changes

@axelavargas axelavargas Awaiting requested review from axelavargas axelavargas was automatically assigned from grafana/dashboards-squad

@bfmatei bfmatei Awaiting requested review from bfmatei bfmatei was automatically assigned from grafana/dashboards-squad

@ivanortegaalba ivanortegaalba Awaiting requested review from ivanortegaalba

@harisrozajac harisrozajac Awaiting requested review from harisrozajac

Assignees

@Sergej-Vlasov Sergej-Vlasov

Labels
area/variables Issues related to variables system in scenes patch Increment the patch version when merged release Create a release when this pr is merged released type/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dashboards: Numerical ascending sort of query variable values is broken
3 participants
@Sergej-Vlasov @leeoniya @torkelo