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

Apply aria-hidden to control-bar when userinactive #3023

Closed
wants to merge 3 commits into from

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Jan 22, 2016

Defaults to aria-hidden:true because the controls are hidden by default.

The next step here is to try and prevent the control bar from hiding when it has keyboard focus.

Defaults to aria-hidden:true because the controls are hidden by default.
@gkatsev
Copy link
Member Author

gkatsev commented Jan 22, 2016

And got it to not go inactive when focused inside the player. That was surprisingly easy.
Though, I do wonder whether we want to do that or not. I guess that we could make sure that when <Esc> is pressed, we would blur out of the player so it can go inactive or something.
@OwenEdwards thoughts?

@@ -2256,6 +2256,11 @@ class Player extends Component {
this.addClass('vjs-user-active');
this.trigger('useractive');
} else {
// if we are focused anywhere in the player, don't go inactive
if (player.el().contains(document.activeElement)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

maybe this should be player.controlBar.el().contains(document.activeElement)?
And then <Esc> would focus the player div?

@gkatsev gkatsev added the a11y This item might affect the accessibility of the player label Jan 22, 2016
@@ -2256,6 +2256,11 @@ class Player extends Component {
this.addClass('vjs-user-active');
this.trigger('useractive');
} else {
// if we are focused anywhere in the player, don't go inactive
if (this.el().contains(document.activeElement)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

maybe this should be this.controlBar.el().contains(document.activeElement)?
And then <Esc> would focus the player div?

@gkatsev
Copy link
Member Author

gkatsev commented Jan 22, 2016

This should help with #2156 but it doesn't currently fix the trying to tab back into the player part of that issue.

@gkatsev
Copy link
Member Author

gkatsev commented Jan 22, 2016

Talking this over with @OwenEdwards, we realized we want to do basically the opposite of what this PR does. When the controls go inactive, it should only be visual. The controls should still work for Screen Readers. I'll close this and approach this problem from that direction.

@gkatsev gkatsev closed this Jan 22, 2016
@gkatsev gkatsev deleted the aria-hidden-controlbar branch January 22, 2016 20:47
@OwenEdwards
Copy link
Member

@gkatsev "...still work for Screen Readers" and keyboard-only users. I'm going to keep hammering on that point, that these are two different groups of users (although they experience related accessibility issues). Honestly, you'll miss me when I don't need to mention it again ;-)

@gkatsev
Copy link
Member Author

gkatsev commented Jan 22, 2016

Yeah, it didn't feel right to not go inactive on focus but didn't understand exactly where I was going wrong. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

2 participants