Explicitly support EUI Icons in EuiContextMenuItemIcon type definition #7804
Conversation
eokoneyo
force-pushed
the
chore/explicitly-support-eui-icon-in-context-menu
branch
from
94a013c to
bbf6e95
Compare
| import { EuiToolTip, EuiToolTipProps } from '../tool_tip'; | ||
|
|
||
| import { euiContextMenuItemStyles } from './context_menu_item.styles'; | ||
|
|
||
| export type EuiContextMenuItemIcon = ReactElement<any> | string | HTMLElement; | ||
| export type EuiContextMenuItemIcon = IconType | ReactElement<any>; |
There was a problem hiding this comment.
This removes | HTMLElement as a supported type - was that intentional? It's not clear to me whether this change was meant to be additive or subtractive. If only the former, this unfortunately may end up messing with Kibana usages that pass, e.g. icon={someRawSvg} or similar.
There was a problem hiding this comment.
Hey @eokoneyo! Just checking in on this PR again - did you have a chance to look at the above? If not, I might go ahead and tweak your PR to add in | HTMLElement in again for safety and merge that if no type errors occur.
There was a problem hiding this comment.
Hey @eokoneyo! Just checking in on this PR again - did you have a chance to look at the above? If not, I might go ahead and tweak your PR to add in
| HTMLElementin again for safety and merge that if no type errors occur.
Hey @cee-chen thanks for the ping, it wasn't particularly intentional. I'd made the assumption that even SVG usages would ideally be of the react component form. I'll be reverting to include support for HTMLElement.
There was a problem hiding this comment.
It's definitely the ideal, but since it was previously supported we have to mark it a breaking change if we're no longer typing for it, which I'd prefer to avoid for small changes. There probably is some consumer out there who's passing a raw <svg /> element to this prop who would be upset otherwise 🫠
|
Preview staging links for this PR:
|
💚 Build Succeeded
History
cc @eokoneyo |
`v95.1.0`⏩`v95.2.0` _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v95.2.0`](https://github.com/elastic/eui/releases/v95.2.0) - Updated `EuiContextMenuItemIcon`'s type definition to explicitly define support for `EuiIcon`'s `IconType` ([#7804](elastic/eui#7804)) - Updated `EuiSteps` to support a new `titleSize="xxs"` style, which outputs the same title font size but smaller unnumbered step indicators ([#7813](elastic/eui#7813)) - Updated `EuiStepsHorizontal` to support a new `size="xs"` style, which outputs smaller unnumbered step indicators ([#7813](elastic/eui#7813)) - Updated `EuiStepNumber` to support new `titleSize="none"` which omits rendering step numbers, and will only render icons ([#7813](elastic/eui#7813)) - Updated `setEuiDevProviderWarning` to additionally accept a custom callback function, which warning messages will be passed to ([#7820](elastic/eui#7820)) - Updated `EuiIcon` to feature updated `logoElasticStack` logo for referencing Elastic Stack platform ([#7838](elastic/eui#7838)) - Updated `EuiIcon` to feature updated `casesApp` design. ([#7840](elastic/eui#7840)) - Updated `EuiComboBox` to no longer autocomplete searched text when used within forms ([#7842](elastic/eui#7842)) **CSS-in-JS conversions** - Converted `EuiFilePicker` to Emotion; Removed `$euiFilePickerTallHeight` ([#7833](elastic/eui#7833)) --------- Co-authored-by: Jon <[email protected]>
Summary
This PR includes changes to the type definition for
EuiContextMenuItemIconso that support for valid EUIIconTypeis explicit.This change would be transparent to the end user, and is more a DX improvement.