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

feat(vwc-banner): add aria live polite #1093

Merged
merged 36 commits into from Oct 28, 2021
Merged

feat(vwc-banner): add aria live polite #1093

merged 36 commits into from Oct 28, 2021

Conversation

rinaok
Copy link
Contributor

@rinaok rinaok commented Oct 12, 2021

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@github-actions
Copy link

🚀

Latest successful build of the PR deployed here.

🚀

components/banner/src/vwc-banner.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@yinonov yinonov left a comment

Choose a reason for hiding this comment

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

banner repeats the text on 1st open
reproduce:

  1. open banner basic story
  2. open screen reader
  3. click close on banner
  4. when banner opens it will pronounce the message twice

Copy link
Contributor

@yinonov yinonov left a comment

Choose a reason for hiding this comment

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

why dont we just import the material accessible snackbar directive?

@yinonov
Copy link
Contributor

yinonov commented Oct 18, 2021

@Vonage/vivid-core any takes on testing screen readers behavior?

@rinaok
Copy link
Contributor Author

rinaok commented Oct 19, 2021

banner repeats the text on 1st open reproduce:

  1. open banner basic story
  2. open screen reader
  3. click close on banner
  4. when banner opens it will pronounce the message twice

why dont we just import the material accessible snackbar directive?

We can import the material accessible snackbar directive, I didn't do it at first as I thought that we might need to make some adjustments, but it works as is.

The repeated text only occurs in the basic story.
Additionally, it is occurring with material's imported class as well as without directives.
It does not occur when you simply toggle the open property in controls.

@rinaok rinaok marked this pull request as draft October 21, 2021 14:13
@rinaok rinaok marked this pull request as ready for review October 26, 2021 12:32
@rinaok rinaok requested a review from yinonov October 26, 2021 12:44
yinonov
yinonov previously approved these changes Oct 26, 2021
@sonarcloud
Copy link

sonarcloud bot commented Oct 27, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@rinaok rinaok requested a review from yinonov October 27, 2021 11:39
@rinaok rinaok merged commit 4f34e7b into master Oct 28, 2021
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.

None yet

2 participants