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.

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

fix: [Rules][AXE-CORE]: Links must have discernible text #177198

Merged
merged 5 commits into from
Feb 22, 2024

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Feb 19, 2024

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

Summary

The axe browser plugin has identified four links without clear text. Upon inspecting the page, it was found that the SVG icons associated with each rule type are labeled as but with a negative tabindex, rendering them inaccessible for keyboard navigation. Additionally, these icons lack an accessible label.

To address this issue, the tag for the SVG icons has been removed. SVGs inherently possess the appropriate role and aria-hidden attributes, designating them as decorative elements for assistive technology.

Screen

image

AXE Report

Before

image

After

image

For discussion only

Alternatively I recommend to refactor LandingLinkIcon component to use EuiCard to make it more EUI friendly e.g.
image

POC: Alternative fix for

@alexwizp alexwizp marked this pull request as ready for review February 19, 2024 14:28
@alexwizp alexwizp requested a review from a team as a code owner February 19, 2024 14:28
@alexwizp alexwizp added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team labels Feb 20, 2024
@elasticmachine
Copy link
Contributor

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

@alexwizp alexwizp added v8.14.0 Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. and removed Team:Threat Hunting Security Solution Threat Hunting Team labels Feb 20, 2024
@elasticmachine
Copy link
Contributor

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

alexwizp added a commit to alexwizp/kibana that referenced this pull request Feb 21, 2024
@alexwizp alexwizp added v8.13.0 Feature:Rule Management Security Solution Detection Rule Management area Team:Detections and Resp Security Detection Response Team Team:Detection Rule Management Security Detection Rule Management Team labels Feb 21, 2024
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@alexwizp
Copy link
Contributor Author

@elastic/security-threat-hunting-explore please have a look

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

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 11.6MB 11.6MB -231.0B
securitySolutionServerless 183.8KB 183.5KB -231.0B
total -462.0B

History

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

@semd
Copy link
Contributor

semd commented Feb 22, 2024

@alexwizp The alternative version using EuiCard looks good, but we would also need to consider the usage of LandingLinksIconsGroups component that we render in the Assets page (serverless only). We should re-think this one as well.

assets_page

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

LGTM!

@alexwizp alexwizp merged commit 4e30d4b into elastic:main Feb 22, 2024
33 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 22, 2024
)

Closes: elastic/security-team#8565

## Summary

The [axe browser plugin](https://deque.com/axe) has identified four
links without clear text. Upon inspecting the page, it was found that
the SVG icons associated with each rule type are labeled as <a> but with
a negative tabindex, rendering them inaccessible for keyboard
navigation. Additionally, these icons lack an accessible label.

To address this issue, the <a> tag for the SVG icons has been removed.
SVGs inherently possess the appropriate role and aria-hidden attributes,
designating them as decorative elements for assistive technology.

### Screen

![image](https://github.com/elastic/kibana/assets/20072247/32ae0237-9b89-40e6-8201-416a1f2d4f17)

### AXE Report

#### Before

![image](https://github.com/elastic/kibana/assets/20072247/8e51168c-2028-422d-af9e-0c4f5aaeb467)

#### After

![image](https://github.com/elastic/kibana/assets/20072247/57b4aa67-84ee-4b46-96c2-d9ccaa49812d)

## For discussion only
Alternatively I recommend to refactor LandingLinkIcon component to use
`EuiCard` to make it more EUI friendly e.g.
<img width="1308" alt="image"
src="https://github.com/elastic/kibana/assets/20072247/ca76191f-bec7-473b-af73-838fcdee76af">

POC: [Alternative fix
for](alexwizp@69c9375)

(cherry picked from commit 4e30d4b)
@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 Feb 27, 2024
) (#177576)

# Backport

This will backport the following commits from `main` to `8.13`:
- [fix: [Rules][AXE-CORE]: Links must have discernible text
(#177198)](#177198)

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

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

<!--BACKPORT [{"author":{"name":"Alexey
Antonov","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-02-22T12:30:58Z","message":"fix:
[Rules][AXE-CORE]: Links must have discernible text (#177198)\n\nCloses:
https://github.com/elastic/security-team/issues/8565\r\n\r\n##
Summary\r\n\r\nThe [axe browser plugin](https://deque.com/axe) has
identified four\r\nlinks without clear text. Upon inspecting the page,
it was found that\r\nthe SVG icons associated with each rule type are
labeled as <a> but with\r\na negative tabindex, rendering them
inaccessible for keyboard\r\nnavigation. Additionally, these icons lack
an accessible label.\r\n\r\nTo address this issue, the <a> tag for the
SVG icons has been removed.\r\nSVGs inherently possess the appropriate
role and aria-hidden attributes,\r\ndesignating them as decorative
elements for assistive technology.\r\n\r\n###
Screen\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/32ae0237-9b89-40e6-8201-416a1f2d4f17)\r\n\r\n###
AXE Report\r\n\r\n####
Before\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/8e51168c-2028-422d-af9e-0c4f5aaeb467)\r\n\r\n\r\n####
After\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/57b4aa67-84ee-4b46-96c2-d9ccaa49812d)\r\n\r\n\r\n\r\n##
For discussion only\r\nAlternatively I recommend to refactor
LandingLinkIcon component to use\r\n`EuiCard` to make it more EUI
friendly e.g.\r\n<img width=\"1308\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/20072247/ca76191f-bec7-473b-af73-838fcdee76af\">\r\n\r\nPOC:
[Alternative
fix\r\nfor](https://github.com/alexwizp/kibana/commit/69c9375192a45e078a5c82cd714415aa726562b3)","sha":"4e30d4bb3699af2ebb7e44250b6530464fffbacb","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","Feature:Rule
Management","Team:Detection Rule
Management","v8.13.0","v8.14.0"],"title":"fix: [Rules][AXE-CORE]: Links
must have discernible
text","number":177198,"url":"https://github.com/elastic/kibana/pull/177198","mergeCommit":{"message":"fix:
[Rules][AXE-CORE]: Links must have discernible text (#177198)\n\nCloses:
https://github.com/elastic/security-team/issues/8565\r\n\r\n##
Summary\r\n\r\nThe [axe browser plugin](https://deque.com/axe) has
identified four\r\nlinks without clear text. Upon inspecting the page,
it was found that\r\nthe SVG icons associated with each rule type are
labeled as <a> but with\r\na negative tabindex, rendering them
inaccessible for keyboard\r\nnavigation. Additionally, these icons lack
an accessible label.\r\n\r\nTo address this issue, the <a> tag for the
SVG icons has been removed.\r\nSVGs inherently possess the appropriate
role and aria-hidden attributes,\r\ndesignating them as decorative
elements for assistive technology.\r\n\r\n###
Screen\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/32ae0237-9b89-40e6-8201-416a1f2d4f17)\r\n\r\n###
AXE Report\r\n\r\n####
Before\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/8e51168c-2028-422d-af9e-0c4f5aaeb467)\r\n\r\n\r\n####
After\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/57b4aa67-84ee-4b46-96c2-d9ccaa49812d)\r\n\r\n\r\n\r\n##
For discussion only\r\nAlternatively I recommend to refactor
LandingLinkIcon component to use\r\n`EuiCard` to make it more EUI
friendly e.g.\r\n<img width=\"1308\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/20072247/ca76191f-bec7-473b-af73-838fcdee76af\">\r\n\r\nPOC:
[Alternative
fix\r\nfor](https://github.com/alexwizp/kibana/commit/69c9375192a45e078a5c82cd714415aa726562b3)","sha":"4e30d4bb3699af2ebb7e44250b6530464fffbacb"}},"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/177198","number":177198,"mergeCommit":{"message":"fix:
[Rules][AXE-CORE]: Links must have discernible text (#177198)\n\nCloses:
https://github.com/elastic/security-team/issues/8565\r\n\r\n##
Summary\r\n\r\nThe [axe browser plugin](https://deque.com/axe) has
identified four\r\nlinks without clear text. Upon inspecting the page,
it was found that\r\nthe SVG icons associated with each rule type are
labeled as <a> but with\r\na negative tabindex, rendering them
inaccessible for keyboard\r\nnavigation. Additionally, these icons lack
an accessible label.\r\n\r\nTo address this issue, the <a> tag for the
SVG icons has been removed.\r\nSVGs inherently possess the appropriate
role and aria-hidden attributes,\r\ndesignating them as decorative
elements for assistive technology.\r\n\r\n###
Screen\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/32ae0237-9b89-40e6-8201-416a1f2d4f17)\r\n\r\n###
AXE Report\r\n\r\n####
Before\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/8e51168c-2028-422d-af9e-0c4f5aaeb467)\r\n\r\n\r\n####
After\r\n\r\n\r\n![image](https://github.com/elastic/kibana/assets/20072247/57b4aa67-84ee-4b46-96c2-d9ccaa49812d)\r\n\r\n\r\n\r\n##
For discussion only\r\nAlternatively I recommend to refactor
LandingLinkIcon component to use\r\n`EuiCard` to make it more EUI
friendly e.g.\r\n<img width=\"1308\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/20072247/ca76191f-bec7-473b-af73-838fcdee76af\">\r\n\r\nPOC:
[Alternative
fix\r\nfor](https://github.com/alexwizp/kibana/commit/69c9375192a45e078a5c82cd714415aa726562b3)","sha":"4e30d4bb3699af2ebb7e44250b6530464fffbacb"}}]}]
BACKPORT-->

Co-authored-by: Alexey Antonov <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
)

Closes: elastic/security-team#8565

## Summary

The [axe browser plugin](https://deque.com/axe) has identified four
links without clear text. Upon inspecting the page, it was found that
the SVG icons associated with each rule type are labeled as <a> but with
a negative tabindex, rendering them inaccessible for keyboard
navigation. Additionally, these icons lack an accessible label.

To address this issue, the <a> tag for the SVG icons has been removed.
SVGs inherently possess the appropriate role and aria-hidden attributes,
designating them as decorative elements for assistive technology.

### Screen


![image](https://github.com/elastic/kibana/assets/20072247/32ae0237-9b89-40e6-8201-416a1f2d4f17)

### AXE Report

#### Before


![image](https://github.com/elastic/kibana/assets/20072247/8e51168c-2028-422d-af9e-0c4f5aaeb467)


#### After


![image](https://github.com/elastic/kibana/assets/20072247/57b4aa67-84ee-4b46-96c2-d9ccaa49812d)



## For discussion only
Alternatively I recommend to refactor LandingLinkIcon component to use
`EuiCard` to make it more EUI friendly e.g.
<img width="1308" alt="image"
src="https://github.com/elastic/kibana/assets/20072247/ca76191f-bec7-473b-af73-838fcdee76af">

POC: [Alternative fix
for](alexwizp@69c9375)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Rule Management Security Solution Detection Rule Management area release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team 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
Development

Successfully merging this pull request may close these issues.

5 participants