Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: [Rules > Shared exception lists][AXE-CORE]: Interactive controls must not be nested #178023

Merged
merged 8 commits into from
Mar 14, 2024
Merged

fix: [Rules > Shared exception lists][AXE-CORE]: Interactive controls must not be nested #178023

merged 8 commits into from
Mar 14, 2024

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Mar 5, 2024 •
edited by kibanamachine
Loading

Closes: https://github.com/elastic/security-team/issues/8611

Description

The axe browser plugin is reporting a nested button (interactive control)in the shared exception list accordion(s). The accordion has a button to open/close and a link to the list detail view. Screenshot attached below.

Steps to recreate

  1. Open the Security Dashboards, then click Rules > Shared Exception Lists
  2. Run an axe browser scan in Chrome, Edge, or Firefox
  3. Verify the nested control error

What was done?

  1. Solution proposed by EUI team was applied. See [EuiAccordion] Do not set arrowDisplay to none if the buttonElement is a div. eui#7545

  2. The component ExceptionsListCard has been slightly changed to make it more responsive.
    Before:

    After:
Screen.Recording.2024-03-13.at.12.22.47.mov

Screen

AXE Report

image

@alexwizp
Copy link
Contributor Author

alexwizp commented Mar 5, 2024

/ci

@alexwizp alexwizp added v8.13.0 v8.14.0 release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:Detection & Response Dashboard Security solution Detection & Response Dashboard feature Team:Detection Rule Management Security Detection Rule Management Team Feature:Rule Management Security Solution Detection Rule Management area labels Mar 13, 2024
@alexwizp
Copy link
Contributor Author

/ci

@alexwizp alexwizp marked this pull request as ready for review March 13, 2024 13:37
@alexwizp alexwizp requested a review from a team as a code owner March 13, 2024 13:37
@alexwizp alexwizp requested a review from rylnd March 13, 2024 13:37
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

rylnd
rylnd requested changes Mar 13, 2024
View reviewed changes
Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good to me, but it looks as though the style of the accordion has changed for the worse:

Before:
Screenshot 2024-03-13 at 1 50 19 PM

After:
Screenshot 2024-03-13 at 1 50 31 PM

Can you please try to fix this?

@banderror banderror added Team:Detections and Resp Security Detection Response Team Feature:Rule Exceptions Security Solution Detection Rule Exceptions area Team:Detection Engine Security Solution Detection Engine Area and removed Feature:Rule Management Security Solution Detection Rule Management area Team:Detection Rule Management Security Detection Rule Management Team Feature:Detection & Response Dashboard Security solution Detection & Response Dashboard feature labels Mar 14, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-engine (Team:Detection Engine)

rylnd
rylnd approved these changes Mar 14, 2024
View reviewed changes
Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CSS fixes look good now, things look back to normal:

Screenshot 2024-03-14 at 10 12 20 AM

Thanks!

@alexwizp alexwizp enabled auto-merge (squash) March 14, 2024 15:22
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Defend Workflows Cypress Tests #5 / Automated Response Actions From alerts should have generated endpoint and rule should have generated endpoint and rule

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 12.8MB 12.8MB +370.0B

History

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

@alexwizp alexwizp merged commit b90e215 into elastic:main Mar 14, 2024
39 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 14, 2024
@alexwizp
fix: [Rules > Shared exception lists][AXE-CORE]: Interactive controls…
84c49c8 
… must not be nested (elastic#178023)

Closes: elastic/security-team#8611

## Description

The [axe browser plugin](https://deque.com/axe) is reporting a nested
button (interactive control)in the shared exception list accordion(s).
The accordion has a button to open/close and a link to the list detail
view. Screenshot attached below.

### Steps to recreate

1. Open the Security Dashboards, then click Rules > Shared Exception
Lists
2. Run an axe browser scan in Chrome, Edge, or Firefox
3. Verify the nested control error

### What was done?

1. Solution proposed by EUI team was applied. See
elastic/eui#7545
2. The component `ExceptionsListCard` has been slightly changed to make
it more responsive.
       Before:

       After:

https://github.com/elastic/kibana/assets/20072247/dd6c6681-980c-40ed-98cf-29a71f896bc2

### Screen

#### AXE Report

![image](https://github.com/elastic/kibana/assets/20072247/43313005-b7ec-49d1-9eed-30bfacbb5ecc)

(cherry picked from commit b90e215)
@kibanamachine kibanamachine mentioned this pull request Mar 14, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.13

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 Mar 14, 2024
@kibanamachine @alexwizp
[8.13] fix: [Rules > Shared exception lists][AXE-CORE]: Interactiv…
dfe85fd 
…e controls must not be nested (#178023) (#178754)

# Backport

This will backport the following commits from `main` to `8.13`:
- [fix: [Rules > Shared exception lists][AXE-CORE]: Interactive
controls must not be nested
(#178023)](#178023)

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

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

<!--BACKPORT [{"author":{"name":"Alexey
Antonov","email":"alexwizp@gmail.com"},"sourceCommit":{"committedDate":"2024-03-14T16:02:50Z","message":"fix:
[Rules > Shared exception lists][AXE-CORE]: Interactive controls must
not be nested (#178023)\n\nCloses:
https://github.com/elastic/security-team/issues/8611\r\n\r\n##
Description\r\n\r\nThe [axe browser plugin](https://deque.com/axe) is
reporting a nested\r\nbutton (interactive control)in the shared
exception list accordion(s).\r\nThe accordion has a button to open/close
and a link to the list detail\r\nview. Screenshot attached
below.\r\n\r\n### Steps to recreate\r\n\r\n1. Open the Security
Dashboards, then click Rules > Shared Exception\r\nLists\r\n2. Run an
axe browser scan in Chrome, Edge, or Firefox\r\n3. Verify the nested
control error\r\n\r\n### What was done? \r\n\r\n1. Solution proposed by
EUI team was applied.
See\r\nhttps://github.com/elastic/eui/issues/7545\r\n2. The component
`ExceptionsListCard` has been slightly changed to make\r\nit more
responsive.\r\n Before: \r\n\r\n After:
\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/20072247/dd6c6681-980c-40ed-98cf-29a71f896bc2\r\n\r\n\r\n\r\n\r\n###
Screen\r\n\r\n#### AXE Report
\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/43313005-b7ec-49d1-9eed-30bfacbb5ecc)","sha":"b90e215d9b4a1462c8cb3be692bcc44edda1e6b8","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","Feature:Rule
Exceptions","Team:Detection Engine","v8.13.0","v8.14.0"],"title":"fix:
[Rules > Shared exception lists][AXE-CORE]: Interactive controls must
not be
nested","number":178023,"url":"https://github.com/elastic/kibana/pull/178023","mergeCommit":{"message":"fix:
[Rules > Shared exception lists][AXE-CORE]: Interactive controls must
not be nested (#178023)\n\nCloses:
https://github.com/elastic/security-team/issues/8611\r\n\r\n##
Description\r\n\r\nThe [axe browser plugin](https://deque.com/axe) is
reporting a nested\r\nbutton (interactive control)in the shared
exception list accordion(s).\r\nThe accordion has a button to open/close
and a link to the list detail\r\nview. Screenshot attached
below.\r\n\r\n### Steps to recreate\r\n\r\n1. Open the Security
Dashboards, then click Rules > Shared Exception\r\nLists\r\n2. Run an
axe browser scan in Chrome, Edge, or Firefox\r\n3. Verify the nested
control error\r\n\r\n### What was done? \r\n\r\n1. Solution proposed by
EUI team was applied.
See\r\nhttps://github.com/elastic/eui/issues/7545\r\n2. The component
`ExceptionsListCard` has been slightly changed to make\r\nit more
responsive.\r\n Before: \r\n\r\n After:
\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/20072247/dd6c6681-980c-40ed-98cf-29a71f896bc2\r\n\r\n\r\n\r\n\r\n###
Screen\r\n\r\n#### AXE Report
\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/43313005-b7ec-49d1-9eed-30bfacbb5ecc)","sha":"b90e215d9b4a1462c8cb3be692bcc44edda1e6b8"}},"sourceBranch":"main","suggestedTargetBranches":["8.13"],"targetPullRequestStates":[{"branch":"8.13","label":"v8.13.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/178023","number":178023,"mergeCommit":{"message":"fix:
[Rules > Shared exception lists][AXE-CORE]: Interactive controls must
not be nested (#178023)\n\nCloses:
https://github.com/elastic/security-team/issues/8611\r\n\r\n##
Description\r\n\r\nThe [axe browser plugin](https://deque.com/axe) is
reporting a nested\r\nbutton (interactive control)in the shared
exception list accordion(s).\r\nThe accordion has a button to open/close
and a link to the list detail\r\nview. Screenshot attached
below.\r\n\r\n### Steps to recreate\r\n\r\n1. Open the Security
Dashboards, then click Rules > Shared Exception\r\nLists\r\n2. Run an
axe browser scan in Chrome, Edge, or Firefox\r\n3. Verify the nested
control error\r\n\r\n### What was done? \r\n\r\n1. Solution proposed by
EUI team was applied.
See\r\nhttps://github.com/elastic/eui/issues/7545\r\n2. The component
`ExceptionsListCard` has been slightly changed to make\r\nit more
responsive.\r\n Before: \r\n\r\n After:
\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/20072247/dd6c6681-980c-40ed-98cf-29a71f896bc2\r\n\r\n\r\n\r\n\r\n###
Screen\r\n\r\n#### AXE Report
\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/43313005-b7ec-49d1-9eed-30bfacbb5ecc)","sha":"b90e215d9b4a1462c8cb3be692bcc44edda1e6b8"}}]}]
BACKPORT-->

Co-authored-by: Alexey Antonov <alexwizp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rylnd rylnd rylnd approved these changes

Assignees
No one assigned
Labels
Feature:Rule Exceptions Security Solution Detection Rule Exceptions area release_note:skip Skip the PR/issue when compiling release notes Team:Detection Engine Security Solution Detection Engine Area Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.13.0 v8.14.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@alexwizp @elasticmachine @kibana-ci @kibanamachine @rylnd @banderror