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

fix(widget-builder): Clear sorting on field and y-axis changes #83679

Conversation

narsaynorath
Copy link
Member

Sorting isn't necessary when you don't have a grouping, so update the state to set a default when a grouping is added (with priority for the yAxis if it exists) and clear the sort if a yAxis changes and a grouping is not applied.

Sorting isn't necessary when you don't have a grouping, so update the
state to set a default when a grouping is added (with priority for the
yAxis if it exists) and clear the sort if a yAxis changes and a grouping
is not applied.
@narsaynorath narsaynorath requested a review from a team as a code owner January 17, 2025 20:57
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jan 17, 2025
Copy link
Member

@nikkikapadia nikkikapadia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm one question

if (
displayType !== DisplayType.TABLE &&
displayType !== DisplayType.BIG_NUMBER &&
action.payload.length > 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wondering if this gets into the state that it is 0 should we clear the sort completely? My brain is also fried rn so ignore if the answer to this is very obvious 😭

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the endpoint, it's fine if there's a sort and no grouping as long as the sort is one of the other fields (e.g. aggregates)... I'm inclined to keep it because it works a bit more nicely with the default queries at the moment! Although you are right, eventually it's probably cleaner to just remove it and acknowledge that it's not really doing anything

@narsaynorath narsaynorath merged commit ea77199 into master Jan 20, 2025
42 checks passed
@narsaynorath narsaynorath deleted the nar/fix/widget-builder-clear-sorting-on-field-and-yaxis-changes branch January 20, 2025 13:36
andrewshie-sentry pushed a commit that referenced this pull request Jan 22, 2025
Sorting isn't necessary when you don't have a grouping, so update the
state to set a default when a grouping is added (with priority for the
yAxis if it exists) and clear the sort if a yAxis changes and a grouping
is not applied.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants