-
Notifications
You must be signed in to change notification settings - Fork 8.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
[Fleet] replace EuiFilterSelectItem with EuiSelectable #175101
[Fleet] replace EuiFilterSelectItem with EuiSelectable #175101
Conversation
/ci |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
import { i18n } from '@kbn/i18n'; | ||
import type { DataViewField, FieldSpec } from '@kbn/data-views-plugin/public'; | ||
|
||
import { useStartServices } from '../../../../../hooks'; | ||
|
||
import { AGENT_LOG_INDEX_PATTERN, DATASET_FIELD, AGENT_DATASET } from './constants'; | ||
|
||
const StyledEuiSelectable = styled(EuiSelectable)` | ||
width: 220px; |
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.
I'm having trouble making the selectable options be as wide as the longest option automatically, added a workaround for a fixed width, I'm not sure if there is a better solution. Tried the different combinations of the truncate options: https://eui.elastic.co/#/forms/selectable#truncation
@breehall Hey, any suggestions? Previously with EuiFilterSelectItem
the select expanded automatically, without truncating the options.
The textWrap: wrap
option doesn't seem to do anything either.
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.
I found a better approach by providing minWidth
in listProps
, it is still a hardcoded value, but fixes the styled-components issue.
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.
@breehall Another UI issue I noticed is when scrolling down the page with a long list of agents with a EuiSelectable
component open: the list of options is visible in front of the top banner.
![image](https://private-user-images.githubusercontent.com/90178898/298643044-1e58ae7c-7d24-4421-99de-9a5f28cd5157.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5NzExMTAsIm5iZiI6MTczODk3MDgxMCwicGF0aCI6Ii85MDE3ODg5OC8yOTg2NDMwNDQtMWU1OGFlN2MtN2QyNC00NDIxLTk5ZGUtOWE1ZjI4Y2Q1MTU3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA3VDIzMjY1MFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWNlN2I1N2RmZDg1YzE3ZTAyMTRkNzMxM2U0Y2EwZTdjZDgzZTZjMjU3NWMzNmRjMTdhOTYwNDJiNjE4M2YxZjgmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.PhSN29JBQYG-e6Aryid6Hf42hn7ykqrI8e9T_qLvaa0)
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.
Using the minWidth
prop works - another alternative is if you know for certain that your selectable content is fairly short, you can use custom width: fit-content;
CSS instead. Overall, I think minWidth
is a safer bet in case your content changes to something super long in the future that should be truncated.
when scrolling down the page with a long list of agents with a EuiSelectable component open: the list of options is visible in front of the top banner
If you know for certain your popover should sit below the sticky header/nav, you can customize this on your end with something like:
const { euiTheme } = useEuiTheme();
<EuiPopover zIndex={euiTheme.levels.header - 1}>
EUI defaults to showing popovers above headers for maximum visibility and because we can't know how the specific intention of end-popover-usages ahead of time. A couple of related issues discussing the difficulty/complexity of this topic: elastic/eui#4756, elastic/eui#4872
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.
thanks for the suggestions! I'll make the changes
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.
Made the change to move the Clear all
button outside, it works much better:
The z-index change works fine too:
I tried the width: fit-content;
option, but doesn't seem to make a difference with minWidth
. When I remove minWidth
, the options are not rendered at all. I think probably because the options are loaded async, and initially not there. I think it's fine to use minWidth
only, so the long options get truncated.
...plications/fleet/sections/agents/agent_details_page/components/agent_logs/filter_dataset.tsx
Outdated
Show resolved
Hide resolved
/ci |
/ci |
/ci |
|
||
const renderTagOption = (option: EuiSelectableOption<string>): ReactNode => { | ||
const tag = option.label; | ||
if (tag === CLEAR_ALL) { |
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.
If a clear all button must be rendered, I would strongly suggest rendering it outside the EuiSelectable
and not as an option. e.g. an <EuiButton size="s" />
or similar.
/ci |
Pinging @elastic/fleet (Team:Fleet) |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
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.
New UI looks great - especially moving the clear button. 🚀
This looks terrific! Thanks for addressing EUI's comments/TODOs in Kibana - y'all are amazing!!! ❤️ |
## Summary Closes elastic#162766 Replace deprecated EUI component. Agent details: select log level <img width="1302" alt="image" src="https://github.com/elastic/kibana/assets/90178898/2ab0cd2c-38f3-4219-9ea2-62dfaa3a8e99"> Agent details: select dataset <img width="1387" alt="image" src="https://github.com/elastic/kibana/assets/90178898/4187c274-e035-4831-9d9d-87c87a3ff9c3"> Agent status filter: <img width="1251" alt="image" src="https://github.com/elastic/kibana/assets/90178898/15e226a2-cc10-4b43-881f-923fe03364f5"> Agent list view: Agent policy select: <img width="461" alt="image" src="https://github.com/elastic/kibana/assets/90178898/4f0f697c-af33-4934-92ef-e9a8e16bf1ed"> Tags select: <img width="280" alt="image" src="https://github.com/elastic/kibana/assets/90178898/0be8719e-cf4b-4a1d-a4c6-c04090f72600"> ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
Summary
Closes #162766
Replace deprecated EUI component.
Agent details: select log level
Agent details: select dataset
Agent status filter:
![image](https://private-user-images.githubusercontent.com/90178898/297790915-15e226a2-cc10-4b43-881f-923fe03364f5.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5NzExMDksIm5iZiI6MTczODk3MDgwOSwicGF0aCI6Ii85MDE3ODg5OC8yOTc3OTA5MTUtMTVlMjI2YTItY2MxMC00YjQzLTg4MWYtOTIzZmUwMzM2NGY1LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA3VDIzMjY0OVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTk1MTg0N2E1ZGFlZWIxNWY5OWRmNjI1MTU5ZDMzMGFjZDUyNGFiOWVkZGQyN2NjM2I4ZWJhNjlhZGYyODc4OTYmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.LKK-dDsc-4oUg-_GCJk_X-t7m0wiyswInyrY3TDP8j8)
Agent list view:
![image](https://private-user-images.githubusercontent.com/90178898/298606667-4f0f697c-af33-4934-92ef-e9a8e16bf1ed.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5NzExMDksIm5iZiI6MTczODk3MDgwOSwicGF0aCI6Ii85MDE3ODg5OC8yOTg2MDY2NjctNGYwZjY5N2MtYWYzMy00OTM0LTkyZWYtZTlhOGUxNmJmMWVkLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA3VDIzMjY0OVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWE3ZDMwNDA4NjJiNjY2N2UwN2M2YTIzYWYxNjhhNGIxNzliOWYzNjlkZTlkYzFkYzEzYzdlZWQxYjAzNjRlYmMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.d0Z2A2LuzB_txASdl3j-9AZxZGTCBHnZE5_210Dz-MM)
Agent policy select:
Tags select:
![image](https://private-user-images.githubusercontent.com/90178898/298626876-0be8719e-cf4b-4a1d-a4c6-c04090f72600.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5NzExMDksIm5iZiI6MTczODk3MDgwOSwicGF0aCI6Ii85MDE3ODg5OC8yOTg2MjY4NzYtMGJlODcxOWUtY2Y0Yi00YTFkLWE0YzYtYzA0MDkwZjcyNjAwLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA3VDIzMjY0OVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWQwMDdiMTMwNzIwYmQ0Njc2ZjFjOTEyMGY1NGY0NWM5ZjhkYjY2Mzk3YzlkNTQwZjE2OTRiY2EyZWEyYzQ2ZjcmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.XmagVjvgWDnZnMN0SDg3Dnl_a7Bbe5NLj5ZuAHytZVM)
Checklist
Delete any items that are not applicable to this PR.