-
Notifications
You must be signed in to change notification settings - Fork 841
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
Deprecate EuiSuggest, EuiColorStops, EuiControlBar, and EuiNotificationEvent #7122
Conversation
- note: not adding a badge to the actual page headers, as deprecated components should be mostly wiped out in favor of a single callout
- EuiColorPalettePicker will eventually need this + rename the type to `PaletteColorStop` for extra explicitness while here - this is not a breaking change as the `ColorStop` type is not exported as a top-level EUI export
Preview documentation changes for this PR: https://eui.elastic.co/pr_7122/ |
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 looked through the deprecations locally and spot checked the deprecation message with axe. Zero issues.
I found one instance where the modal errors in the EuiColorSelection
containers section. If you remove the snippet <EuiFormRow label="Color stops">{stops}</EuiFormRow>
on line 62 of containers.js the modal opens as it should.
this is why things being in typescript is so useful D:
🤦 Thanks a million for catching that Trevor!! |
Preview documentation changes for this PR: https://eui.elastic.co/pr_7122/ |
💚 Build Succeeded
History
|
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.
Modal is working again, all LGTM!
Thanks Trevor! I'll hold tight on merging this PR until before the release on Monday, in case anyone else on the team has thoughts/objections to the deprecations and their messaging. |
No objections during our team sync today - merging |
## PR change summary `v87.2.0`⏩`v88.1.0`⚠️ The biggest thing to QA in this PR is several **breaking changes** to `EuiDescriptionList`. ### Description list `columnWidths` prop This PR introduces a new `columnWidths` prop and removes several Kibana instances of custom CSS overrides to title/description column widths. The primary motivation behind this is not just to reduce custom CSS, but also because v88.0.0 introduced an underlying CSS change of `column` description lists to using [`display: grid` CSS](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_grid_layout). The new prop allows us to support existing description list custom widths while not requiring Kibana developers to understand or write grid CSS (except for 1 or two scenarios around `max-width`).⚠️ **No user-facing UI around column widths should have regressed as a result of these changes. If they have, please let us know in this PR.** ### Description list gutter size changes The prop `gutterSize` has been renamed to `rowGutterSize` and the default size is now `s` instead of `m`. The change to `s` from `m` means there is an **expected** smaller gap between list items (see below screenshots): **Current `EuiDescriptionList` with default `rowGutterSize="s"`** <img width="753" alt="" src="https://github.com/elastic/kibana/assets/934879/c17aef28-ed3b-40c4-84c3-396e788b13bb"> **Prior `EuiDescriptionList` with default `gutterSize="m"`** <img width="721" alt="" src="https://github.com/elastic/kibana/assets/934879/84d5f5a2-8fa6-4f99-9dc0-73fd143aa1e1"> If Kibana teams prefer to keep the previous `m` gutter for their instances of `EuiDescriptionList`, you have a couple of options: 1. Let EUI team know in the PR and we can set usage back to what it was before 2. Set `rowGutterSize="m"` yourselves manually --- ## [`88.1.0`](https://github.com/elastic/eui/tree/v88.1.0) - Added `font.defaultUnits` theme token. EUI component font sizes default to `rem` units - this token allows consumers to configure this to `px` or `em` ([#7133](elastic/eui#7133)) - Updated `EuiDescriptionList` with new `columnWidths` prop ([#7146](elastic/eui#7146)) **Bug fixes** - Fixed `EuiDataGrid`'s keyboard shortcuts popover display ([#7146](elastic/eui#7146)) **CSS-in-JS conversions** - Renamed `useEuiFontSize()`'s `measurement` option to `unit` for clarity ([#7133](elastic/eui#7133)) ## [`88.0.0`](https://github.com/elastic/eui/tree/v88.0.0) - Updated `EuiDescriptionList` with a new `columnGutterSize` prop ([#7062](elastic/eui#7062)) **Deprecations** - Deprecated `EuiSuggest`. We recommend using `EuiSelectable` or `EuiComboBox` instead ([#7122](elastic/eui#7122)) - Deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([#7122](elastic/eui#7122)) - Deprecated `EuiColorStops`. We recommend copying the component to your application if necessary ([#7122](elastic/eui#7122)) - Deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([#7122](elastic/eui#7122)) **Breaking changes** - Renamed `EuiDescriptionList`'s `gutterSize` prop to `rowGutterSize` ([#7062](elastic/eui#7062)) - `EuiDescriptionList`'s `rowGutterSize` prop now defaults to a size of `s` (was previously `m`) ([#7062](elastic/eui#7062)) **Accessibility** - Fixed the dark mode colors of inline `EuiDescriptionListTitle`s to meet WCAG color contrast requirements ([#7062](elastic/eui#7062)) **CSS-in-JS conversions** - Converted `EuiKeyPadMenuItem` to Emotion; Removed `$euiKeyPadMenuSize` and `$euiKeyPadMenuMarginSize` ([#7118](elastic/eui#7118)) --------- Co-authored-by: Cee Chen <[email protected]> Co-authored-by: Cee Chen <[email protected]> Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Nikita Indik <[email protected]>
Summary
These components all have zero usage across Kibana and only one use across all internal Elastic products. For non-Elastic consumers using these components, we would strongly suggest copying them into your own application if you wish to use them moving forward.
As a team, EUI is currently discussing our governance policy over components we do and don't want to add/maintain. We've decided that there are either more generic versions of these components (
EuiSuggest->EuiSelectable
,EuiControlBar->EuiBottomBar
), or in the case ofEuiColorStops
andEuiNotificationEvent
, their use-cases are so specific they simply don't belong in a component library - the cost of maintaining them is higher than the value consumers get out of them.QA
General checklist
@deprecated
jsdoc are correctly struck out in IDEs