[Pipeline Viewer] Refactor collapsible statement component to wrap props.children #20252
Conversation
justinkambic
added
v7.0.0
Team:Logstash
v6.4.0
Team:Monitoring
|
@ycombinator sorry to ping you again, but I decided to ask for your review since this PR was based on a suggestion you had in a previous review for this component. Feel free to take as long as you need to get to this. |
💚 Build Succeeded |
| statement, | ||
| statement: { vertex }, | ||
| onShowVertexDetails | ||
| }) { |
There was a problem hiding this comment.
Why does this function take one argument that's an object vs. multiple arguments corresponding to the values in the object? Not saying it's right/wrong, just that it seems inconsistent with the other functions in this module and functions elsewhere in general. I'm guessing I'm just missing something that's unique to this function or these particular parameters, maybe?
There was a problem hiding this comment.
I'm guessing I'm just missing something that's unique to this function or these particular parameters, maybe?
I think you make a good point actually. There's no reason for these keys to not just be their own params.
There was a problem hiding this comment.
I like this a lot better actually, it simplifies everything and removes some unnecessary abstraction. WDYT?
💚 Build Succeeded |
…ops.children (#20252) * Rename config view to PipelineViewer. * Decouple CollapsibleStatement from if/else using props.children. * Modify function to take params instead of single object.
|
Backported to: |
justinkambic
deleted the
monitoring/collapsible-component-wrapper-class-refactor
branch
Changes
The goal here is to decouple the
CollapsibleStatementcomponent from whatever type of plugin/branch body it will contain. To that end, we've moved the rendering of its children outside of the component, and refactored it to rely onprops.children.Resolves #19844
Testing this PR
There aren't any functional changes in this PR, so simply smoke testing the components should be sufficient.
if...elsebranch, with X-Pack Monitoring enabled.a. Pipeline Viewer loads
b. You are able to use Detail Drawer
c. You can collapse/expand branches