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(slo): search bar #166945

Merged
merged 3 commits into from
Sep 21, 2023
Merged

fix(slo): search bar #166945

merged 3 commits into from
Sep 21, 2023

Conversation

kdelemme
Copy link
Contributor

Resolves #166689
Resolves #166683

🍒 Summary

This PR fixes two bugs:

  1. When the auto refetch was triggered, the search bar was disabled, resulting in a lost of focus if the user was actively typing in the search bar.
  2. When submitting a search query, we were forcing refetch but doing so was using the previous state of the query. Instead, we only update the query state, and let react react hook query trigger a refetch based on the state update.

@kdelemme kdelemme self-assigned this Sep 21, 2023
@kdelemme kdelemme added release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.11.0 labels Sep 21, 2023
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@kdelemme kdelemme marked this pull request as ready for review September 21, 2023 15:12
@kdelemme kdelemme requested a review from a team as a code owner September 21, 2023 15:12
@elasticmachine
Copy link
Contributor

Pinging @elastic/actionable-observability (Team: Actionable Observability)

@mgiota mgiota self-requested a review September 21, 2023 15:20
Copy link
Contributor

@mgiota mgiota left a comment

Choose a reason for hiding this comment

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

Only code review LGTM! I will check locally as well

@mgiota
Copy link
Contributor

mgiota commented Sep 21, 2023

I tested it and what you describe works fine. But look what happens when I detele the search query. I would expect to get back the list of all slos. Is it something we control? This behaviour is also on main, so I guess we can create a separate issue for this.

Screen.Recording.2023-09-21.at.17.49.44.mov

@kdelemme
Copy link
Contributor Author

@mgiota Yes i've noticed that before. Indeed this is not related to this fix. In that case, the problem is the QueryStringInput component does not trigger the onSubmit callback when clicking on the "clear" icon.
I need to see if I can bubble up the submit event, and catch it just above to trigger manually the onsubmit.

@kdelemme
Copy link
Contributor Author

I've improved the fix.

search.mov

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
observability 1.0MB 1.0MB +1.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @kdelemme

@mgiota
Copy link
Contributor

mgiota commented Sep 21, 2023

Looks great!

@kdelemme kdelemme merged commit 45c9cd0 into elastic:main Sep 21, 2023
19 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 21, 2023
@kdelemme kdelemme deleted the fix/slo-search-bar branch September 21, 2023 18:55
kdelemme added a commit to kdelemme/kibana that referenced this pull request Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:SLO release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.11.0
Projects
None yet
6 participants