[Security Solution]Expandable flyout - Replace rule sections with new components

[christineweng]

This PR updates rule preview panel in the document expandable flyout:

• Replaced rule details sections with the simplified components from [Security Solution] Refactor Rule Details page #166158
• Added itemRenderer to allow custom render of the description list
• Removed isPanelView props from the rule detail read only components. It was added to accommodate the preview styling ([Security Solution] Expandable flyout - Rule preview contents #163027)

No UI change from this PR

How to test

• Go to alerts page and generate some alerts
• Expand a row in the table, a flyout should appear
• Click Show rule summary to expand the rule preview panel

[elasticmachine]

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

[PhilippeOberti]

@christineweng something bothers me with this approach but I'm not sure what is exactly...

Maybe it's the fact that we're lacking documentation of the props, so it's unclear what the itemRenderer does exactly. Unless I look at the component's implementation, I wouldn't be able to tell that the itemRender overrides the default render.

Maybe it's the fact that ultimately the default renderer and the one we're passing from our flyout component aren't that different. They both use the EuiDescriptionList component. Yes the type, rowGutterSize and className are different, but couldn't we make these props and have a single render inside the component?

Maybe it's the face that we have a default render in the component and the other ones are passed in via props. We could have no default render and have the itemRenderer a required props instead of optional. We could even have the RuleAboutSection render {children} and force the parent to define how they want the list to be rendered?

Just a bunch of ideas, but I feel this can be improved, maybe with the help of the @elastic/security-detection-rule-management team?

[christineweng]

@PhilippeOberti The render prop approach aims at avoiding too many props purely for UI. I wonder if it's more bothering because it is repeated 3 times? maybe it will help description list is extracted and reused..

But ultimately, all we wanted is to show rule details in a smaller font and more compressed. I don't have strong preference on either approach. I'll defer to @elastic/security-detection-rule-management / @nikitaindik on their preference?

[nikitaindik]

First of all, thanks for doing this refactoring work! ❤️ It brings us one step closer to decoupling rule view components from rule creation components. Once we also refactor the remaining bits of the creation page, we can remove a lot of tangled code. And that's great!

As for passing the itemRenderer, I think the most straightforward way would be to just expand the props of section components with EuiDescriptionList props. Since the only difference between default rendering and custom rendering are props passed to EuiDescriptionList.

It can look like this:

        <RuleDefinitionSection
          rule={castRuleAsRuleResponse(rule)}
          type="row"
          className={panelViewStyle}
          rowGutterSize="s"
        />

And then this the section component code:

export interface RuleDefinitionSectionProps
  extends React.ComponentProps<typeof EuiDescriptionList> { // allow EuiDescriptionList props
  rule: Partial<RuleResponse>;
  isInteractive?: boolean;
  dataTestSubj?: string;
}

export const RuleDefinitionSection = ({
  rule,
  isInteractive = false,
  dataTestSubj,
  ...descriptionListProps
}: RuleDefinitionSectionProps) => {
  /* ... */
  return (
    <div data-test-subj={dataTestSubj}>
      <EuiDescriptionList
        {...descriptionListProps}
        type={descriptionListProps.type ?? 'column'} // Set defaults here to avoid changing existing usage of section components
        rowGutterSize={descriptionListProps.rowGutterSize ?? 'm'} // Or pass these manually in all places where sections are used (not a lot of places yet)
        listItems={definitionSectionListItems}
        columnWidths={DESCRIPTION_LIST_COLUMN_WIDTHS}
      />
    </div>
  );
};

Passing the itemRenderer is also a fine way IMO, and we might need it in the future, but for now I think just adding the props would suffice.

What do you folks think?

[PhilippeOberti]

yup thanks for your input @nikitaindik, I like this approach the best!

[nikitaindik]

Tested locally - LGTM!

One minor thing I noticed, not directly related to this PR.
The padding above title of the first foldable section (About) is a little smaller than for other sections. Feel free to address in this or other PR.

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: d7bfd78

Failed CI Steps

• FTR Configs #56

Test Failures

• [job] [logs] FTR Configs #56 / discover feature controls discover feature controls security "after all" hook in "discover feature controls security"
• [job] [logs] FTR Configs #56 / discover feature controls discover feature controls security global discover read-only privileges does not allow saving changes to saved query from the saved query management component

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
securitySolution	13.0MB	12.8MB	-140.3KB

History

• 💔 Build #170593 failed 1ed469f
• 💚 Build #168377 succeeded 0eda0cc
• 💔 Build #168368 failed 810efa5ef7c2fb095f3917c3010f5a0cc249d708

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @christineweng

[nikitaindik]

Approved

[PhilippeOberti]

thanks for making the change @christineweng, tested and code LGTM! :)