Skip to content

Feature/84849 & Feature/92930 accessibility fixes #93

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 11 commits into from
Apr 14, 2025

Conversation

sushmi21
Copy link
Contributor

@sushmi21 sushmi21 commented Apr 10, 2025

Success criteria

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

  • Screen reader only home screen title is changed to 'Chat home screen'
  • Target size of the icon buttons are a minimum of 24px by 24px
  • Failed file upload text color has at least 4.5:1 contrast against the grey background
  • Aria-labelledby is added to element with role="log"(Adding a label is making the message announcements very verbose with NVDA, as the label is announced with every sent or received message). Redundant aria-live="polite" is removed
  • Tab index 0 is added to the element with role=log only when there are enough messages to make the log scrollable.
  • Tests are modified to work with the changes in chat-components

How to test

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

  1. Please test the above success criteria by inspecting the DOM

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.

@sushmi21 sushmi21 requested a review from Copilot April 10, 2025 07:33
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 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/webchat-ui/components/history/ChatScroller.tsx:131

  • Ensure that an element with the ID 'webchatChatHistoryHeading' exists in the DOM so that the aria-labelledby reference is properly resolved.
aria-labelledby="webchatChatHistoryHeading"

src/webchat-ui/components/Modal/Modal.tsx:53

  • [nitpick] Verify that the absolute positioning and right offset of -4 for the CloseButton maintains an adequate clickable area across different devices.
position: "absolute", right: -4,

@@ -103,7 +103,7 @@ describe("Bot message", () => {
cy.openWebchat().startConversation();
cy.receiveMessage("bot message", {}, "bot");

cy.get('[alt="bot avatar"]').should(
cy.get('[data-testid="avatar"]').should(
Copy link
Preview

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

The data-testid for the bot avatar is inconsistent with the other tests that use 'bot-avatar'; consider updating it for consistency.

Suggested change
cy.get('[data-testid="avatar"]').should(
cy.get('[data-testid="bot-avatar"]').should(

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

@sushmi21
Copy link
Contributor Author

The tests will fail until I update chat-components to a newer version. I will update the chat-components once Cognigy/chat-components#118 is merged

@sushmi21 sushmi21 requested a review from vj-venkatesan April 10, 2025 09:40
vj-venkatesan
vj-venkatesan previously approved these changes Apr 10, 2025
@sushmi21 sushmi21 changed the title Feature/84849 accessibility fixes Feature/84849 & Feature/92930 accessibility fixes Apr 10, 2025
@vj-venkatesan vj-venkatesan self-requested a review April 14, 2025 08:27
@sushmi21 sushmi21 merged commit 596e279 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