-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[ML] Add border to text editor for ES|QL data visualizer #192726
Conversation
Pinging @elastic/ml-ui (:ml) |
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.
LGTM ⚡
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.
LGTM.
Were you planning on combining this styling edit with a fix for the top values content overflowing too? #192637
@@ -168,7 +168,8 @@ export const TopValues: FC<Props> = ({ | |||
return ( | |||
<ExpandedRowPanel | |||
dataTestSubj={'dataVisualizerFieldDataTopValues'} | |||
className={classNames('dvPanel__wrapper', compressed ? 'dvPanel--compressed' : undefined)} | |||
css={css({ width: '95%', overflow: 'hidden' })} |
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.
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.
Looked into this but it's a bit of a rabbit hole. Noticed:
- the flex layout seems not correct because the +/- filter buttons are not wrapped in FlexItem.
- not sure but I think the progress bar does some "magical" animated resize on its own, i'm not sure if that messes with the sizing
- tried to apply EUI's
EuiTextTruncate
on the progress label but couldn't get it to work.
I wonder if the whole thing should be a table instead of flex layout?
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.
LGTM. Scrollbar for the top values seems a sensible solution to me.
@elasticmachine merge upstream |
@elasticmachine merge upstream |
merge conflict between base and head |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @qn895 |
) ## Summary This PR adds border to text editor for ES|QL data visualizer so make it more clear where the boundaries for the editor is. <img width="1505" alt="image" src="https://github.com/user-attachments/assets/264f4892-f361-4013-b49f-55fa5b90aa8f"> Also fixes the content of the top values overflowing. https://github.com/user-attachments/assets/19c90cb3-2d35-41df-aea4-889a00e6ebdb Fixes elastic#192788 Fixes elastic#192637 --------- Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 486b16e)
💔 Some backports could not be created
Note: Successful backport PRs will be merged automatically after passing CI. Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
…) (#193621) # Backport This will backport the following commits from `main` to `8.x`: - [[ML] Add border to text editor for ES|QL data visualizer (#192726)](#192726) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Quynh Nguyen (Quinn)","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-20T16:45:22Z","message":"[ML] Add border to text editor for ES|QL data visualizer (#192726)\n\n## Summary\r\n\r\nThis PR adds border to text editor for ES|QL data visualizer so make it\r\nmore clear where the boundaries for the editor is.\r\n\r\n<img width=\"1505\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/264f4892-f361-4013-b49f-55fa5b90aa8f\">\r\n\r\nAlso fixes the content of the top values overflowing.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/19c90cb3-2d35-41df-aea4-889a00e6ebdb\r\n\r\n\r\nFixes https://github.com/elastic/kibana/issues/192788\r\nFixes https://github.com/elastic/kibana/issues/192637\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"486b16e56f76ba0124b80b564f8516e2e22bc37b","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":[":ml","release_note:skip","v9.0.0","backport:prev-major","v8.16.0","backport:current-major"],"title":"[ML] Add border to text editor for ES|QL data visualizer","number":192726,"url":"https://github.com/elastic/kibana/pull/192726","mergeCommit":{"message":"[ML] Add border to text editor for ES|QL data visualizer (#192726)\n\n## Summary\r\n\r\nThis PR adds border to text editor for ES|QL data visualizer so make it\r\nmore clear where the boundaries for the editor is.\r\n\r\n<img width=\"1505\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/264f4892-f361-4013-b49f-55fa5b90aa8f\">\r\n\r\nAlso fixes the content of the top values overflowing.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/19c90cb3-2d35-41df-aea4-889a00e6ebdb\r\n\r\n\r\nFixes https://github.com/elastic/kibana/issues/192788\r\nFixes https://github.com/elastic/kibana/issues/192637\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"486b16e56f76ba0124b80b564f8516e2e22bc37b"}},"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/192726","number":192726,"mergeCommit":{"message":"[ML] Add border to text editor for ES|QL data visualizer (#192726)\n\n## Summary\r\n\r\nThis PR adds border to text editor for ES|QL data visualizer so make it\r\nmore clear where the boundaries for the editor is.\r\n\r\n<img width=\"1505\" alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/264f4892-f361-4013-b49f-55fa5b90aa8f\">\r\n\r\nAlso fixes the content of the top values overflowing.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/19c90cb3-2d35-41df-aea4-889a00e6ebdb\r\n\r\n\r\nFixes https://github.com/elastic/kibana/issues/192788\r\nFixes https://github.com/elastic/kibana/issues/192637\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"486b16e56f76ba0124b80b564f8516e2e22bc37b"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Quynh Nguyen (Quinn) <[email protected]>
Summary
This PR adds border to text editor for ES|QL data visualizer so make it more clear where the boundaries for the editor is.
Also fixes the content of the top values overflowing.
Screen.Recording.2024-09-18.at.11.34.10.mov
Fixes #192788
Fixes #192637