-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
feat(dashboards): Dashboard Widgets Open in Discover button redirects with all selected Y-Axis from the Widget #28637
feat(dashboards): Dashboard Widgets Open in Discover button redirects with all selected Y-Axis from the Widget #28637
Conversation
…er-multi-y-axis-checkbox-dropdown
update to allow deselecting all y axis added y axis selection restriction based on plot type default to count() on empty y axis fixed tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
smol question about how we set yAxis, but regardless LGTM
@@ -38,6 +38,9 @@ class DashboardWidgetQuerySelectorModal extends React.Component<Props> { | |||
selection, | |||
widget.displayType | |||
); | |||
const discoverLocation = eventView.getResultsViewUrlTarget(organization.slug); | |||
// Pull Y-Axis from the widget | |||
discoverLocation.query.yAxis = query.fields; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible for query.fields
to contain yAxis incompatible fields here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, I've added some checks to ensure we only use compatible fields via eventView.getYAxisOptions
and also made sure to limit to 3 yAxis max as the table widget allows 3+ fields to be selected..
…en-in-discover-multi-y-axis
Updates the
Open in Discover
links in Dashboard Widgets to redirect with all Y-Axis selected from the Widget rather than using only the first Y-Axis from the Widget.