[EuiTreeView] Migrate to a function component - #9932
Conversation
|
💚 CLA has been signed |
|
👋 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? |
github-actions
Bot
added
the
community contribution
|
@VitorBernalRodrigues could you please sign the CLA before we proceed with the review? |
|
buildkite test this |
weronikaolejniczak
added
the
skip-changelog
💔 Build Failed
Failed CI StepsHistory |
💚 Build Succeeded
|
| const EuiTreeViewPublicComponent = | ||
| EuiTreeViewComponent as React.ForwardRefExoticComponent< | ||
| EuiTreeViewProps & React.RefAttributes<HTMLUListElement> | ||
| >; |
There was a problem hiding this comment.
Instead, let's do it like this:
const _EuiTreeView = forwardRef<HTMLUListElement, CommonTreeProps>(...)
_EuiTreeView.displayName = 'EuiTreeView'
export const EuiTreeView = Object.assign(_EuiTreeView, {
Item: EuiTreeViewItem,
})This EuiTreeViewPublicComponent breaks our docgen.
| const nextButton = nodeButtons[currentIndex + 1] as HTMLElement; | ||
| if (nextButton) { | ||
| // Enable keyboard navigation | ||
| const onKeyDown = (event: React.KeyboardEvent, node: Node) => { |
There was a problem hiding this comment.
nit: Let's destructure all of these from the React import using type keyword, not use the namespace directly. Applies to the whole file.
| const treeIdGeneratorRef = useRef< | ||
| ReturnType<typeof htmlIdGenerator> | undefined | ||
| >(undefined); | ||
| if (treeIdGeneratorRef.current === undefined) { | ||
| treeIdGeneratorRef.current = htmlIdGenerator('euiTreeView'); | ||
| } | ||
| const treeIdGenerator = treeIdGeneratorRef.current; |
There was a problem hiding this comment.
nit: We have a dedicated useGeneratedHtmlId hook.
| const styles = euiTreeViewStyles(theme); | ||
| const cssStyles = [styles.euiTreeView, styles[display]]; |
There was a problem hiding this comment.
non-blocking: Let's use useEuiMemoizedStyles.
There was a problem hiding this comment.
non-blocking: Could we use userEvent here instead of fireEvent?
|
Hey @VitorBernalRodrigues, are you planning on continuing this contribution? |
2 participants
Summary
Migrates
EuiTreeViewfrom a class component to a function component.withEuiThemewithuseEuiTheme.EuiTreeView.Item.EuiTreeViewClass, as discussed in [EuiTreeView] Migrate from class to function component #9496.<ul>.Closes #9496.
Compatibility
Current Kibana
mainwas audited forEuiTreeViewusage.EuiTreeViewClassconsumers were found.EuiTreeViewref consumers were found.expandByDefault,isExpanded, callbacks,Node, DOM/classes, andEuiTreeView.Itemremain compatible.No source-breaking Kibana usage was identified.
Testing
git diff --checkpassedyarn pre-pushpassedScreenshots
No visual changes are expected.
The migrated component was manually validated in Storybook for mouse interaction, nested expansion,
expandByDefault, expansion arrows, keyboard navigation, and focus behavior.