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

Restore all outlines #3829

Merged
merged 2 commits into from
Jan 18, 2017
Merged

Restore all outlines #3829

merged 2 commits into from
Jan 18, 2017

Conversation

misteroneill
Copy link
Member

This restores outlines for everything for accessibility purposes.

The progress bar looks awkward, but I will be working on that in another PR.

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.

LGTM, we just need some kind of story/PR to link this too though

@@ -24,7 +24,6 @@
border: 0;
clip: rect(0 0 0 0);
height: 1px;
margin: -1px;
Copy link
Member Author

Choose a reason for hiding this comment

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

For reasoning behind this, see a080f17ebd2ef291a5f0d984f9cdbd106691a733

Copy link
Contributor

Choose a reason for hiding this comment

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

seems like a reasonable change, still LGTM

@misteroneill
Copy link
Member Author

Added one more commit to this PR.

@misteroneill misteroneill mentioned this pull request Dec 6, 2016
2 tasks
This looks like it was copy/pasted from HTML5 Boilerplate, but there
does not seem to be a purpose to the `margin: -1px;` line and it
causes unsightly "bumps" in the outline of certain elements (sliders).

I browsed through 5 years of history of HTML5 Boilerplate and gave up
looking for a justification for this.

The WebAIM link they even used to cite (and the snook.ca link they
currently cite) does not mention having this margin:

http://webaim.org/techniques/css/invisiblecontent/
@gkatsev gkatsev merged commit 29ffbfb into videojs:master Jan 18, 2017
@misteroneill misteroneill deleted the outlines branch January 19, 2017 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants