[Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues #207122
+240
−9
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
… accessibility (a11y) issues This PR fixes the following Security Assistant accessibility (a11y) issues: - <elastic#206348> - _The ai assistant settings and actions button is announced wrong_ - <elastic#206362> - _Close button on View in AI assistant is missing discernible text_ - <elastic#206875> - _Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled_ ### Details #### [206348](elastic#206348) - The ai assistant settings and actions button is announced wrong This issue was resolved by adding an `aria-label` to the assistant settings context menu. This fix was desk tested using Voiceover, as illustrated by the following screenshots: **Before:**  **After:**  Desk testing: see [206348](elastic#206348) for reproduction steps #### [206362](elastic#206362) - Close button on View in AI assistant is missing discernible text This issue was resolved by adding an `aria-label` to the assistant close button. This fix was desk tested using Axe, as illustrated by the following screenshots: **Before:**  **After:**  Desk testing: see [206362](elastic#206362) for reproduction steps #### [206875](elastic#206875) - Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled Issue [206875](elastic#206875) includes the following statement: > Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets disabled. All it says right now "show anonymized" The first part of the statement above: > Anonymization button doesn't get announced appears to be in reference to when the Anonymization toggle button is disabled. This is unfortunately expected, because screen readers do NOT announce disabled buttons, as described in articles like <https://css-tricks.com/making-disabled-buttons-more-inclusive/> The second part of the statement above: > doesn't have enough context in the tooltip about when it gets enabled is addressed by this PR, though there is still a quirk described in detail below. In this PR, when a conversation does NOT have replacements, a new (different) tooltip is displayed, as illustrated by the before / after screenshots below: **Before:**  _Above: Before the fix, the tooltip for the disabled button reads:_ `Show anonymized` **After:**  _Above: After the fix, the tooltip for the disabled button reads:_ `This conversation does not include anonymized fields` Note that there is still a quirk with the button, which is not addressed by this fix: The current implementation enables the `Show anonymized` button when the conversation has _any_ replacements, regardless of whether or not the replacements are applicable to the rendered conversation. As a result, when replacements are present, but not applicable to the rendered conversation, the user may toggle the enabled button, but will not observe any changes to the rendered conversation. Alternatively, the replacements could be applied to the conversation before rendering to facilitate a comparison: If the original conversation and applied conversation are identical, the anonymization button should be disabled. If they are the different, the button should be enabled. This alternative was NOT implemented in this PR. Desk testing: see [206875](elastic#206875) for reproduction steps
andrew-goldstein
added
release_note:skip
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
@elasticmachine merge upstream |
stephmilovic
approved these changes
Jan 22, 2025
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
|
|
Starting backport for target branches: 8.x |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Jan 22, 2025
…essibility (a11y) issues (elastic#207122) ### [Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues This PR fixes the following Security Assistant accessibility (a11y) issues: - <elastic#206348> - _The ai assistant settings and actions button is announced wrong_ - <elastic#206362> - _Close button on View in AI assistant is missing discernible text_ - <elastic#206875> - _Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled_ ### Details #### [206348](elastic#206348) - The ai assistant settings and actions button is announced wrong This issue was resolved by adding an `aria-label` to the assistant settings context menu. This fix was desk tested using Voiceover, as illustrated by the following screenshots: **Before:**  **After:**  Desk testing: see [206348](elastic#206348) for reproduction steps #### [206362](elastic#206362) - Close button on View in AI assistant is missing discernible text This issue was resolved by adding an `aria-label` to the assistant close button. This fix was desk tested using Axe, as illustrated by the following screenshots: **Before:**  **After:**  Desk testing: see [206362](elastic#206362) for reproduction steps #### [206875](elastic#206875) - Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled Issue [206875](elastic#206875) includes the following statement: > Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets disabled. All it says right now "show anonymized" The first part of the statement above: > Anonymization button doesn't get announced appears to be in reference to when the Anonymization toggle button is disabled. This is unfortunately expected, because screen readers do NOT announce disabled buttons, as described in articles like <https://css-tricks.com/making-disabled-buttons-more-inclusive/> The second part of the statement above: > doesn't have enough context in the tooltip about when it gets enabled is addressed by this PR, though there is still a quirk described in detail below. In this PR, when a conversation does NOT have replacements, a new (different) tooltip is displayed, as illustrated by the before / after screenshots below: **Before:**  _Above: Before the fix, the tooltip for the disabled button reads:_ `Show anonymized` **After:**  _Above: After the fix, the tooltip for the disabled button reads:_ `This conversation does not include anonymized fields` Note that there is still a quirk with the button, which is not addressed by this fix: The current implementation enables the `Show anonymized` button when the conversation has _any_ replacements, regardless of whether or not the replacements are applicable to the rendered conversation. As a result, when replacements are present, but not applicable to the rendered conversation, the user may toggle the enabled button, but will not observe any changes to the rendered conversation. Alternatively, the replacements could be applied to the conversation before rendering to facilitate a comparison: If the original conversation and applied conversation are identical, the anonymization button should be disabled. If they are the different, the button should be enabled. This alternative was NOT implemented in this PR. Desk testing: see [206875](elastic#206875) for reproduction steps (cherry picked from commit 0e715b6)
💚 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
Jan 22, 2025
…nt accessibility (a11y) issues (#207122) (#207895) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues (#207122)](#207122) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Andrew Macri","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-22T18:06:54Z","message":"[Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues (#207122)\n\n### [Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues\r\n\r\nThis PR fixes the following Security Assistant accessibility (a11y) issues:\r\n\r\n- <#206348> - _The ai assistant settings and actions button is announced wrong_\r\n- <#206362> - _Close button on View in AI assistant is missing discernible text_\r\n- <#206875> - _Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled_\r\n\r\n### Details\r\n\r\n#### [206348](#206348) - The ai assistant settings and actions button is announced wrong\r\n\r\nThis issue was resolved by adding an `aria-label` to the assistant settings context menu.\r\n\r\nThis fix was desk tested using Voiceover, as illustrated by the following screenshots:\r\n\r\n**Before:**\r\n\r\n\r\n\r\n**After:**\r\n\r\n\r\n\r\nDesk testing: see [206348](#206348) for reproduction steps\r\n\r\n#### [206362](#206362) - Close button on View in AI assistant is missing discernible text\r\n\r\nThis issue was resolved by adding an `aria-label` to the assistant close button.\r\n\r\nThis fix was desk tested using Axe, as illustrated by the following screenshots:\r\n\r\n**Before:**\r\n\r\n\r\n\r\n**After:**\r\n\r\n\r\n\r\nDesk testing: see [206362](#206362) for reproduction steps\r\n\r\n#### [206875](#206875) - Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled\r\n\r\nIssue [206875](#206875) includes the following statement:\r\n\r\n> Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets disabled. All it says right now \"show anonymized\"\r\n\r\nThe first part of the statement above:\r\n\r\n> Anonymization button doesn't get announced\r\n\r\nappears to be in reference to when the Anonymization toggle button is disabled. This is unfortunately expected, because screen readers do NOT announce disabled buttons, as described in articles like <https://css-tricks.com/making-disabled-buttons-more-inclusive/>\r\n\r\nThe second part of the statement above:\r\n\r\n> doesn't have enough context in the tooltip about when it gets enabled\r\n\r\nis addressed by this PR, though there is still a quirk described in detail below.\r\n\r\nIn this PR, when a conversation does NOT have replacements, a new (different) tooltip is displayed, as illustrated by the before / after screenshots below:\r\n\r\n**Before:**\r\n\r\n\r\n\r\n_Above: Before the fix, the tooltip for the disabled button reads:_ `Show anonymized`\r\n\r\n**After:**\r\n\r\n\r\n\r\n_Above: After the fix, the tooltip for the disabled button reads:_ `This conversation does not include anonymized fields`\r\n\r\nNote that there is still a quirk with the button, which is not addressed by this fix:\r\n\r\nThe current implementation enables the `Show anonymized` button when the conversation has _any_ replacements, regardless of whether or not the replacements are applicable to the rendered conversation. As a result, when replacements are present, but not applicable to the rendered conversation, the user may toggle the enabled button, but will not observe any changes to the rendered conversation.\r\n\r\nAlternatively, the replacements could be applied to the conversation before rendering to facilitate a comparison: If the original conversation and applied conversation are identical, the anonymization button should be disabled. If they are the different, the button should be enabled. This alternative was NOT implemented in this PR.\r\n\r\nDesk testing: see [206875](#206875) for reproduction steps","sha":"0e715b650e402c4bd7d7391e5c009ff64f896ef8","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team: SecuritySolution","Team:Security Generative AI","backport:version","v8.18.0"],"title":"[Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues","number":207122,"url":"https://github.com/elastic/kibana/pull/207122","mergeCommit":{"message":"[Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues (#207122)\n\n### [Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues\r\n\r\nThis PR fixes the following Security Assistant accessibility (a11y) issues:\r\n\r\n- <#206348> - _The ai assistant settings and actions button is announced wrong_\r\n- <#206362> - _Close button on View in AI assistant is missing discernible text_\r\n- <#206875> - _Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled_\r\n\r\n### Details\r\n\r\n#### [206348](#206348) - The ai assistant settings and actions button is announced wrong\r\n\r\nThis issue was resolved by adding an `aria-label` to the assistant settings context menu.\r\n\r\nThis fix was desk tested using Voiceover, as illustrated by the following screenshots:\r\n\r\n**Before:**\r\n\r\n\r\n\r\n**After:**\r\n\r\n\r\n\r\nDesk testing: see [206348](#206348) for reproduction steps\r\n\r\n#### [206362](#206362) - Close button on View in AI assistant is missing discernible text\r\n\r\nThis issue was resolved by adding an `aria-label` to the assistant close button.\r\n\r\nThis fix was desk tested using Axe, as illustrated by the following screenshots:\r\n\r\n**Before:**\r\n\r\n\r\n\r\n**After:**\r\n\r\n\r\n\r\nDesk testing: see [206362](#206362) for reproduction steps\r\n\r\n#### [206875](#206875) - Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled\r\n\r\nIssue [206875](#206875) includes the following statement:\r\n\r\n> Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets disabled. All it says right now \"show anonymized\"\r\n\r\nThe first part of the statement above:\r\n\r\n> Anonymization button doesn't get announced\r\n\r\nappears to be in reference to when the Anonymization toggle button is disabled. This is unfortunately expected, because screen readers do NOT announce disabled buttons, as described in articles like <https://css-tricks.com/making-disabled-buttons-more-inclusive/>\r\n\r\nThe second part of the statement above:\r\n\r\n> doesn't have enough context in the tooltip about when it gets enabled\r\n\r\nis addressed by this PR, though there is still a quirk described in detail below.\r\n\r\nIn this PR, when a conversation does NOT have replacements, a new (different) tooltip is displayed, as illustrated by the before / after screenshots below:\r\n\r\n**Before:**\r\n\r\n\r\n\r\n_Above: Before the fix, the tooltip for the disabled button reads:_ `Show anonymized`\r\n\r\n**After:**\r\n\r\n\r\n\r\n_Above: After the fix, the tooltip for the disabled button reads:_ `This conversation does not include anonymized fields`\r\n\r\nNote that there is still a quirk with the button, which is not addressed by this fix:\r\n\r\nThe current implementation enables the `Show anonymized` button when the conversation has _any_ replacements, regardless of whether or not the replacements are applicable to the rendered conversation. As a result, when replacements are present, but not applicable to the rendered conversation, the user may toggle the enabled button, but will not observe any changes to the rendered conversation.\r\n\r\nAlternatively, the replacements could be applied to the conversation before rendering to facilitate a comparison: If the original conversation and applied conversation are identical, the anonymization button should be disabled. If they are the different, the button should be enabled. This alternative was NOT implemented in this PR.\r\n\r\nDesk testing: see [206875](#206875) for reproduction steps","sha":"0e715b650e402c4bd7d7391e5c009ff64f896ef8"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/207122","number":207122,"mergeCommit":{"message":"[Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues (#207122)\n\n### [Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues\r\n\r\nThis PR fixes the following Security Assistant accessibility (a11y) issues:\r\n\r\n- <#206348> - _The ai assistant settings and actions button is announced wrong_\r\n- <#206362> - _Close button on View in AI assistant is missing discernible text_\r\n- <#206875> - _Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled_\r\n\r\n### Details\r\n\r\n#### [206348](#206348) - The ai assistant settings and actions button is announced wrong\r\n\r\nThis issue was resolved by adding an `aria-label` to the assistant settings context menu.\r\n\r\nThis fix was desk tested using Voiceover, as illustrated by the following screenshots:\r\n\r\n**Before:**\r\n\r\n\r\n\r\n**After:**\r\n\r\n\r\n\r\nDesk testing: see [206348](#206348) for reproduction steps\r\n\r\n#### [206362](#206362) - Close button on View in AI assistant is missing discernible text\r\n\r\nThis issue was resolved by adding an `aria-label` to the assistant close button.\r\n\r\nThis fix was desk tested using Axe, as illustrated by the following screenshots:\r\n\r\n**Before:**\r\n\r\n\r\n\r\n**After:**\r\n\r\n\r\n\r\nDesk testing: see [206362](#206362) for reproduction steps\r\n\r\n#### [206875](#206875) - Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled\r\n\r\nIssue [206875](#206875) includes the following statement:\r\n\r\n> Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets disabled. All it says right now \"show anonymized\"\r\n\r\nThe first part of the statement above:\r\n\r\n> Anonymization button doesn't get announced\r\n\r\nappears to be in reference to when the Anonymization toggle button is disabled. This is unfortunately expected, because screen readers do NOT announce disabled buttons, as described in articles like <https://css-tricks.com/making-disabled-buttons-more-inclusive/>\r\n\r\nThe second part of the statement above:\r\n\r\n> doesn't have enough context in the tooltip about when it gets enabled\r\n\r\nis addressed by this PR, though there is still a quirk described in detail below.\r\n\r\nIn this PR, when a conversation does NOT have replacements, a new (different) tooltip is displayed, as illustrated by the before / after screenshots below:\r\n\r\n**Before:**\r\n\r\n\r\n\r\n_Above: Before the fix, the tooltip for the disabled button reads:_ `Show anonymized`\r\n\r\n**After:**\r\n\r\n\r\n\r\n_Above: After the fix, the tooltip for the disabled button reads:_ `This conversation does not include anonymized fields`\r\n\r\nNote that there is still a quirk with the button, which is not addressed by this fix:\r\n\r\nThe current implementation enables the `Show anonymized` button when the conversation has _any_ replacements, regardless of whether or not the replacements are applicable to the rendered conversation. As a result, when replacements are present, but not applicable to the rendered conversation, the user may toggle the enabled button, but will not observe any changes to the rendered conversation.\r\n\r\nAlternatively, the replacements could be applied to the conversation before rendering to facilitate a comparison: If the original conversation and applied conversation are identical, the anonymization button should be disabled. If they are the different, the button should be enabled. This alternative was NOT implemented in this PR.\r\n\r\nDesk testing: see [206875](#206875) for reproduction steps","sha":"0e715b650e402c4bd7d7391e5c009ff64f896ef8"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Andrew Macri <[email protected]>
This was referenced Jan 22, 2025
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.
[Security Solution] [Security Assistant] Fixes Security Assistant accessibility (a11y) issues
This PR fixes the following Security Assistant accessibility (a11y) issues:
Details
206348 - The ai assistant settings and actions button is announced wrong
This issue was resolved by adding an
aria-labelto the assistant settings context menu.This fix was desk tested using Voiceover, as illustrated by the following screenshots:
Before:
After:
Desk testing: see 206348 for reproduction steps
206362 - Close button on View in AI assistant is missing discernible text
This issue was resolved by adding an
aria-labelto the assistant close button.This fix was desk tested using Axe, as illustrated by the following screenshots:
Before:
After:
Desk testing: see 206362 for reproduction steps
206875 - Anonymization button doesn't get announced and doesn't have enough context in the tooltip about when it gets enabled
Issue 206875 includes the following statement:
The first part of the statement above:
appears to be in reference to when the Anonymization toggle button is disabled. This is unfortunately expected, because screen readers do NOT announce disabled buttons, as described in articles like https://css-tricks.com/making-disabled-buttons-more-inclusive/
The second part of the statement above:
is addressed by this PR, though there is still a quirk described in detail below.
In this PR, when a conversation does NOT have replacements, a new (different) tooltip is displayed, as illustrated by the before / after screenshots below:
Before:
Above: Before the fix, the tooltip for the disabled button reads:
Show anonymizedAfter:
Above: After the fix, the tooltip for the disabled button reads:
This conversation does not include anonymized fieldsNote that there is still a quirk with the button, which is not addressed by this fix:
The current implementation enables the
Show anonymizedbutton when the conversation has any replacements, regardless of whether or not the replacements are applicable to the rendered conversation. As a result, when replacements are present, but not applicable to the rendered conversation, the user may toggle the enabled button, but will not observe any changes to the rendered conversation.Alternatively, the replacements could be applied to the conversation before rendering to facilitate a comparison: If the original conversation and applied conversation are identical, the anonymization button should be disabled. If they are the different, the button should be enabled. This alternative was NOT implemented in this PR.
Desk testing: see 206875 for reproduction steps