[Security Solution] Fix broken Rule Filters components when content is extremely long and when alias is present #176590
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jpdjere
added
release_note:fix
Feature:Detection Rules
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
banderror
added
v8.12.2
Feature:Rule Details
banderror
reviewed
Feb 9, 2024
.../public/detection_engine/rule_management/components/rule_details/rule_definition_section.tsx
Outdated
Show resolved
Hide resolved
dplumlee
approved these changes
Feb 12, 2024
banderror
approved these changes
Feb 13, 2024
There was a problem hiding this comment.
Awesome! This looks like a more robust fix and I hope it will be enough to not have any more regressions in the future.
Tested locally with the rule from the SDH (#145076 (comment)) and some modifications of it. This is how it looked like (I think it looks good!):
I found a bug which is not related to this fix and created a ticket for it: #176866
LGTM 👍
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @jpdjere |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 14, 2024
…s extremely long and when alias is present (elastic#176590) Fixes: elastic#145076 Fixes: elastic#162543 ## Summary This PR solves two separate issues in the Filters component, used in the Rule Details page. 1. **when rule filter is extremely long, the component would break the layout of the whole page**: fixed by adding a styled wrapper component to our About, Definition and Schedule section, [that allows wrapping of extremely long text `anywhere`](https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap). This was precisely the issue that was breaking our layout when the filters were extremely long, with the aggravating factor that the filters were async loaded, and populated the component after the css was loaded. 2. **when a rule filter had a name (alias) that should have been displayed as a label instead of the actual filter**. This was like this before 8.8, but was apparently lost during some refactoring. This PR reintroduces that logic. ## Screenshots ### Broken page with long filters #### Before  #### After  **Mobile:**  #### Alias not showing as name ### Before  ### After  ## Browser compatibility - Above screenshots are **Chrome** - **Firefox**  - **Safari**  ### Checklist Delete any items that are not applicable to this PR. - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co> (cherry picked from commit 532ac06)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Feb 14, 2024
…ntent is extremely long and when alias is present (#176590) (#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 (#176590)](#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":"jpdjeredjian@gmail.com"},"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 (#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\r\n\r\n#### After\r\n\r\n\r\n\r\n**Mobile:**\r\n\r\n\r\n\r\n\r\n#### Alias not showing as name\r\n\r\n### Before\r\n\r\n\r\n### After\r\n\r\n\r\n\r\n\r\n\r\n## Browser compatibility\r\n\r\n- Above screenshots are **Chrome**\r\n- **Firefox**\r\n\r\n\r\n\r\n- **Safari**\r\n\r\n\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 <georgii.gorbachev@elastic.co>","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 (#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\r\n\r\n#### After\r\n\r\n\r\n\r\n**Mobile:**\r\n\r\n\r\n\r\n\r\n#### Alias not showing as name\r\n\r\n### Before\r\n\r\n\r\n### After\r\n\r\n\r\n\r\n\r\n\r\n## Browser compatibility\r\n\r\n- Above screenshots are **Chrome**\r\n- **Firefox**\r\n\r\n\r\n\r\n- **Safari**\r\n\r\n\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 <georgii.gorbachev@elastic.co>","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 (#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\r\n\r\n#### After\r\n\r\n\r\n\r\n**Mobile:**\r\n\r\n\r\n\r\n\r\n#### Alias not showing as name\r\n\r\n### Before\r\n\r\n\r\n### After\r\n\r\n\r\n\r\n\r\n\r\n## Browser compatibility\r\n\r\n- Above screenshots are **Chrome**\r\n- **Firefox**\r\n\r\n\r\n\r\n- **Safari**\r\n\r\n\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 <georgii.gorbachev@elastic.co>","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 <jpdjeredjian@gmail.com>
CoenWarmer
pushed a commit
to CoenWarmer/kibana
that referenced
this pull request
Feb 15, 2024
…s extremely long and when alias is present (elastic#176590) Fixes: elastic#145076 Fixes: elastic#162543 ## Summary This PR solves two separate issues in the Filters component, used in the Rule Details page. 1. **when rule filter is extremely long, the component would break the layout of the whole page**: fixed by adding a styled wrapper component to our About, Definition and Schedule section, [that allows wrapping of extremely long text `anywhere`](https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap). This was precisely the issue that was breaking our layout when the filters were extremely long, with the aggravating factor that the filters were async loaded, and populated the component after the css was loaded. 2. **when a rule filter had a name (alias) that should have been displayed as a label instead of the actual filter**. This was like this before 8.8, but was apparently lost during some refactoring. This PR reintroduces that logic. ## Screenshots ### Broken page with long filters #### Before  #### After  **Mobile:**  #### Alias not showing as name ### Before  ### After  ## Browser compatibility - Above screenshots are **Chrome** - **Firefox**  - **Safari**  ### Checklist Delete any items that are not applicable to this PR. - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>
banderror
added
the
impact:high
fkanout
pushed a commit
to fkanout/kibana
that referenced
this pull request
Mar 4, 2024
…s extremely long and when alias is present (elastic#176590) Fixes: elastic#145076 Fixes: elastic#162543 ## Summary This PR solves two separate issues in the Filters component, used in the Rule Details page. 1. **when rule filter is extremely long, the component would break the layout of the whole page**: fixed by adding a styled wrapper component to our About, Definition and Schedule section, [that allows wrapping of extremely long text `anywhere`](https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap). This was precisely the issue that was breaking our layout when the filters were extremely long, with the aggravating factor that the filters were async loaded, and populated the component after the css was loaded. 2. **when a rule filter had a name (alias) that should have been displayed as a label instead of the actual filter**. This was like this before 8.8, but was apparently lost during some refactoring. This PR reintroduces that logic. ## Screenshots ### Broken page with long filters #### Before  #### After  **Mobile:**  #### Alias not showing as name ### Before  ### After  ## Browser compatibility - Above screenshots are **Chrome** - **Firefox**  - **Safari**  ### Checklist Delete any items that are not applicable to this PR. - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #145076
Fixes: #162543
Summary
This PR solves two separate issues in the Filters component, used in the Rule Details page.
anywhere. This was precisely the issue that was breaking our layout when the filters were extremely long, with the aggravating factor that the filters were async loaded, and populated the component after the css was loaded.Screenshots
Broken page with long filters
Before
After
Mobile:
Alias not showing as name
Before
After
Browser compatibility
Above screenshots are Chrome
Firefox
Safari
Checklist
Delete any items that are not applicable to this PR.
For maintainers