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

Prometheus/Loki: Run query explicitly instead of onblur in panel edit #64815

Merged
merged 5 commits into from
Mar 21, 2023

Conversation

torkelo
Copy link
Member

@torkelo torkelo commented Mar 15, 2023

  • Run query explicitly instead of onblur in panel edit
  • Also highlight the Run queries button when a change has been made
Edit-panel---prometheus-query-max---Dashboards---Grafana.webm

@torkelo torkelo changed the title Prometheus: Run query explicitly instead of onblur in panel edit Prometheus/Loki: Run query explicitly instead of onblur in panel edit Mar 15, 2023
@torkelo torkelo requested a review from a team as a code owner March 15, 2023 14:26
@torkelo torkelo added the no-backport Skip backport of PR label Mar 15, 2023
@torkelo torkelo added this to the 9.5.0 milestone Mar 15, 2023
@leeoniya
Copy link
Contributor

leeoniya commented Mar 15, 2023

maybe useful to add an Enter or Shift + Enter or Ctrl + Enter shortcut to run queries, too?

Copy link
Contributor

@svennergr svennergr left a comment

Choose a reason for hiding this comment

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

Loki change LGTM.

(Prom also, but I'll defer to the metrics squad).

maybe useful to add an Enter or Shift + Enter or Ctrl + Enter shortcut to run queries, too?

@leeoniya yea, Shift+Enter is already working to run queries.

@github-actions
Copy link
Contributor

Backend code coverage report for PR #64815
No changes

@github-actions
Copy link
Contributor

github-actions bot commented Mar 15, 2023

Frontend code coverage report for PR #64815

Plugin Main PR Difference
loki 83.61% 83.66% .05%

@@ -76,7 +76,7 @@ export class LokiQueryField extends React.PureComponent<LokiQueryFieldProps, Lok
>
<div className="gf-form gf-form--grow flex-shrink-1 min-width-15">
<MonacoQueryFieldWrapper
runQueryOnBlur={app !== CoreApp.Explore}
runQueryOnBlur={false}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can safely remove this prop and all related code, since it was only used for the on-blur run in dashboards.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done removed this prop for both the prometheus and loki variant

@torkelo
Copy link
Member Author

torkelo commented Mar 15, 2023

@leeoniya I think they already exist

Copy link
Contributor

@matyax matyax left a comment

Choose a reason for hiding this comment

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

🚀

@itsmylife
Copy link
Contributor

itsmylife commented Mar 20, 2023

Personally, I like the onBlur behavior maybe because I am used to it. And I see we add this in the changelog so users like me will be aware of that change 🤞. Highlighting the Run queries button is also a good touch.
So PR LGTM but I wonder if we have any user complaints related to OnBlur behavior or any issues we encountered.

@torkelo
Copy link
Member Author

torkelo commented Mar 21, 2023

So PR LGTM but I wonder if we have any user complaints related to OnBlur behavior or any issues we encountered.

@itsmylife only from internal users so far, and some comments here #51979

@torkelo torkelo merged commit 395890c into main Mar 21, 2023
@torkelo torkelo deleted the prometheus-run-on-change branch March 21, 2023 06:55
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.

None yet

6 participants