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

[Unified Search] Limit height of query bar input for long KQL queries #193737

Merged
merged 14 commits into from
Oct 13, 2024
Merged

[Unified Search] Limit height of query bar input for long KQL queries #193737

merged 14 commits into from
Oct 13, 2024

Conversation

linghaoSu
Copy link
Contributor

@linghaoSu linghaoSu commented Sep 23, 2024 •
edited by kibanamachine
Loading

Summary

Fixes #190067

Add max-height to limit the search height of unified query to avoid overflow caused by long KQL query.

image

@linghaoSu linghaoSu requested a review from a team as a code owner September 23, 2024 15:18
@ryankeairns ryankeairns requested a review from a team September 23, 2024 16:16
@linghaoSu linghaoSu force-pushed the fix/unified-search-max-height branch from c2f5290 to 932aa34 Compare September 24, 2024 01:15
@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

nickofthyme
nickofthyme reviewed Sep 24, 2024
View reviewed changes
Copy link
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

@linghaoSu Thanks for opening this PR!

src/plugins/unified_search/public/query_string_input/query_string_input.scss Show resolved Hide resolved
@nickofthyme nickofthyme added release_note:fix backport:all-open Backport to all branches that could still receive a release backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:all-open Backport to all branches that could still receive a release labels Sep 24, 2024
@linghaoSu linghaoSu force-pushed the fix/unified-search-max-height branch from ea38b21 to 29fed4e Compare September 25, 2024 15:46
@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

@MichaelMarcialis

This comment was marked as resolved.

Sign in to view
@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

@linghaoSu
Copy link
Contributor Author

linghaoSu commented Oct 2, 2024 •
edited
Loading

@MichaelMarcialis Thank you for your review!
1 & 3: They may be the same issue. The collision disappeared after I applied the EUI style scrollbar. I can’t use useEuiScrollBar since this is a class component, so I can only apply the class eui-scrollBar.

As for 2, I have limited the height of the search and clear icons, ensuring they are always positioned at the top.

If you have any other suggestions, please let me know! Thanks!

MichaelMarcialis
MichaelMarcialis approved these changes Oct 7, 2024
View reviewed changes
Copy link
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

Looks good from my perspective. Thanks for making these changes, @linghaoSu!

@linghaoSu
Copy link
Contributor Author

@elasticmachine merge upstream

@mbondyra mbondyra added Team:Visualizations Visualization editors, elastic-charts and infrastructure v9.0.0 v8.16.0 labels Oct 11, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

mbondyra
mbondyra approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

Tested on Chrome and it works great! Thank you for contribution!

nickofthyme

This comment was marked as resolved.

Sign in to view

nickofthyme
nickofthyme approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

Looks great 🎉

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

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
unifiedSearch 349.9KB 350.2KB +267.0B

@nickofthyme nickofthyme changed the title [Unified Search]: add max-height for unified search query limit query input height [Unified Search] Limit height of query bar input for long KQL queries Oct 13, 2024
@nickofthyme nickofthyme merged commit d6e2b26 into elastic:main Oct 13, 2024
25 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11315650553

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 13, 2024
@linghaoSu
[Unified Search] Limit height of query bar input for long KQL queries (…
84ff416 
…elastic#193737)

(cherry picked from commit d6e2b26)
@kibanamachine kibanamachine mentioned this pull request Oct 13, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 13, 2024
@kibanamachine @linghaoSu
[8.x] [Unified Search] Limit height of query bar input for long KQL q…
a4795c3 
…ueries (#193737) (#196027)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Unified Search] Limit height of query bar input for long KQL queries
(#193737)](#193737)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Linghao
Su","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-13T15:39:18Z","message":"[Unified
Search] Limit height of query bar input for long KQL queries
(#193737)","sha":"d6e2b266e81e857a30c7976c2622634751978a30","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Visualizations","💝community","v9.0.0","backport:prev-minor","v8.16.0"],"title":"[Unified
Search] Limit height of query bar input for long KQL
queries","number":193737,"url":"https://github.com/elastic/kibana/pull/193737","mergeCommit":{"message":"[Unified
Search] Limit height of query bar input for long KQL queries
(#193737)","sha":"d6e2b266e81e857a30c7976c2622634751978a30"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193737","number":193737,"mergeCommit":{"message":"[Unified
Search] Limit height of query bar input for long KQL queries
(#193737)","sha":"d6e2b266e81e857a30c7976c2622634751978a30"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Linghao Su <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickofthyme nickofthyme nickofthyme approved these changes

@mbondyra mbondyra mbondyra approved these changes

@MichaelMarcialis MichaelMarcialis MichaelMarcialis approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) 💝community release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Unified Search] Really long KQL queries cause the query bar to overflow the page and become difficult to edit
6 participants
@linghaoSu @MichaelMarcialis @elasticmachine @kibanamachine @mbondyra @nickofthyme