Fix/eui migrate class to function components#9618
Fix/eui migrate class to function components#9618toffku wants to merge 3 commits intoelastic:mainfrom
Conversation
…//github.com/toffku/eui into fix/eui-migrate-class-to-function-components
|
👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually? |
There was a problem hiding this comment.
Pull request overview
Migrates EuiFormControlLayoutIcons from a stateless class component to a function component to address #9461, keeping render behavior the same while simplifying the implementation.
Changes:
- Converted
EuiFormControlLayoutIconsfromclass extends Componentto a function component with destructured props/defaults - Extracted former class render methods into module-level helper functions (
renderCustomIcon,renderLoadingSpinner, etc.) - Updated React import to remove the unused
Componentimport
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
💚 Build Succeeded |
💚 Build Succeeded
|
| const renderCustomIcon = ( | ||
| icon: EuiFormControlLayoutIconsProps['icon'], | ||
| isDisabled: EuiFormControlLayoutIconsProps['isDisabled'] | ||
| ) => { | ||
| if (!icon) { | ||
| return null; | ||
| } | ||
|
|
||
| renderDropdownIcon() { | ||
| const { isDropdown, isDisabled } = this.props; | ||
|
|
||
| if (!isDropdown) { | ||
| return null; | ||
| } | ||
| // Normalize the icon to an object if it's a string. | ||
| const iconProps: IconShape = isIconShape(icon) | ||
| ? icon | ||
| : { | ||
| type: icon, | ||
| }; | ||
| const { ref: iconRef, side, ...iconRest } = iconProps; | ||
|
|
||
| return ( | ||
| <EuiFormControlLayoutCustomIcon | ||
| size="m" | ||
| disabled={isDisabled} | ||
| iconRef={iconRef} | ||
| {...iconRest} | ||
| /> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
@toffku It's absolutely okay for the purposes of the class to function migration to simply raise these render functions to module level and call it a day. But a more idiomatic approach here would be to make them into components.
E.g.
| const renderCustomIcon = ( | |
| icon: EuiFormControlLayoutIconsProps['icon'], | |
| isDisabled: EuiFormControlLayoutIconsProps['isDisabled'] | |
| ) => { | |
| if (!icon) { | |
| return null; | |
| } | |
| renderDropdownIcon() { | |
| const { isDropdown, isDisabled } = this.props; | |
| if (!isDropdown) { | |
| return null; | |
| } | |
| // Normalize the icon to an object if it's a string. | |
| const iconProps: IconShape = isIconShape(icon) | |
| ? icon | |
| : { | |
| type: icon, | |
| }; | |
| const { ref: iconRef, side, ...iconRest } = iconProps; | |
| return ( | |
| <EuiFormControlLayoutCustomIcon | |
| size="m" | |
| disabled={isDisabled} | |
| iconRef={iconRef} | |
| {...iconRest} | |
| /> | |
| ); | |
| }; | |
| const CustomIcon = ({ icon, isDisabled }: { | |
| icon: EuiFormControlLayoutIconsProps['icon'], | |
| isDisabled: EuiFormControlLayoutIconsProps['isDisabled'] | |
| }) => { | |
| if (!icon) return null; | |
| // Normalize the icon to an object if it's a string. | |
| const iconProps: IconShape = isIconShape(icon) | |
| ? icon | |
| : { type: icon }; | |
| const { ref: iconRef, side, ...iconRest } = iconProps; | |
| return ( | |
| <EuiFormControlLayoutCustomIcon | |
| size="m" | |
| disabled={isDisabled} | |
| iconRef={iconRef} | |
| {...iconRest} | |
| /> | |
| ); | |
| }; |
and then in EuiFormControlLayoutIcons render: <CustomIcon icon={icon} isDisabled={isDisabled} />
weronikaolejniczak
left a comment
There was a problem hiding this comment.
I have just one comment that is very straightforward to address. Otherwise, we're good to go! Thank you for contributing, @toffku 🙏🏻
Summary
What: Migrated EuiFormControlLayoutIcons from class to function component. Converted class methods (renderCustomIcon, renderLoadingSpinner, renderClearButton, renderInvalidIcon, renderDropdownIcon) into helper functions.
Why: Fixes [EuiFormControlLayoutIcons] Migrate from class to function components #9461
How:
API Changes
Screenshots
Impact Assessment
Note: Most PRs should be tested in Kibana to help gauge their Impact before merging.
Impact level: 🟢 None
Release Readiness
QA instructions for reviewer
Checklist before marking Ready for Review
breaking changelabel (if applicable)Reviewer checklist