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

Change accessibility test in grunt.js to remove unnecessary warning message #4008

Merged
merged 1 commit into from Feb 7, 2017

Conversation

OwenEdwards
Copy link
Member

Description

Minor change in the grunt file to remove unnecessary warning messages from the accessibility test

Specific Changes proposed

(See above)

Requirements Checklist

  • Feature implemented / Bug fixed
  • Reviewed by Two Core Contributors

Copy link
Contributor

@brandonocasey brandonocasey left a comment

Choose a reason for hiding this comment

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

So this just ignores the two warnings caused by the "please enable JavaScript warning" HTML? Seems like it should be okay to just ignore them, but I am wondering if we want to update an example somewhere that does follow accessibility guidelines.

@OwenEdwards
Copy link
Member Author

The example it tests does follow accessibility guidelines - it's just that automatic testing can't distinguish when one thing is turned off by Javascript, and another thing is turned on, so it thinks the two elements overlay each other. That's why it's flagged as a warning, not an error - it's recommending that this issue is checked manually. There isn't a standard way to flag to the checker that this is okay, and shouldn't cause a warning, so the easiest thing is to turn off that warning. Alternatively we could turn off all warnings, and just look for errors, but this gives better coverage than that approach.

@gkatsev
Copy link
Member

gkatsev commented Feb 2, 2017

We'd want a similar fix against master.

@gkatsev gkatsev added 5.x a11y This item might affect the accessibility of the player labels Feb 2, 2017
@gkatsev gkatsev merged commit daad492 into videojs:5.x Feb 7, 2017
@OwenEdwards OwenEdwards deleted the fix/remove-test-a11y-warning branch March 4, 2017 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.x a11y This item might affect the accessibility of the player
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants