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

Fix Captions Settings Dialog labeling for accessibility #3281

Conversation

OwenEdwards
Copy link
Member

Description

Based on #3054, this PR fixes screen reader labeling of the Captions Settings Dialog. This fixes point 1 of #2746.

Specific Changes proposed

Add fieldsets & legends, and off-screen text for labels. Also, remove the '---' values.

Requirements Checklist

  • Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Unit Tests updated or fixed
  • Reviewed by Two Core Contributors


let template = `
<div role="document">
<h1 id="${dialogLabelId}" class="vjs-control-text">Captions Settings Dialog</h1>
Copy link
Member

Choose a reason for hiding this comment

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

We probably should use a div here instead of h1, I had to change the elements used in videojs-dock because websites often style h1 elements and it broke the title and descriptions of the plugin

Copy link
Member Author

Choose a reason for hiding this comment

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

Even though class="vjs-control-text" means this one is visually hidden, and just there for Screen Readers?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right, I guess that's fine then.

@gkatsev gkatsev added minor This PR can be added to a minor release. It should not be added to a patch release. needs: updates labels Apr 29, 2016
@gkatsev
Copy link
Member

gkatsev commented Jun 20, 2016

Hey, @OwenEdwards any updates on this one? Thoughts on my latest comment?

@gkatsev gkatsev closed this in 238c752 Jul 19, 2016
@OwenEdwards OwenEdwards deleted the fix/captions-settings-labeling-for-a11y branch July 22, 2016 21:52
@OwenEdwards OwenEdwards restored the fix/captions-settings-labeling-for-a11y branch July 22, 2016 22:23
@OwenEdwards OwenEdwards deleted the fix/captions-settings-labeling-for-a11y branch July 22, 2016 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor This PR can be added to a minor release. It should not be added to a patch release. needs: updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants