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

Fixes #2156 make controls visible #3237

Closed

Conversation

chrisauclair
Copy link
Contributor

Description

Fixes the a11y issue in #2156, where keyboard and screen reader users lose control focus when the control bar hides due to inactivity. I set the controls to visibility: visible and tested with NVDA and JAWS across all supported browsers.

For IE8, I opted to keep the controls hidden because -ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=0)"; won't make all of the children transparent. More work could be done on the controls for IE8, if someone can make a compelling case for it.

@@ -10,6 +10,15 @@
@include background-color-with-alpha($primary-background-color, $primary-background-transparency);
}

// IE8 is flakey with fonts, and you have to change the actual content to force
Copy link
Member

Choose a reason for hiding this comment

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

why move this section?

Copy link
Member

Choose a reason for hiding this comment

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

oh, I see, it's for the variable. Maybe move just the variable but lease the section where it was?

@gkatsev gkatsev added needs: LGTM Needs one or more additional approvals a11y This item might affect the accessibility of the player labels Apr 5, 2016
@gkatsev
Copy link
Member

gkatsev commented Apr 5, 2016

LGTM.
@OwenEdwards can you take a look? Thanks.

@OwenEdwards
Copy link
Member

LGTM 👍

@gkatsev gkatsev added confirmed minor This PR can be added to a minor release. It should not be added to a patch release. and removed needs: LGTM Needs one or more additional approvals labels Apr 8, 2016
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 confirmed minor This PR can be added to a minor release. It should not be added to a patch release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants