-
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
Upgrade EUI to v85.1.0 #162660
Upgrade EUI to v85.1.0 #162660
Conversation
- in favor of an actual component
- This CSS is no longer valid as of `EuiFilterSelectItem`'s Emotion conversion, and additionally we strongly recommend devs switch to using `EuiSelectable` as `EuiFilterSelectItem` will be deprecated at some point in the future
…e of EUI. This includes the conversion of EuiFilterGroupItem to Emotion and the removal of various euiFilterSelect classes.
Pinging @elastic/eui-team (EUI) |
Pinging @elastic/fleet (Team:Fleet) |
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.
ML related changes LGTM.
</p> | ||
</div> | ||
</div> | ||
<EuiSelectableMessage> |
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 what are the plans for switching out the deprecated EuiFilterSelectItem
for EuiSelectable
, as is still being used in our file here?
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's a few too many usages in Kibana for EUI to tackle every single one of these ourselves, so we left the inline comments in hopes that Kibana teams would grab the deprecation conversions in their own time. It should hopefully be a fairly straightforward conversion, and if needed you can reference our docs example: https://elastic.github.io/eui/#/forms/filter-group#multi-select
As I mentioned in another comment, the final removal of EuiFilterSelectItem
is still 6months+ out, so hopefully plenty of time for y'all - but please let us know if we can be a resource for this!
x-pack/plugins/ml/public/application/components/multi_select_picker/multi_select_picker.tsx
Show resolved
Hide resolved
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.
Kibana security (Spaces navigation) changes 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.
I found a small visual regression in Index Management. Please let me know if you need help addressing. Thanks!
...components/component_templates/component_template_selector/components/filter_list_button.tsx
Show resolved
Hide resolved
@@ -76,7 +83,10 @@ export function FilterListButton({ onChange, filters }: Props) { | |||
panelPaddingSize="none" | |||
data-test-subj="filterList" | |||
> | |||
<div className="euiFilterSelect__items"> | |||
{/* EUI NOTE: Please use EuiSelectable (which already has height/scrolling built in) |
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.
Is there an approximate timeline yet for the deprecation of EuiFilterSelectItem
?
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.
Apologies for not clarifying this - we're shooting for the end of the 2023, although this isn't a firm timeline on our end and we're super happy to flex/work with Kibana teams on this!
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.
Checked out, tested locally and Security Detection Rules changes (ML Popover) 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.
ent-search changes 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.
Presentation team changes LGTM 👍
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
@elasticmachine merge upstream |
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.
Deployment Management changes LGTM
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
Thanks everyone for your terrific reviews and QA so far! ❤️ Pinging the remaining listed CODEOWNERS: @elastic/kibana-design, @elastic/security-threat-hunting-explore, @elastic/security-detection-engine and @gergoabraham. I'll be asking @elastic/kibana-operations to admin merge this in sometime tomorrow as a heads up. |
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.
Added #162943 to our todo list.
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.
Detection Engine area LGTM
85.0.0
➡️85.1.0
EuiFilterSelectItem
CSS classes and styles associated with those classes. EUI has previously exported component-specific CSS classes for usage such as.euiFilterSelect__items
,.euiFilterGroup__popoverPanel
, or.euiAccordionForm
.❌ As of our move to CSS-in-JS, we are actively moving away from this architecture. The goal is to either provide either a component or prop directly to you instead of a CSS class. As a reminder, our classNames are not guaranteed APIs and are subject to change without warning - should you need to tweak or customize EUI's styling, we strongly recommend passing in your own
className
.💬 Moving forward, EUI will attempted to convert any usages of removed CSS classes and their direct usages in Kibana for you. In most cases, we'll hopefully be able to take the correct path of using a component or prop instead. In cases where we cannot or significant/complex changes are required, we may temporarily convert the CSS to a static CSS-in-JS usage instead and add a TODO asking the relevant team to address this in their own time (for example, the deprecation of
EuiFilterSelectItem
and its conversion toEuiSelectable
).85.1.0
EuiComboBox
'soptions
to acceptoption.append
andoption.prepend
props (#6953).substr()
usages to.substring()
(#6954)EuiInlineEdit
's read mode button to include a title tooltip, increasing readability of truncated text (#6966)Bug fixes
EuiFilterGroup
's responsive styles (#6983)Deprecations
EuiFilterSelectItem
; UseEuiSelectable
instead (#6982)CSS-in-JS conversions
EuiFilterSelectItem
to Emotion (#6982).euiFilterSelect__items
CSS; UseEuiSelectable
instead (#6982).euiFilterSelect__note
and.euiFilterSelect__noteContent
CSS; UseEuiSelectableMessage
instead (#6982)focus.transparency
andfocus.backgroundColor
theme tokens (#6984)