-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[Canvas][tech-debt] Refactor Layout Annotations (kill recompose.pure Part 2) #73305
[Canvas][tech-debt] Refactor Layout Annotations (kill recompose.pure Part 2) #73305
Conversation
Pinging @elastic/kibana-canvas (Team:Canvas) |
x-pack/plugins/canvas/public/components/layout_annotations/layout_annotations.scss
Outdated
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.
Marking as approved since stuff is just being moved to another file. There is some future clean up that could happen here, as noted.
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.
Changes look good. 👍
width: number; | ||
} | ||
|
||
export const BorderConnection: FC<Props> = ({ transformMatrix, width, height }) => ( |
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.
Question: A few of the annotations like this one get called frequently. Should we look into doing any memoization of these components? Or do you think the overhead of the memoization would outweigh any performance gains?
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.
That's not a bad idea. I'm not sure of the perf gain, but the rendering aspect could certainly weigh heavily. I'll test before committing.
💚 Build SucceededBuild metricsasync chunks size
History
To update your PR or re-run it, just comment with: |
Summary
While converting various layout annotations to remove the
pure
HOC, I noticed they're 1/ all very alike, and 2/ spread out throughout thecomponents
directory. Seemed to make sense to combine them, (and their styles).This PR kills
recompose.pure
, refactors the files and related CSS, and applies best practices.