Skip to content

Commit

Permalink
[8.12] [Security Solution] Fix broken Rule Filters components when co…
Browse files Browse the repository at this point in the history
…ntent is extremely long and when alias is present (elastic#176590) (elastic#176928)

# Backport

This will backport the following commits from `main` to `8.12`:
- [[Security Solution] Fix broken Rule Filters components when content
is extremely long and when alias is present
(elastic#176590)](elastic#176590)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Juan Pablo
Djeredjian","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-02-14T15:13:07Z","message":"[Security
Solution] Fix broken Rule Filters components when content is extremely
long and when alias is present (elastic#176590)\n\nFixes:
https://github.com/elastic/kibana/issues/145076\r\nFixes:
https://github.com/elastic/kibana/issues/162543\r\n\r\n##
Summary\r\n\r\nThis PR solves two separate issues in the Filters
component, used in the\r\nRule Details page.\r\n\r\n1. **when rule
filter is extremely long, the component would break the\r\nlayout of the
whole page**: fixed by adding a styled wrapper component\r\nto our
About, Definition and Schedule section, [that allows wrapping
of\r\nextremely long
text\r\n`anywhere`](https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap).\r\nThis
was precisely the issue that was breaking our layout when the\r\nfilters
were extremely long, with the aggravating factor that the\r\nfilters
were async loaded, and populated the component after the css
was\r\nloaded.\r\n2. **when a rule filter had a name (alias) that should
have been\r\ndisplayed as a label instead of the actual filter**. This
was like this\r\nbefore 8.8, but was apparently lost during some
refactoring. This PR\r\nreintroduces that logic.\r\n\r\n##
Screenshots\r\n\r\n### Broken page with long filters\r\n\r\n####
Before\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/928f642d-fce2-4bd7-b0ee-2f318109777a)\r\n\r\n####
After\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/dcac5f7b-c716-44c9-9f9c-124cd616bcf8)\r\n**Mobile:**\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/a2ef0f17-2cab-49d9-99bd-0a9d3a712a2d)\r\n\r\n\r\n####
Alias not showing as name\r\n\r\n###
Before\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/d68c7569-2f86-4f58-8b45-d67ee53e6821)\r\n###
After\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/f4f24427-8e82-4abe-9fa2-dbc8690dbb51)\r\n\r\n\r\n##
Browser compatibility\r\n\r\n- Above screenshots are **Chrome**\r\n-
**Firefox**\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/e2ab0221-bfde-4544-afb2-6f5e1a4db0ff)\r\n\r\n-
**Safari**\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/962dd314-1ba9-4aa2-81c1-955c1c1f9036)\r\n\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Georgii Gorbachev
<[email protected]>","sha":"532ac0604651dc7be83361653ddfb8d4682780c2","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Rule Details","8.13
candidate","v8.13.0","v8.12.2"],"title":"[Security Solution] Fix broken
Rule Filters components when content is extremely long and when alias is
present","number":176590,"url":"https://github.com/elastic/kibana/pull/176590","mergeCommit":{"message":"[Security
Solution] Fix broken Rule Filters components when content is extremely
long and when alias is present (elastic#176590)\n\nFixes:
https://github.com/elastic/kibana/issues/145076\r\nFixes:
https://github.com/elastic/kibana/issues/162543\r\n\r\n##
Summary\r\n\r\nThis PR solves two separate issues in the Filters
component, used in the\r\nRule Details page.\r\n\r\n1. **when rule
filter is extremely long, the component would break the\r\nlayout of the
whole page**: fixed by adding a styled wrapper component\r\nto our
About, Definition and Schedule section, [that allows wrapping
of\r\nextremely long
text\r\n`anywhere`](https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap).\r\nThis
was precisely the issue that was breaking our layout when the\r\nfilters
were extremely long, with the aggravating factor that the\r\nfilters
were async loaded, and populated the component after the css
was\r\nloaded.\r\n2. **when a rule filter had a name (alias) that should
have been\r\ndisplayed as a label instead of the actual filter**. This
was like this\r\nbefore 8.8, but was apparently lost during some
refactoring. This PR\r\nreintroduces that logic.\r\n\r\n##
Screenshots\r\n\r\n### Broken page with long filters\r\n\r\n####
Before\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/928f642d-fce2-4bd7-b0ee-2f318109777a)\r\n\r\n####
After\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/dcac5f7b-c716-44c9-9f9c-124cd616bcf8)\r\n**Mobile:**\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/a2ef0f17-2cab-49d9-99bd-0a9d3a712a2d)\r\n\r\n\r\n####
Alias not showing as name\r\n\r\n###
Before\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/d68c7569-2f86-4f58-8b45-d67ee53e6821)\r\n###
After\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/f4f24427-8e82-4abe-9fa2-dbc8690dbb51)\r\n\r\n\r\n##
Browser compatibility\r\n\r\n- Above screenshots are **Chrome**\r\n-
**Firefox**\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/e2ab0221-bfde-4544-afb2-6f5e1a4db0ff)\r\n\r\n-
**Safari**\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/962dd314-1ba9-4aa2-81c1-955c1c1f9036)\r\n\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Georgii Gorbachev
<[email protected]>","sha":"532ac0604651dc7be83361653ddfb8d4682780c2"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/176590","number":176590,"mergeCommit":{"message":"[Security
Solution] Fix broken Rule Filters components when content is extremely
long and when alias is present (elastic#176590)\n\nFixes:
https://github.com/elastic/kibana/issues/145076\r\nFixes:
https://github.com/elastic/kibana/issues/162543\r\n\r\n##
Summary\r\n\r\nThis PR solves two separate issues in the Filters
component, used in the\r\nRule Details page.\r\n\r\n1. **when rule
filter is extremely long, the component would break the\r\nlayout of the
whole page**: fixed by adding a styled wrapper component\r\nto our
About, Definition and Schedule section, [that allows wrapping
of\r\nextremely long
text\r\n`anywhere`](https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap).\r\nThis
was precisely the issue that was breaking our layout when the\r\nfilters
were extremely long, with the aggravating factor that the\r\nfilters
were async loaded, and populated the component after the css
was\r\nloaded.\r\n2. **when a rule filter had a name (alias) that should
have been\r\ndisplayed as a label instead of the actual filter**. This
was like this\r\nbefore 8.8, but was apparently lost during some
refactoring. This PR\r\nreintroduces that logic.\r\n\r\n##
Screenshots\r\n\r\n### Broken page with long filters\r\n\r\n####
Before\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/928f642d-fce2-4bd7-b0ee-2f318109777a)\r\n\r\n####
After\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/dcac5f7b-c716-44c9-9f9c-124cd616bcf8)\r\n**Mobile:**\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/a2ef0f17-2cab-49d9-99bd-0a9d3a712a2d)\r\n\r\n\r\n####
Alias not showing as name\r\n\r\n###
Before\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/d68c7569-2f86-4f58-8b45-d67ee53e6821)\r\n###
After\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/f4f24427-8e82-4abe-9fa2-dbc8690dbb51)\r\n\r\n\r\n##
Browser compatibility\r\n\r\n- Above screenshots are **Chrome**\r\n-
**Firefox**\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/e2ab0221-bfde-4544-afb2-6f5e1a4db0ff)\r\n\r\n-
**Safari**\r\n\r\n![image](https://github.com/elastic/kibana/assets/5354282/962dd314-1ba9-4aa2-81c1-955c1c1f9036)\r\n\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Georgii Gorbachev
<[email protected]>","sha":"532ac0604651dc7be83361653ddfb8d4682780c2"}},{"branch":"8.12","label":"v8.12.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Juan Pablo Djeredjian <[email protected]>
  • Loading branch information
kibanamachine and jpdjere authored Feb 14, 2024
1 parent 78cb16c commit 4884e48
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,14 @@ const StyledMinHeightTabContainer = styled.div`
min-height: 800px;
`;

/**
* Wrapper for the About, Definition and Schedule sections.
* - Allows for overflow wrapping of extremely long text, that might otherwise break the layout.
*/
const RuleFieldsSectionWrapper = styled.div`
overflow-wrap: anywhere;
`;

type DetectionEngineComponentProps = PropsFromRedux;

const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({
Expand Down Expand Up @@ -648,50 +656,52 @@ const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({
{ruleError}
<LegacyUrlConflictCallOut rule={rule} spacesApi={spacesApi} />
<EuiSpacer />
<EuiFlexGroup>
<EuiFlexItem data-test-subj="aboutRule" component="section" grow={1}>
{rule !== null && (
<StepAboutRuleToggleDetails
loading={isLoading}
stepData={aboutRuleData}
stepDataDetails={modifiedAboutRuleDetailsData}
rule={rule}
/>
)}
</EuiFlexItem>

<EuiFlexItem grow={1}>
<EuiFlexGroup direction="column">
<EuiFlexItem component="section" grow={1} data-test-subj="defineRule">
<StepPanel loading={isLoading} title={ruleI18n.DEFINITION}>
{rule !== null && !isStartingJobs && (
<RuleDefinitionSection
rule={rule}
isInteractive
dataTestSubj="definitionRule"
/>
)}
</StepPanel>
</EuiFlexItem>
<EuiSpacer />
<EuiFlexItem data-test-subj="schedule" component="section" grow={1}>
<StepPanel loading={isLoading} title={ruleI18n.SCHEDULE}>
{rule != null && <RuleScheduleSection rule={rule} />}
</StepPanel>
</EuiFlexItem>
{hasActions && (
<EuiFlexItem data-test-subj="actions" component="section" grow={1}>
<StepPanel loading={isLoading} title={ruleI18n.ACTIONS}>
<StepRuleActionsReadOnly
addPadding={false}
defaultValues={ruleActionsData}
/>
<RuleFieldsSectionWrapper>
<EuiFlexGroup>
<EuiFlexItem data-test-subj="aboutRule" component="section" grow={1}>
{rule !== null && (
<StepAboutRuleToggleDetails
loading={isLoading}
stepData={aboutRuleData}
stepDataDetails={modifiedAboutRuleDetailsData}
rule={rule}
/>
)}
</EuiFlexItem>

<EuiFlexItem grow={1}>
<EuiFlexGroup direction="column">
<EuiFlexItem component="section" grow={1} data-test-subj="defineRule">
<StepPanel loading={isLoading} title={ruleI18n.DEFINITION}>
{rule !== null && !isStartingJobs && (
<RuleDefinitionSection
rule={rule}
isInteractive
dataTestSubj="definitionRule"
/>
)}
</StepPanel>
</EuiFlexItem>
)}
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer />
<EuiFlexItem data-test-subj="schedule" component="section" grow={1}>
<StepPanel loading={isLoading} title={ruleI18n.SCHEDULE}>
{rule != null && <RuleScheduleSection rule={rule} />}
</StepPanel>
</EuiFlexItem>
{hasActions && (
<EuiFlexItem data-test-subj="actions" component="section" grow={1}>
<StepPanel loading={isLoading} title={ruleI18n.ACTIONS}>
<StepRuleActionsReadOnly
addPadding={false}
defaultValues={ruleActionsData}
/>
</StepPanel>
</EuiFlexItem>
)}
</EuiFlexGroup>
</EuiFlexItem>
</EuiFlexGroup>
</RuleFieldsSectionWrapper>
<EuiSpacer />
<TabNavigation navTabs={pageTabs} />
<EuiSpacer />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,22 +90,25 @@ const Filters = ({ filters, dataViewId, index, 'data-test-subj': dataTestSubj }:

return (
<EuiFlexGroup wrap responsive={false} gutterSize="xs" data-test-subj={dataTestSubj}>
{flattenedFilters.map((filter, idx) => (
<EuiFlexItem
grow={false}
key={`filter-${idx}`}
css={{ width: '100%' }}
data-test-subj={`filterItem-${filter.meta.key}`}
>
<EuiBadgeWrap color="hollow">
{indexPattern != null ? (
<FilterBadgeGroup filters={[filter]} dataViews={[indexPattern]} />
) : (
<EuiLoadingSpinner size="m" />
)}
</EuiBadgeWrap>
</EuiFlexItem>
))}
{flattenedFilters.map((filter, idx) => {
const displayContent = filter.meta.alias ? (
filter.meta.alias
) : (
<FilterBadgeGroup filters={[filter]} dataViews={[indexPattern]} />
);
return (
<EuiFlexItem
grow={false}
key={`filter-${idx}`}
css={{ width: '100%' }}
data-test-subj={`filterItem-${filter.meta.key}`}
>
<EuiBadgeWrap color="hollow">
{indexPattern != null ? displayContent : <EuiLoadingSpinner size="m" />}
</EuiBadgeWrap>
</EuiFlexItem>
);
})}
</EuiFlexGroup>
);
};
Expand Down

0 comments on commit 4884e48

Please sign in to comment.