Skip to content

Feature/#94258 extend aria label translations #92

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

Merged
merged 17 commits into from
Apr 14, 2025

Conversation

vj-venkatesan
Copy link
Contributor

Success criteria

Please describe what should be possible after this change. List all individual items on a separate line.

  • All aria-labels should be configurable for accessibility
  • All default aria-labels should be rendered correctly
  • When configured through settings the aria-labels should be rendered accordingly

How to test

Please describe the individual steps on how a peer can test your change.

  1. Open any webchat endpoint
  2. Use the ariaLabels setting inside customTranslations and modify any labels
  3. Inspect the modified label in the dev tools , observe the changes reflected

Security

  • Possible injection vector
  • Authentication/Access controls touched
  • Sensitive Data could be exposed
  • XSS
  • Logging/Monitoring touched
  • Exchanges data with external systems
  • No security implications

Additional considerations

  • This PR might have performance implications

Documentation Considerations

These are hints for the documentation team to help write the docs.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/webchat-ui/components/plugins/input/file/PreviewUploadedFiles.tsx:114

  • [nitpick] The variable name 'removeFileAttachment' is ambiguous since it represents an aria-label value; consider renaming it to 'removeFileAttachmentLabel' to improve clarity.
aria-label={`${removeFileAttachment ?? "Remove File Attachment"} ${index + 1}`}

vj-venkatesan and others added 2 commits April 9, 2025 13:54
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sushmi21
Copy link
Contributor

Looks good. There are some aria-labels in the chat-components repo too. That can go in a follow-up PR as well. Also, in addition to aria-labels, we have hidden screen-reader only texts that also needs translation :(

@vj-venkatesan
Copy link
Contributor Author

vj-venkatesan commented Apr 10, 2025

Also, in addition to aria-labels, we have hidden screen-reader only texts that also needs translation :(

This is not a lot , I can include it in the same PR . wdyt ?

@vj-venkatesan
Copy link
Contributor Author

There are some aria-labels in the chat-components repo too.

Ah I did not know that , are they hard coded there ?

@sushmi21
Copy link
Contributor

There are some aria-labels in the chat-components repo too.

Ah I did not know that , are they hard coded there ?

Yes. There are aria-labels and screen-reader-only texts in chat-components too

@vj-venkatesan
Copy link
Contributor Author

vj-venkatesan commented Apr 11, 2025

Looks good. There are some aria-labels in the chat-components repo too. That can go in a follow-up PR as well. Also, in addition to aria-labels, we have hidden screen-reader only texts that also needs translation :(

I have resolved all your comments and added SR only texts. Could you please check again ?

As agreed will create a follow up for integrating chat components

| thumbsDown | string | "Thumbs down" | Label for the negative feedback button. |
| openConversation | string | "Open conversation" | Label for the button that opens a conversation thread. |

#### Screen reader
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: I think ariaLabels and Screen-reader objects can be combined. Both aria-label and screen-reader only texts are not visible in the screen and used by screen readers. So, it does not make a difference to our customer building a bot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this

@vj-venkatesan vj-venkatesan requested a review from sushmi21 April 14, 2025 14:00
@vj-venkatesan vj-venkatesan merged commit 31bad8c into main Apr 14, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants