[ResponseObs] "Active" alert table row is not highlighted when flyout is open #155487
Assignees
Labels
bug
Fixes for quality problems that affect the customer experience
fixed
Team:ResponseOps
Label for the ResponseOps team (formerly the Cases and Alerting teams)
Team:Threat Hunting:Investigations
Security Solution Investigations Team
v8.8.0
Comments
janmonschke
added
bug
|
Pinging @elastic/response-ops (Team:ResponseOps) |
|
Will provide a PR with a fix shortly |
1 task
janmonschke
added a commit
that referenced
this issue
Apr 28, 2023
## Summary As described in #152318, we noticed that building block alerts were not highlighted anymore after the migration to the new alerts table. A preferred implementation of building block alert highlighting would follow the [`EUIDataGrid` approach of row highlighting](https://eui.elastic.co/#/tabular-content/data-grid-style-display#grid-row-classes). The `DataGrid` allows you to pass custom CSS class names for each row (`gridStyle.rowClasses`). That would allow us to highlight table rows with building block alerts. However, without access to the underlying data, we would not be able generate the correct `rowClasses` for rows with building block alerts. So simply passing `gridStyle.rowClasses` to the `AlertsStateTable` was not an option. Therefore in this PR we're introducing a new prop on the `AlertsTable`, the `highlightedRowMapper`. It's a callback function that receives the alert data and when it returns true, that row will be highlighted. This allows for highlighting of rows from the outside without exposing too many details about the underlying data structures. **Screenshot of the alerts table with a highlightedRowMapper that highlights building block alerts** <img width="1259" alt="Screenshot 2023-04-21 at 13 03 54" src="https://user-images.githubusercontent.com/68591/233620704-a56204c0-e285-4289-897a-58481f440446.png"> ### Additional notes - Since the alerts table has default grid styles, it allows to pass `gridStyle` and it computes its own `rowClasses` for "active row" highlighting, the logic for merging all those styles looks intimidating. I tried my best to comment that part of code to make it clear why the merges are necessary and how they work. - While working on the issue, I noticed that active rows are not highlighted anymore (related bug: #155487). The changes in this PR fix that behaviour as well as you can see in the screenshot below: <img width="936" alt="Screenshot 2023-04-21 at 13 04 15" src="https://user-images.githubusercontent.com/68591/233620752-d752dada-9c97-4f00-933a-5425e19a5793.png"> ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this issue
Apr 28, 2023
) ## Summary As described in elastic#152318, we noticed that building block alerts were not highlighted anymore after the migration to the new alerts table. A preferred implementation of building block alert highlighting would follow the [`EUIDataGrid` approach of row highlighting](https://eui.elastic.co/#/tabular-content/data-grid-style-display#grid-row-classes). The `DataGrid` allows you to pass custom CSS class names for each row (`gridStyle.rowClasses`). That would allow us to highlight table rows with building block alerts. However, without access to the underlying data, we would not be able generate the correct `rowClasses` for rows with building block alerts. So simply passing `gridStyle.rowClasses` to the `AlertsStateTable` was not an option. Therefore in this PR we're introducing a new prop on the `AlertsTable`, the `highlightedRowMapper`. It's a callback function that receives the alert data and when it returns true, that row will be highlighted. This allows for highlighting of rows from the outside without exposing too many details about the underlying data structures. **Screenshot of the alerts table with a highlightedRowMapper that highlights building block alerts** <img width="1259" alt="Screenshot 2023-04-21 at 13 03 54" src="https://user-images.githubusercontent.com/68591/233620704-a56204c0-e285-4289-897a-58481f440446.png"> ### Additional notes - Since the alerts table has default grid styles, it allows to pass `gridStyle` and it computes its own `rowClasses` for "active row" highlighting, the logic for merging all those styles looks intimidating. I tried my best to comment that part of code to make it clear why the merges are necessary and how they work. - While working on the issue, I noticed that active rows are not highlighted anymore (related bug: elastic#155487). The changes in this PR fix that behaviour as well as you can see in the screenshot below: <img width="936" alt="Screenshot 2023-04-21 at 13 04 15" src="https://user-images.githubusercontent.com/68591/233620752-d752dada-9c97-4f00-933a-5425e19a5793.png"> ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit ce71264)
kibanamachine
added a commit
that referenced
this issue
May 2, 2023
…) (#156137) # Backport This will backport the following commits from `main` to `8.8`: - [[SecuritySolution] Fix building block alert highlighting (#155497)](#155497) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Jan Monschke","email":"jan.monschke@elastic.co"},"sourceCommit":{"committedDate":"2023-04-28T07:50:21Z","message":"[SecuritySolution] Fix building block alert highlighting (#155497)\n\n## Summary\r\n\r\nAs described in #152318, we\r\nnoticed that building block alerts were not highlighted anymore after\r\nthe migration to the new alerts table.\r\n\r\nA preferred implementation of building block alert highlighting would\r\nfollow the [`EUIDataGrid` approach of row\r\nhighlighting](https://eui.elastic.co/#/tabular-content/data-grid-style-display#grid-row-classes).\r\nThe `DataGrid` allows you to pass custom CSS class names for each row\r\n(`gridStyle.rowClasses`). That would allow us to highlight table rows\r\nwith building block alerts.\r\n\r\nHowever, without access to the underlying data, we would not be able\r\ngenerate the correct `rowClasses` for rows with building block alerts.\r\nSo simply passing `gridStyle.rowClasses` to the `AlertsStateTable` was\r\nnot an option.\r\n\r\nTherefore in this PR we're introducing a new prop on the `AlertsTable`,\r\nthe `highlightedRowMapper`. It's a callback function that receives the\r\nalert data and when it returns true, that row will be highlighted.\r\n\r\nThis allows for highlighting of rows from the outside without exposing\r\ntoo many details about the underlying data structures.\r\n\r\n**Screenshot of the alerts table with a highlightedRowMapper that\r\nhighlights building block alerts**\r\n\r\n<img width=\"1259\" alt=\"Screenshot 2023-04-21 at 13 03 54\"\r\nsrc=\"https://user-images.githubusercontent.com/68591/233620704-a56204c0-e285-4289-897a-58481f440446.png\">\r\n\r\n### Additional notes\r\n\r\n- Since the alerts table has default grid styles, it allows to pass\r\n`gridStyle` and it computes its own `rowClasses` for \"active row\"\r\nhighlighting, the logic for merging all those styles looks intimidating.\r\nI tried my best to comment that part of code to make it clear why the\r\nmerges are necessary and how they work.\r\n- While working on the issue, I noticed that active rows are not\r\nhighlighted anymore (related bug:\r\nhttps://github.com//issues/155487). The changes in this PR\r\nfix that behaviour as well as you can see in the screenshot below:\r\n\r\n<img width=\"936\" alt=\"Screenshot 2023-04-21 at 13 04 15\"\r\nsrc=\"https://user-images.githubusercontent.com/68591/233620752-d752dada-9c97-4f00-933a-5425e19a5793.png\">\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>","sha":"ce71264d88c46c45ed8ac00dc6c74bcbd05809c7","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:ResponseOps","Team:Threat Hunting:Investigations","backport:prev-minor","v8.8.0","v8.9.0"],"number":155497,"url":"#155497 Fix building block alert highlighting (#155497)\n\n## Summary\r\n\r\nAs described in #152318, we\r\nnoticed that building block alerts were not highlighted anymore after\r\nthe migration to the new alerts table.\r\n\r\nA preferred implementation of building block alert highlighting would\r\nfollow the [`EUIDataGrid` approach of row\r\nhighlighting](https://eui.elastic.co/#/tabular-content/data-grid-style-display#grid-row-classes).\r\nThe `DataGrid` allows you to pass custom CSS class names for each row\r\n(`gridStyle.rowClasses`). That would allow us to highlight table rows\r\nwith building block alerts.\r\n\r\nHowever, without access to the underlying data, we would not be able\r\ngenerate the correct `rowClasses` for rows with building block alerts.\r\nSo simply passing `gridStyle.rowClasses` to the `AlertsStateTable` was\r\nnot an option.\r\n\r\nTherefore in this PR we're introducing a new prop on the `AlertsTable`,\r\nthe `highlightedRowMapper`. It's a callback function that receives the\r\nalert data and when it returns true, that row will be highlighted.\r\n\r\nThis allows for highlighting of rows from the outside without exposing\r\ntoo many details about the underlying data structures.\r\n\r\n**Screenshot of the alerts table with a highlightedRowMapper that\r\nhighlights building block alerts**\r\n\r\n<img width=\"1259\" alt=\"Screenshot 2023-04-21 at 13 03 54\"\r\nsrc=\"https://user-images.githubusercontent.com/68591/233620704-a56204c0-e285-4289-897a-58481f440446.png\">\r\n\r\n### Additional notes\r\n\r\n- Since the alerts table has default grid styles, it allows to pass\r\n`gridStyle` and it computes its own `rowClasses` for \"active row\"\r\nhighlighting, the logic for merging all those styles looks intimidating.\r\nI tried my best to comment that part of code to make it clear why the\r\nmerges are necessary and how they work.\r\n- While working on the issue, I noticed that active rows are not\r\nhighlighted anymore (related bug:\r\nhttps://github.com//issues/155487). The changes in this PR\r\nfix that behaviour as well as you can see in the screenshot below:\r\n\r\n<img width=\"936\" alt=\"Screenshot 2023-04-21 at 13 04 15\"\r\nsrc=\"https://user-images.githubusercontent.com/68591/233620752-d752dada-9c97-4f00-933a-5425e19a5793.png\">\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>","sha":"ce71264d88c46c45ed8ac00dc6c74bcbd05809c7"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"#155497 Fix building block alert highlighting (#155497)\n\n## Summary\r\n\r\nAs described in #152318, we\r\nnoticed that building block alerts were not highlighted anymore after\r\nthe migration to the new alerts table.\r\n\r\nA preferred implementation of building block alert highlighting would\r\nfollow the [`EUIDataGrid` approach of row\r\nhighlighting](https://eui.elastic.co/#/tabular-content/data-grid-style-display#grid-row-classes).\r\nThe `DataGrid` allows you to pass custom CSS class names for each row\r\n(`gridStyle.rowClasses`). That would allow us to highlight table rows\r\nwith building block alerts.\r\n\r\nHowever, without access to the underlying data, we would not be able\r\ngenerate the correct `rowClasses` for rows with building block alerts.\r\nSo simply passing `gridStyle.rowClasses` to the `AlertsStateTable` was\r\nnot an option.\r\n\r\nTherefore in this PR we're introducing a new prop on the `AlertsTable`,\r\nthe `highlightedRowMapper`. It's a callback function that receives the\r\nalert data and when it returns true, that row will be highlighted.\r\n\r\nThis allows for highlighting of rows from the outside without exposing\r\ntoo many details about the underlying data structures.\r\n\r\n**Screenshot of the alerts table with a highlightedRowMapper that\r\nhighlights building block alerts**\r\n\r\n<img width=\"1259\" alt=\"Screenshot 2023-04-21 at 13 03 54\"\r\nsrc=\"https://user-images.githubusercontent.com/68591/233620704-a56204c0-e285-4289-897a-58481f440446.png\">\r\n\r\n### Additional notes\r\n\r\n- Since the alerts table has default grid styles, it allows to pass\r\n`gridStyle` and it computes its own `rowClasses` for \"active row\"\r\nhighlighting, the logic for merging all those styles looks intimidating.\r\nI tried my best to comment that part of code to make it clear why the\r\nmerges are necessary and how they work.\r\n- While working on the issue, I noticed that active rows are not\r\nhighlighted anymore (related bug:\r\nhttps://github.com//issues/155487). The changes in this PR\r\nfix that behaviour as well as you can see in the screenshot below:\r\n\r\n<img width=\"936\" alt=\"Screenshot 2023-04-21 at 13 04 15\"\r\nsrc=\"https://user-images.githubusercontent.com/68591/233620752-d752dada-9c97-4f00-933a-5425e19a5793.png\">\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>","sha":"ce71264d88c46c45ed8ac00dc6c74bcbd05809c7"}}]}] BACKPORT--> Co-authored-by: Jan Monschke <jan.monschke@elastic.co>
![https://user-images.githubusercontent.com/68591/233620704-a56204c0-e285-4289-897a-58481f440446.png\">\r\n\r\n###](https://user-images.githubusercontent.com/68591/233620704-a56204c0-e285-4289-897a-58481f440446.png\">\r\n\r\n###){kind=link}
![https://user-images.githubusercontent.com/68591/233620752-d752dada-9c97-4f00-933a-5425e19a5793.png\">\r\n\r\n###](https://user-images.githubusercontent.com/68591/233620752-d752dada-9c97-4f00-933a-5425e19a5793.png\">\r\n\r\n###){kind=link}
|
Fixed in #155497 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Kibana version: 8.8
Describe the bug:
The active alert table row is not highlighted when the flyout is open.
Steps to reproduce:
Observability -> AlertsView alert detailsfrom the three dot-menuExpected behavior:
The first row should be highlighted, since its flyout is open.
Screenshots (if relevant):
Screen.Recording.2023-04-21.at.10.36.19.mov
Any additional context:
We suspect this to have happened when security switched to the new alert table: #149128
The text was updated successfully, but these errors were encountered: